djsundog/circuitpython
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

py: Fix native functions so they run with their correct globals context.

Browse Source 
Prior to this commit a function compiled with the native decorator
@micropython.native would not work correctly when accessing global
variables, because the globals dict was not being set upon function entry.

This commit fixes this problem by, upon function entry, setting as the
current globals dict the globals dict context the function was defined
within, as per normal Python semantics, and as bytecode does.  Upon
function exit the original globals dict is restored.

In order to restore the globals dict when an exception is raised the native
function must guard its internals with an nlr_push/nlr_pop pair.  Because
this push/pop is relatively expensive, in both C stack usage for the
nlr_buf_t and CPU execution time, the implementation here optimises things
as much as possible.  First, the compiler keeps track of whether a function
even needs to access global variables.  Using this information the native
emitter then generates three different kinds of code:

1. no globals used, no exception handlers: no nlr handling code and no
   setting of the globals dict.

2. globals used, no exception handlers: an nlr_buf_t is allocated on the
   C stack but it is not used if the globals dict is unchanged, saving
   execution time because nlr_push/nlr_pop don't need to run.

3. function has exception handlers, may use globals: an nlr_buf_t is
   allocated and nlr_push/nlr_pop are always called.

In the end, native functions that don't access globals and don't have
exception handlers will run more efficiently than those that do.

Fixes issue #1573.
This commit is contained in:
Damien George 2018-09-13 22:03:48 +10:00
parent 9fb1f18cf4
commit 4f3d9429b5
7 changed files with 96 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
py/compile.c
View File

@ -567,6 +567,11 @@ STATIC void close_over_variables_etc(compiler_t *comp, scope_t *this_scope, int
}
this_scope->num_def_pos_args = n_pos_defaults;
#if MICROPY_EMIT_NATIVE
// When creating a function/closure it will take a reference to the current globals
comp->scope_cur->scope_flags |= MP_SCOPE_FLAG_REFGLOBALS;
#endif
// make closed over variables, if any
// ensure they are closed over in the order defined in the outer scope (mainly to agree with CPython)
int nfree = 0;
@ -3304,6 +3309,12 @@ STATIC void scope_compute_things(scope_t *scope) {
if (SCOPE_IS_FUNC_LIKE(scope->kind) && id->kind == ID_INFO_KIND_GLOBAL_IMPLICIT) {
id->kind = ID_INFO_KIND_GLOBAL_EXPLICIT;
}
#if MICROPY_EMIT_NATIVE
if (id->kind == ID_INFO_KIND_GLOBAL_EXPLICIT) {
// This function makes a reference to a global variable
scope->scope_flags |= MP_SCOPE_FLAG_REFGLOBALS;
}
#endif
// params always count for 1 local, even if they are a cell
if (id->kind == ID_INFO_KIND_LOCAL || (id->flags & ID_FLAG_IS_PARAM)) {
id->local_num = scope->num_locals++;

87
py/emitnative.c
View File

@ -75,7 +75,8 @@
#define NLR_BUF_IDX_RET_VAL (1)
// Whether the native/viper function needs to be wrapped in an exception handler
#define NEED_GLOBAL_EXC_HANDLER(emit) ((emit)->scope->exc_stack_size > 0)
#define NEED_GLOBAL_EXC_HANDLER(emit) ((emit)->scope->exc_stack_size > 0 \
|| (!(emit)->do_viper_types && ((emit)->scope->scope_flags & MP_SCOPE_FLAG_REFGLOBALS)))
// Whether registers can be used to store locals (only true if there are no
// exception handlers, because otherwise an nlr_jump will restore registers to
@ -928,30 +929,56 @@ STATIC void emit_native_global_exc_entry(emit_t *emit) {
mp_uint_t start_label = *emit->label_slot + 2;
mp_uint_t global_except_label = *emit->label_slot + 3;
// Clear the unwind state
ASM_XOR_REG_REG(emit->as, REG_TEMP0, REG_TEMP0);
ASM_MOV_LOCAL_REG(emit->as, LOCAL_IDX_EXC_HANDLER_UNWIND(emit), REG_TEMP0);
if (!emit->do_viper_types) {
// Set new globals
ASM_MOV_REG_LOCAL(emit->as, REG_ARG_1, offsetof(mp_code_state_t, fun_bc) / sizeof(uintptr_t));
ASM_LOAD_REG_REG_OFFSET(emit->as, REG_ARG_1, REG_ARG_1, offsetof(mp_obj_fun_bc_t, globals) / sizeof(uintptr_t));
emit_call(emit, MP_F_NATIVE_SWAP_GLOBALS);
// Put PC of start code block into REG_LOCAL_1
ASM_MOV_REG_PCREL(emit->as, REG_LOCAL_1, start_label);
// Save old globals (or NULL if globals didn't change)
ASM_MOV_LOCAL_REG(emit->as, offsetof(mp_code_state_t, old_globals) / sizeof(uintptr_t), REG_RET);
}
// Wrap everything in an nlr context
emit_native_label_assign(emit, nlr_label);
ASM_MOV_REG_LOCAL(emit->as, REG_LOCAL_2, LOCAL_IDX_EXC_HANDLER_UNWIND(emit));
emit_get_stack_pointer_to_reg_for_push(emit, REG_ARG_1, sizeof(nlr_buf_t) / sizeof(uintptr_t));
emit_call(emit, MP_F_NLR_PUSH);
ASM_MOV_LOCAL_REG(emit->as, LOCAL_IDX_EXC_HANDLER_UNWIND(emit), REG_LOCAL_2);
ASM_JUMP_IF_REG_NONZERO(emit->as, REG_RET, global_except_label, true);
if (emit->scope->exc_stack_size == 0) {
// Optimisation: if globals didn't change don't push the nlr context
ASM_JUMP_IF_REG_ZERO(emit->as, REG_RET, start_label, false);
// Clear PC of current code block, and jump there to resume execution
ASM_XOR_REG_REG(emit->as, REG_TEMP0, REG_TEMP0);
ASM_MOV_LOCAL_REG(emit->as, LOCAL_IDX_EXC_HANDLER_PC(emit), REG_TEMP0);
ASM_JUMP_REG(emit->as, REG_LOCAL_1);
// Wrap everything in an nlr context
emit_get_stack_pointer_to_reg_for_push(emit, REG_ARG_1, sizeof(nlr_buf_t) / sizeof(uintptr_t));
emit_call(emit, MP_F_NLR_PUSH);
ASM_JUMP_IF_REG_ZERO(emit->as, REG_RET, start_label, true);
} else {
// Clear the unwind state
ASM_XOR_REG_REG(emit->as, REG_TEMP0, REG_TEMP0);
ASM_MOV_LOCAL_REG(emit->as, LOCAL_IDX_EXC_HANDLER_UNWIND(emit), REG_TEMP0);
// Global exception handler: check for valid exception handler
emit_native_label_assign(emit, global_except_label);
ASM_MOV_REG_LOCAL(emit->as, REG_LOCAL_1, LOCAL_IDX_EXC_HANDLER_PC(emit));
ASM_JUMP_IF_REG_NONZERO(emit->as, REG_LOCAL_1, nlr_label, false);
// Put PC of start code block into REG_LOCAL_1
ASM_MOV_REG_PCREL(emit->as, REG_LOCAL_1, start_label);
// Wrap everything in an nlr context
emit_native_label_assign(emit, nlr_label);
ASM_MOV_REG_LOCAL(emit->as, REG_LOCAL_2, LOCAL_IDX_EXC_HANDLER_UNWIND(emit));
emit_get_stack_pointer_to_reg_for_push(emit, REG_ARG_1, sizeof(nlr_buf_t) / sizeof(uintptr_t));
emit_call(emit, MP_F_NLR_PUSH);
ASM_MOV_LOCAL_REG(emit->as, LOCAL_IDX_EXC_HANDLER_UNWIND(emit), REG_LOCAL_2);
ASM_JUMP_IF_REG_NONZERO(emit->as, REG_RET, global_except_label, true);
// Clear PC of current code block, and jump there to resume execution
ASM_XOR_REG_REG(emit->as, REG_TEMP0, REG_TEMP0);
ASM_MOV_LOCAL_REG(emit->as, LOCAL_IDX_EXC_HANDLER_PC(emit), REG_TEMP0);
ASM_JUMP_REG(emit->as, REG_LOCAL_1);
// Global exception handler: check for valid exception handler
emit_native_label_assign(emit, global_except_label);
ASM_MOV_REG_LOCAL(emit->as, REG_LOCAL_1, LOCAL_IDX_EXC_HANDLER_PC(emit));
ASM_JUMP_IF_REG_NONZERO(emit->as, REG_LOCAL_1, nlr_label, false);
}
if (!emit->do_viper_types) {
// Restore old globals
ASM_MOV_REG_LOCAL(emit->as, REG_ARG_1, offsetof(mp_code_state_t, old_globals) / sizeof(uintptr_t));
emit_call(emit, MP_F_NATIVE_SWAP_GLOBALS);
}
// Re-raise exception out to caller
ASM_MOV_REG_LOCAL(emit->as, REG_ARG_1, LOCAL_IDX_EXC_VAL(emit));
@ -967,10 +994,28 @@ STATIC void emit_native_global_exc_exit(emit_t *emit) {
emit_native_label_assign(emit, emit->exit_label);
if (NEED_GLOBAL_EXC_HANDLER(emit)) {
if (!emit->do_viper_types) {
// Get old globals
ASM_MOV_REG_LOCAL(emit->as, REG_ARG_1, offsetof(mp_code_state_t, old_globals) / sizeof(uintptr_t));
if (emit->scope->exc_stack_size == 0) {
// Optimisation: if globals didn't change then don't restore them and don't do nlr_pop
ASM_JUMP_IF_REG_ZERO(emit->as, REG_ARG_1, emit->exit_label + 1, false);
}
// Restore old globals
emit_call(emit, MP_F_NATIVE_SWAP_GLOBALS);
}
// Pop the nlr context
emit_call(emit, MP_F_NLR_POP);
adjust_stack(emit, -(mp_int_t)(sizeof(nlr_buf_t) / sizeof(uintptr_t)));
if (emit->scope->exc_stack_size == 0) {
// Destination label for above optimisation
emit_native_label_assign(emit, emit->exit_label + 1);
}
// Load return value
ASM_MOV_REG_LOCAL(emit->as, REG_RET, LOCAL_IDX_RET_VAL(emit));
}

1
py/emitnx86.c
View File

@ -18,6 +18,7 @@
STATIC byte mp_f_n_args[MP_F_NUMBER_OF] = {
[MP_F_CONVERT_OBJ_TO_NATIVE] = 2,
[MP_F_CONVERT_NATIVE_TO_OBJ] = 2,
[MP_F_NATIVE_SWAP_GLOBALS] = 1,
[MP_F_LOAD_NAME] = 1,
[MP_F_LOAD_GLOBAL] = 1,
[MP_F_LOAD_BUILD_CLASS] = 0,

15
py/nativeglue.c
View File

@ -83,6 +83,20 @@ mp_obj_t mp_convert_native_to_obj(mp_uint_t val, mp_uint_t type) {
#if MICROPY_EMIT_NATIVE
mp_obj_dict_t *mp_native_swap_globals(mp_obj_dict_t *new_globals) {
if (new_globals == NULL) {
// Globals were the originally the same so don't restore them
return NULL;
}
mp_obj_dict_t *old_globals = mp_globals_get();
if (old_globals == new_globals) {
// Don't set globals if they are the same, and return NULL to indicate this
return NULL;
}
mp_globals_set(new_globals);
return old_globals;
}
// wrapper that accepts n_args and n_kw in one argument
// (native emitter can only pass at most 3 arguments to a function)
mp_obj_t mp_native_call_function_n_kw(mp_obj_t fun_in, size_t n_args_kw, const mp_obj_t *args) {
@ -127,6 +141,7 @@ STATIC mp_obj_t mp_native_iternext(mp_obj_iter_buf_t *iter) {
void *const mp_fun_table[MP_F_NUMBER_OF] = {
mp_convert_obj_to_native,
mp_convert_native_to_obj,
mp_native_swap_globals,
mp_load_name,
mp_load_global,
mp_load_build_class,

1
py/runtime.h
View File

@ -168,6 +168,7 @@ NORETURN void mp_raise_recursion_depth(void);
// helper functions for native/viper code
mp_uint_t mp_convert_obj_to_native(mp_obj_t obj, mp_uint_t type);
mp_obj_t mp_convert_native_to_obj(mp_uint_t val, mp_uint_t type);
mp_obj_dict_t *mp_native_swap_globals(mp_obj_dict_t *new_globals);
mp_obj_t mp_native_call_function_n_kw(mp_obj_t fun_in, size_t n_args_kw, const mp_obj_t *args);
void mp_native_raise(mp_obj_t o);

2
py/runtime0.h
View File

@ -31,6 +31,7 @@
#define MP_SCOPE_FLAG_VARKEYWORDS (0x02)
#define MP_SCOPE_FLAG_GENERATOR (0x04)
#define MP_SCOPE_FLAG_DEFKWARGS (0x08)
#define MP_SCOPE_FLAG_REFGLOBALS (0x10) // used only if native emitter enabled
// types for native (viper) function signature
#define MP_NATIVE_TYPE_OBJ (0x00)
@ -145,6 +146,7 @@ typedef enum {
typedef enum {
MP_F_CONVERT_OBJ_TO_NATIVE = 0,
MP_F_CONVERT_NATIVE_TO_OBJ,
MP_F_NATIVE_SWAP_GLOBALS,
MP_F_LOAD_NAME,
MP_F_LOAD_GLOBAL,
MP_F_LOAD_BUILD_CLASS,

1
tests/run-tests
View File

@ -376,7 +376,6 @@ def run_tests(pyb, tests, args, base_path="."):
skip_tests.add('micropython/schedule.py') # native code doesn't check pending events
skip_tests.add('stress/gc_trace.py') # requires yield
skip_tests.add('stress/recursive_gen.py') # requires yield
skip_tests.add('extmod/vfs_userfs.py') # because native doesn't properly handle globals across different modules
for test_file in tests:
test_file = test_file.replace('\\', '/')