From f040685b0cbec18feb981fdad96389a3b28b676d Mon Sep 17 00:00:00 2001 From: Damien George Date: Tue, 27 Sep 2016 12:37:21 +1000 Subject: [PATCH] py: Only store the exception instance on Py stack in bytecode try block. When an exception is raised and is to be handled by the VM, it is stored on the Python value stack so the bytecode can access it. CPython stores 3 objects on the stack for each exception: exc type, exc instance and traceback. uPy followed this approach, but it turns out not to be necessary. Instead, it is enough to store just the exception instance on the Python value stack. The only place where the 3 values are needed explicitly is for the __exit__ handler of a with-statement context, but for these cases the 3 values can be extracted from the single exception instance. This patch removes the need to store 3 values on the stack, and instead just stores the exception instance. Code size is reduced by about 50-100 bytes, the compiler and VM are slightly simpler, generate bytecode is smaller (by 2 bytes for each try block), and the Python value stack is reduced in size for functions that handle exceptions. --- py/compile.c | 9 ++++----- py/emitbc.c | 14 +++++++------- py/vm.c | 51 +++++++++++++++++++-------------------------------- 3 files changed, 30 insertions(+), 44 deletions(-) diff --git a/py/compile.c b/py/compile.c index 7207ac2e02..2fae5c9f64 100644 --- a/py/compile.c +++ b/py/compile.c @@ -1495,6 +1495,8 @@ STATIC void compile_try_except(compiler_t *comp, mp_parse_node_t pn_body, int n_ EMIT_ARG(label_assign, l1); // start of exception handler EMIT(start_except_handler); + // at this point the top of the stack contains the exception instance that was raised + uint l2 = comp_next_label(comp); for (int i = 0; i < n_except; i++) { @@ -1528,16 +1530,13 @@ STATIC void compile_try_except(compiler_t *comp, mp_parse_node_t pn_body, int n_ EMIT_ARG(pop_jump_if, false, end_finally_label); } - EMIT(pop_top); - + // either discard or store the exception instance if (qstr_exception_local == 0) { EMIT(pop_top); } else { compile_store_id(comp, qstr_exception_local); } - EMIT(pop_top); - uint l3 = 0; if (qstr_exception_local != 0) { l3 = comp_next_label(comp); @@ -1561,7 +1560,7 @@ STATIC void compile_try_except(compiler_t *comp, mp_parse_node_t pn_body, int n_ } EMIT_ARG(jump, l2); EMIT_ARG(label_assign, end_finally_label); - EMIT_ARG(adjust_stack_size, 3); // stack adjust for the 3 exception items + EMIT_ARG(adjust_stack_size, 1); // stack adjust for the exception instance } compile_decrease_except_level(comp); diff --git a/py/emitbc.c b/py/emitbc.c index 8c712e1fdc..40485108d3 100644 --- a/py/emitbc.c +++ b/py/emitbc.c @@ -751,10 +751,9 @@ void mp_emit_bc_unwind_jump(emit_t *emit, mp_uint_t label, mp_uint_t except_dept } void mp_emit_bc_setup_with(emit_t *emit, mp_uint_t label) { - // TODO We can probably optimise the amount of needed stack space, since - // we don't actually need 4 slots during the entire with block, only in - // the cleanup handler in certain cases. It needs some thinking. - emit_bc_pre(emit, 4); + // The SETUP_WITH opcode pops ctx_mgr from the top of the stack + // and then pushes 3 entries: __exit__, ctx_mgr, as_value. + emit_bc_pre(emit, 2); emit_write_bytecode_byte_unsigned_label(emit, MP_BC_SETUP_WITH, label); } @@ -762,8 +761,9 @@ void mp_emit_bc_with_cleanup(emit_t *emit, mp_uint_t label) { mp_emit_bc_pop_block(emit); mp_emit_bc_load_const_tok(emit, MP_TOKEN_KW_NONE); mp_emit_bc_label_assign(emit, label); - emit_bc_pre(emit, -4); + emit_bc_pre(emit, 2); // ensure we have enough stack space to call the __exit__ method emit_write_bytecode_byte(emit, MP_BC_WITH_CLEANUP); + emit_bc_pre(emit, -4); // cancel the 2 above, plus the 2 from mp_emit_bc_setup_with } void mp_emit_bc_setup_except(emit_t *emit, mp_uint_t label) { @@ -955,11 +955,11 @@ void mp_emit_bc_yield_from(emit_t *emit) { } void mp_emit_bc_start_except_handler(emit_t *emit) { - mp_emit_bc_adjust_stack_size(emit, 6); // stack adjust for the 3 exception items, +3 for possible UNWIND_JUMP state + mp_emit_bc_adjust_stack_size(emit, 4); // stack adjust for the exception instance, +3 for possible UNWIND_JUMP state } void mp_emit_bc_end_except_handler(emit_t *emit) { - mp_emit_bc_adjust_stack_size(emit, -5); // stack adjust + mp_emit_bc_adjust_stack_size(emit, -3); // stack adjust } #if MICROPY_EMIT_NATIVE diff --git a/py/vm.c b/py/vm.c index da8697fad2..93766bd116 100644 --- a/py/vm.c +++ b/py/vm.c @@ -587,6 +587,8 @@ dispatch_loop: // and __exit__ method (with self) underneath it. Bytecode calls __exit__, // and "deletes" it off stack, shifting "exception control block" // to its place. + // The bytecode emitter ensures that there is enough space on the Python + // value stack to hold the __exit__ method plus an additional 4 entries. if (TOP() == mp_const_none) { // stack: (..., __exit__, ctx_mgr, None) sp[1] = mp_const_none; @@ -620,31 +622,26 @@ dispatch_loop: } sp -= 2; // we removed (__exit__, ctx_mgr) } else { - assert(mp_obj_is_exception_type(TOP())); - // stack: (..., __exit__, ctx_mgr, traceback, exc_val, exc_type) - // Need to pass (sp[0], sp[-1], sp[-2]) as arguments so must reverse the - // order of these on the value stack (don't want to create a temporary - // array because it increases stack footprint of the VM). - mp_obj_t obj = sp[-2]; - sp[-2] = sp[0]; - sp[0] = obj; - mp_obj_t ret_value = mp_call_method_n_kw(3, 0, sp - 4); + assert(mp_obj_is_exception_instance(TOP())); + // stack: (..., __exit__, ctx_mgr, exc_instance) + // Need to pass (exc_type, exc_instance, None) as arguments to __exit__. + sp[1] = sp[0]; + sp[0] = mp_obj_get_type(sp[0]); + sp[2] = mp_const_none; + sp -= 2; + mp_obj_t ret_value = mp_call_method_n_kw(3, 0, sp); if (mp_obj_is_true(ret_value)) { // We need to silence/swallow the exception. This is done // by popping the exception and the __exit__ handler and // replacing it with None, which signals END_FINALLY to just // execute the finally handler normally. - sp -= 4; SET_TOP(mp_const_none); assert(exc_sp >= exc_stack); POP_EXC_BLOCK(); } else { // We need to re-raise the exception. We pop __exit__ handler - // and copy the 3 exception values down (remembering that they - // are reversed due to above code). - sp[-4] = sp[0]; - sp[-3] = sp[-1]; - sp -= 2; + // by copying the exception instance down to the new top-of-stack. + sp[0] = sp[3]; } } DISPATCH(); @@ -698,18 +695,12 @@ unwind_jump:; ENTRY(MP_BC_END_FINALLY): MARK_EXC_IP_SELECTIVE(); - // not fully implemented - // if TOS is an exception, reraises the exception (3 values on TOS) // if TOS is None, just pops it and continues - // if TOS is an integer, does something else - // else error - if (mp_obj_is_exception_type(TOP())) { - RAISE(sp[-1]); - } + // if TOS is an integer, finishes coroutine and returns control to caller + // if TOS is an exception, reraises the exception if (TOP() == mp_const_none) { sp--; - } else { - assert(MP_OBJ_IS_SMALL_INT(TOP())); + } else if (MP_OBJ_IS_SMALL_INT(TOP())) { // We finished "finally" coroutine and now dispatch back // to our caller, based on TOS value mp_unwind_reason_t reason = MP_OBJ_SMALL_INT_VALUE(POP()); @@ -719,6 +710,9 @@ unwind_jump:; assert(reason == UNWIND_JUMP); goto unwind_jump; } + } else { + assert(mp_obj_is_exception_instance(TOP())); + RAISE(TOP()); } DISPATCH(); @@ -751,14 +745,9 @@ unwind_jump:; // matched against: SETUP_EXCEPT ENTRY(MP_BC_POP_EXCEPT): - // TODO need to work out how blocks work etc - // pops block, checks it's an exception block, and restores the stack, saving the 3 exception values to local threadstate assert(exc_sp >= exc_stack); assert(currently_in_except_block); - //sp = (mp_obj_t*)(*exc_sp--); - //exc_sp--; // discard ip POP_EXC_BLOCK(); - //sp -= 3; // pop 3 exception values DISPATCH(); ENTRY(MP_BC_BUILD_TUPLE): { @@ -1359,10 +1348,8 @@ unwind_loop: mp_obj_t *sp = MP_TAGPTR_PTR(exc_sp->val_sp); // save this exception in the stack so it can be used in a reraise, if needed exc_sp->prev_exc = nlr.ret_val; - // push(traceback, exc-val, exc-type) - PUSH(mp_const_none); + // push exception object so it can be handled by bytecode PUSH(MP_OBJ_FROM_PTR(nlr.ret_val)); - PUSH(MP_OBJ_FROM_PTR(((mp_obj_base_t*)nlr.ret_val)->type)); code_state->sp = sp; #if MICROPY_STACKLESS