py: Put all global state together in state structures. #1032
Conversation
| @@ -64,8 +64,6 @@ struct _nlr_buf_t { | |||
| #endif | |||
| }; | |||
|
|
|||
| extern nlr_buf_t *nlr_top; | |||
There was a problem hiding this comment.
Removing this declaration makes the code below fail when MICROPY_NLR_SETJMP == 1. Should probably #include "py/mpstate.h" in that case?
There was a problem hiding this comment.
Since it's part of the global structure (btw what does ctx stand for?) you now also need something like
diff --git a/py/nlr.h b/py/nlr.h
index a8a8315..679cab8 100644
--- a/py/nlr.h
+++ b/py/nlr.h
@@ -68,8 +68,8 @@ struct _nlr_buf_t {
NORETURN void nlr_setjmp_jump(void *val);
// nlr_push() must be defined as a macro, because "The stack context will be
// invalidated if the function which called setjmp() returns."
-#define nlr_push(buf) ((buf)->prev = nlr_top, nlr_top = (buf), setjmp((buf)->jmpbuf))
-#define nlr_pop() { nlr_top = nlr_top->prev; }
+#define nlr_push(buf) ((buf)->prev = MP_STATE_CTX(nlr_top), MP_STATE_CTX(nlr_top) = (buf), setjmp((buf)->jmpbuf))
+#define nlr_pop() { MP_STATE_CTX(nlr_top) = MP_STATE_CTX(nlr_top)->prev; }
#define nlr_jump(val) nlr_setjmp_jump(val)
#else
unsigned int nlr_push(nlr_buf_t *);
diff --git a/py/nlrsetjmp.c b/py/nlrsetjmp.c
index 8b8e8d9..efd99ee 100644
--- a/py/nlrsetjmp.c
+++ b/py/nlrsetjmp.c
@@ -29,8 +29,8 @@
#if MICROPY_NLR_SETJMP
void nlr_setjmp_jump(void *val) {
- nlr_buf_t *buf = nlr_top;
- nlr_top = buf->prev;
+ nlr_buf_t *buf = MP_STATE_CTX(nlr_top);
+ MP_STATE_CTX(nlr_top) = buf->prev;
buf->ret_val = val;
longjmp(buf->jmpbuf, 1);
}
There was a problem hiding this comment.
ctx is common abbreviation for "context".
There was a problem hiding this comment.
Yes, I'll fix that nlr_top reference.
|
I updated the PR: added some comments to better explain what is going on, and put all the global state structs into 1 big struct. So now there really is only 1 structure. nlr seems to work for x64 and thumb, but not xtensa (esp8266 port). |
|
Looks good. You surely checked code size after these changes to exclude gcc codegeneration wonders? Otherwise, let's have this merged? |
I did, but I want to check again.
Yes, I'll just tidy a few more little things, fix nlrxtensa, then merge it. |
|
Rebased and merged in b4b10fd. unix, stmhal and esp8266 port are all tested and working. Assuming teensy and windows ports work. Code size changes:
All-in-all a good change, especially since GC can be more efficient and precise, and global state is all in one place. |
This is a second attempt at consolidating global state into one place. It supersedes #1017.
Improvements upon previous attempt: