Using Pointer to State in NLR Assembly

C programming, build, interpreter/VM.
Target audience: MicroPython Developers.
Post Reply
oclyke
Posts: 18
Joined: Tue Mar 19, 2019 4:55 am

Using Pointer to State in NLR Assembly

Post by oclyke » Tue Mar 19, 2019 7:44 am

Hi everyone.

I'd like to submit a PR soon for changing the MP_STATE_XXX(x) macros so that they access the current state through a pointer. I was up-to-date with the repo at commit [41e7ad647] then I diverged and got demo code working on an ESP32. Now I'm making the effort to consolidate my changes into a neat and minimal PR. I started with a recent commit ec6e62efc and began making the essential changes when I found trouble with the NLR code.

Wow, I don't even know what NLR stands for an am having trouble finding it online but I get the impression that NLR is handling exceptions. So, usually I'm gung-ho to research a problem out of curiosity but that energy all drained out of me staring at assembly for x64 and several other platforms...

Here's what I'm seeing:

Code: Select all

LINK mpy-cross
Undefined symbols for architecture x86_64:
  "_p_mp_active_state_ctx", referenced from:
      _nlr_push_tail in nlr.o
      _nlr_pop in nlr.o
      _nlr_jump in nlrx64.o
      _gc_init in gc.o
      _gc_lock in gc.o
      _gc_unlock in gc.o
      _gc_is_locked in gc.o
      ...
ld: symbol(s) not found for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)
make[1]: *** [mpy-cross] Error 1
Which presumably has to do with my changes to mp_state.h:

Code: Select all

#define MP_STATE_THREAD(x)
was:

Code: Select all

(mp_state_ctx.thread.x)
but is changed to

Code: Select all

(p_mp_active_state_ctx->thread.x)
and you can assume the pointer points at a valid state structure

In thumb, x64, x86, and xtensa ___nlr.c files (in 'void nlr_jump( void )') we see the MP_NLR_JUMP_HEAD(val, top) macro which uses the THREAD macro mentioned above, however the macro is not inside the inline assembly... And ultimately the type of the thing accessed via pointer is still the same. This calls to mind how after this change you cannot use the STATE macros in initializer lists because access through a pointer isn't constant. Is this related?

So can someone help explain to me why this pointer access doesn't work here? Even better can someone point me in the right direction to fix this? Will I need to go learn assembly for these platforms? :shock:

Thanks!
Last edited by oclyke on Tue Mar 19, 2019 3:01 pm, edited 1 time in total.

stijn
Posts: 735
Joined: Thu Apr 24, 2014 9:13 am

Re: Using Pointer to State in NLR Assembly

Post by stijn » Tue Mar 19, 2019 12:06 pm

Can you link to the actual code causing the problem?

oclyke
Posts: 18
Joined: Tue Mar 19, 2019 4:55 am

Re: Using Pointer to State in NLR Assembly

Post by oclyke » Tue Mar 19, 2019 2:55 pm

Sure, here is a link to my fork. For what its worth this is not specific to the ESP32 port - you should be able to see the error if you just try to build mpy-cross.

stijn
Posts: 735
Joined: Thu Apr 24, 2014 9:13 am

Re: Using Pointer to State in NLR Assembly

Post by stijn » Tue Mar 19, 2019 4:07 pm

At first sight: there's a declaration of p_mp_active_state_ctx but no definition?

oclyke
Posts: 18
Joined: Tue Mar 19, 2019 4:55 am

Re: Using Pointer to State in NLR Assembly

Post by oclyke » Tue Mar 19, 2019 4:28 pm

Oops, you got it - thanks for spotting that! Ahh what a relief. I was so blinded by worry that some recent changes to the ESP32 port had caused this that I must've just forgot to make that little change. What I'm taking away from this is that the link error was trickier to spot than usual, probably because of my inexperience with assembly.

Is it right to say that pointers somehow get prepended with an underscore in assembly? I was partly thrown off by not being able to find '_p_mp_active_state_ctx' anywhere in my source.

User avatar
dhylands
Posts: 3821
Joined: Mon Jan 06, 2014 6:08 pm
Location: Peachland, BC, Canada
Contact:

Re: Using Pointer to State in NLR Assembly

Post by dhylands » Tue Mar 19, 2019 4:35 pm

Prepending C labels with underscores is a common thing that compilers do, but is definitely compiler specific (maybe even platform specific - I seem to recall working with some platforms using gcc where underscores were not prepended, but it was many years ago).

pfalcon
Posts: 1155
Joined: Fri Feb 28, 2014 2:05 pm

Re: Using Pointer to State in NLR Assembly

Post by pfalcon » Wed Mar 20, 2019 7:58 am

oclyke wrote:
Tue Mar 19, 2019 7:44 am
I'd like to submit a PR soon for changing the MP_STATE_XXX(x) macros so that they access the current state through a pointer.
Sorry, but there's no need for such a patch (on its own). The whole idea of these macros is that they can be redefined as needed by interested parties. However, the default definition is the right one and should stay like that.
Wow, I don't even know what NLR stands for
It's pretty sad, because the very first comment (after the copyright blurv) in nlr.h explains it. Perhaps, you should spend more time learning MicroPython codebase before starting to patch it?
So, usually I'm gung-ho to research a problem out of curiosity but that energy all drained out of me staring at assembly for x64 and several other platforms...
Perhaps, you started too hard a task? Did you consider starting your endeavor with MicroPython by using it, not changing it?
Awesome MicroPython list
Pycopy - A better MicroPython https://github.com/pfalcon/micropython
MicroPython standard library for all ports and forks - https://github.com/pfalcon/micropython-lib
More up to date docs - http://pycopy.readthedocs.io/

oclyke
Posts: 18
Joined: Tue Mar 19, 2019 4:55 am

Re: Using Pointer to State in NLR Assembly

Post by oclyke » Wed Mar 20, 2019 4:21 pm

dhylands, thanks for letting me know - learning something new all the time!
pfalcon wrote:Sorry, but there's no need for such a patch (on its own).
Thanks - your parenthetical is pretty enticing. What else would you want to see along with such a patch?

I'd still argue in favor of making this change and here's why:
- As development continues making this change will become more challenging. It's not quite as simple as redefining the macros thanks to a few initializer lists that contain the macro as well as a few direct references to 'mp_state_ctx' in port files. Therefore changing the macros to operate through a pointer, even if they still point at a single statically allocated state array, would keep future additions to the code compatible with a change that interested parties may like to make.
- Such a change would have little to no noticeable effect in MicroPython in terms of performance or size - is that wrong?

pfalcon wrote:However, the default definition is the right one and should stay like that.
I'd definitely like to understand the reason you say this. Some evidence or reasons could definitely help me understand the project better. As for learning the codebase there seems to me to be no better way than diving in and trying it out.

pfalcon
Posts: 1155
Joined: Fri Feb 28, 2014 2:05 pm

Re: Using Pointer to State in NLR Assembly

Post by pfalcon » Sun Mar 24, 2019 5:06 pm

oclyke wrote:
Wed Mar 20, 2019 4:21 pm
pfalcon wrote:Sorry, but there's no need for such a patch (on its own).
Thanks - your parenthetical is pretty enticing. What else would you want to see along with such a patch?
Well, your goal appears to be to implement multiple interpreter instances, so if changes for MP_STATE_THREAD() will be included in a properly implemented patchset for multiple interps, there might be no questions specifically about the (properly implemented) MP_STATE_THREAD() part. Instead, all questions will shift to the entire "multiple interps" patch itself ;-).
I'd still argue in favor of making this change and here's why:
- As development continues making this change will become more challenging. It's not quite as simple as redefining the macros thanks to a few initializer lists that contain the macro as well as a few direct references to 'mp_state_ctx' in port files.
So, that's where issue lies, and that's what needs to be fixed. The idea is that it should be possible to do anything by redefining those macros. In reality, it may be not that simple indeed, but the solution is to add missing parts, not conjugate everything to not an efficient solution.
Therefore changing the macros to operate through a pointer, even if they still point at a single statically allocated state array, would keep future additions to the code compatible with a change that interested parties may like to make.
There's not need to use pointer to access a single statically allocated state array, consequently there's no need to redefine the default implementation of MP_STATE_THREAD(). It's as simple as that, really. The principle is known for centuries (millennia?): https://en.wikipedia.org/wiki/Occam%27s_razor
- Such a change would have little to no noticeable effect in MicroPython in terms of performance or size - is that wrong?
Depends on a way to measure. Someone may say that adding "for (int i = 0; i < 100; i++);" after each statement in MicroPython code has "no noticeable effect". The criteria which I (the guy who contributed at least 1/3 of changes to MicroPython) use and recommend:
  • If something takes less than half a bit of memory, it has "no noticeable effect". Anything else has "very noticeable effect".
  • Likewise, if something takes less than half a clock of CPU time, it has "no noticeable effect". Anything else has "very noticeable effect".
pfalcon wrote:However, the default definition is the right one and should stay like that.
I'd definitely like to understand the reason you say this. Some evidence or reasons could definitely help me understand the project better.
Sure, a lot of effort was spent to let anyone interested to find that info easily, without need to ask, then wait long for replies, etc. E.g., it's in the README of MicroPython project: https://github.com/micropython/micropython#contributing . Or very this subforum where you posted a message has a pinned topic Contributor Guidelines.
Awesome MicroPython list
Pycopy - A better MicroPython https://github.com/pfalcon/micropython
MicroPython standard library for all ports and forks - https://github.com/pfalcon/micropython-lib
More up to date docs - http://pycopy.readthedocs.io/

oclyke
Posts: 18
Joined: Tue Mar 19, 2019 4:55 am

Re: Using Pointer to State in NLR Assembly

Post by oclyke » Mon Mar 25, 2019 12:29 am

@pfalcon thanks, the first couple of bullet points there actually helped clarify why the default implementation should remain untouched. Those are some pretty strict guidelines for what is noticeable -- but it figures that you've got to be selective when advancing such a popular project :D

In the meantime I'll keep chugging on multiple interpreters, and anyone is welcome to come visit.

Post Reply