CircuitPython - a Python implementation for teaching coding with microcontrollers https://git.suspectdevices.com/suspect-devices/circuitpython/atom?h=5.3.x 2019-12-12T19:10:10+00:00 2019-12-12T19:10:10+00:00 Damien George damien.p.george@gmail.com 2019-06-17T13:19:34+00:00 urn:sha1:767d47dd6025e8046b03e6b307fd0ba6a81cf504 These s16-s21 registers are used by gcc so need to be saved. Future versions of gcc (beyond v9.1.0), or other compilers, may eventually need additional registers saved/restored. See issue #4844. 2019-12-12T19:01:39+00:00 Damien George damien.p.george@gmail.com 2019-08-19T05:50:02+00:00 urn:sha1:63046d800d64081a4f7756476f5249179179ff5e And use it to replace the same pattern at the end of nlrthumb.c:nlr_jump. 2018-07-11T20:45:30+00:00 Dan Halbert halbert@halwitz.org 2018-07-11T20:45:30+00:00 urn:sha1:7c219600a246d8956d0b23ea3f5d125a820e6b6a 2018-04-27T05:10:42+00:00 Ayke van Laethem aykevanlaethem@gmail.com 2018-04-24T15:47:45+00:00 urn:sha1:deaa46aa66230071d355c0ffd004ccf0cbefd8b9 Clang defines __GNUC__ so we have to check for it specifically. 2018-02-18T00:35:27+00:00 Ayke van Laethem aykevanlaethem@gmail.com 2018-02-13T21:00:20+00:00 urn:sha1:5591bd237a69a3a1a6ac03cb1dc9edcde835f708 By adding __builtin_unreachable() at the end of nlr_push, we're essentially telling the compiler that this function will never return. When GCC LTO is in use, this means that any time nlr_push() is called (which is often), the compiler thinks this function will never return and thus eliminates all code following the call. Note: I've added a 'return 0' for older GCC versions like 4.6 which complain about not returning anything (which doesn't make sense in a naked function). Newer GCC versions (tested 4.8, 5.4 and some others) don't complain about this. 2018-01-02T21:41:34+00:00 Dan Halbert halbert@halwitz.org 2018-01-02T21:41:34+00:00 urn:sha1:ce81c8dda9686d3cd78d9c5383b6982c148ac7ec Avoid gcc 7.2.1 compiler issues in nlr_push() 2017-12-28T05:46:30+00:00 Damien George damien.p.george@gmail.com 2017-12-28T05:46:30+00:00 urn:sha1:b25f92160b318a096c516c430afde5472a944c19 Each NLR implementation (Thumb, x86, x64, xtensa, setjmp) duplicates a lot of the NLR code, specifically that dealing with pushing and popping the NLR pointer to maintain the linked-list of NLR buffers. This patch factors all of that code out of the specific implementations into generic functions in nlr.c, along with a helper macro in nlr.h. This eliminates duplicated code. 2017-12-28T05:18:39+00:00 Damien George damien.p.george@gmail.com 2017-12-28T05:18:39+00:00 urn:sha1:5bf8e85fc828974199d469db711aa2f9649c467b If MICROPY_NLR_SETJMP is not enabled and the machine is auto-detected then nlr.h now defines some convenience macros for the individual NLR implementations to use (eg MICROPY_NLR_THUMB). This keeps nlr.h and the implementation in sync, and also makes the nlr_buf_t struct easier to read. 2017-12-28T04:59:09+00:00 Damien George damien.p.george@gmail.com 2017-12-28T04:59:09+00:00 urn:sha1:97cc48553828ed091325d0d3922eb4cd6c377221 A function with a naked attribute must only contain basic inline asm statements and no C code. For nlr_push this means removing the "return 0" statement. But for some gcc versions this induces a compiler warning so the __builtin_unreachable() line needs to be added. For nlr_jump, this function contains a combination of C code and inline asm so cannot be naked. 2017-12-26T17:27:58+00:00 Paul Sokolovsky pfalcon@users.sourceforge.net 2017-12-26T16:39:51+00:00 urn:sha1:096e967aad6df760c16de4878b8b2eea02330011 This reverts commit 6a3a742a6c9caaa2be0fd0aac7a5df4ac816081c. The above commit has number of faults starting from the motivation down to the actual implementation. 1. Faulty implementation. The original code contained functions like: NORETURN void nlr_jump(void *val) { nlr_buf_t **top_ptr = &MP_STATE_THREAD(nlr_top); nlr_buf_t *top = *top_ptr; ... __asm volatile ( "mov %0, %%edx \n" // %edx points to nlr_buf "mov 28(%%edx), %%esi \n" // load saved %esi "mov 24(%%edx), %%edi \n" // load saved %edi "mov 20(%%edx), %%ebx \n" // load saved %ebx "mov 16(%%edx), %%esp \n" // load saved %esp "mov 12(%%edx), %%ebp \n" // load saved %ebp "mov 8(%%edx), %%eax \n" // load saved %eip "mov %%eax, (%%esp) \n" // store saved %eip to stack "xor %%eax, %%eax \n" // clear return register "inc %%al \n" // increase to make 1, non-local return "ret \n" // return : // output operands : "r"(top) // input operands : // clobbered registers ); } Which clearly stated that C-level variable should be a parameter of the assembly, whcih then moved it into correct register. Whereas now it's: NORETURN void nlr_jump_tail(nlr_buf_t *top) { (void)top; __asm volatile ( "mov 28(%edx), %esi \n" // load saved %esi "mov 24(%edx), %edi \n" // load saved %edi "mov 20(%edx), %ebx \n" // load saved %ebx "mov 16(%edx), %esp \n" // load saved %esp "mov 12(%edx), %ebp \n" // load saved %ebp "mov 8(%edx), %eax \n" // load saved %eip "mov %eax, (%esp) \n" // store saved %eip to stack "xor %eax, %eax \n" // clear return register "inc %al \n" // increase to make 1, non-local return "ret \n" // return ); for (;;); // needed to silence compiler warning } Which just tries to perform operations on a completely random register (edx in this case). The outcome is the expected: saving the pure random luck of the compiler putting the right value in the random register above, there's a crash. 2. Non-critical assessment. The original commit message says "There is a small overhead introduced (typically 1 machine instruction)". That machine instruction is a call if a compiler doesn't perform tail optimization (happens regularly), and it's 1 instruction only with the broken code shown above, fixing it requires adding more. With inefficiencies already presented in the NLR code, the overhead becomes "considerable" (several times more than 1%), not "small". The commit message also says "This eliminates duplicated code.". An obvious way to eliminate duplication would be to factor out common code to macros, not introduce overhead and breakage like above. 3. Faulty motivation. All this started with a report of warnings/errors happening for a niche compiler. It could have been solved in one the direct ways: a) fixing it just for affected compiler(s); b) rewriting it in proper assembly (like it was before BTW); c) by not doing anything at all, MICROPY_NLR_SETJMP exists exactly to address minor-impact cases like thar (where a) or b) are not applicable). Instead, a backwards "solution" was put forward, leading to all the issues above. The best action thus appears to be revert and rework, not trying to work around what went haywire in the first place.