# HG changeset patch # User Petter T. # Date 1691680039 -7200 # Thu Aug 10 17:07:19 2023 +0200 # Node ID 7d924d061e9df5bda12933bc8864849550a147cc # Parent c52ae0cfd9fcf001a941c1d247748a140a58c0ca VM Support anonymous functions New opcodes to support dynamic nargout and ignored outputs for anonymous functions. * libinterp/corefcn/compile.cc: Print anonymous function's bytecode * libinterp/corefcn/stack-frame.cc: Correct ip range for argname lookup * libinterp/octave-value/ov-fcn-handle.cc: Anon. fn. use VM * libinterp/parse-tree/pt-bytecode-vm-internal.h: * libinterp/parse-tree/pt-bytecode-vm.cc: New opcodes, refactor ignore data class * libinterp/parse-tree/pt-bytecode-vm.h: * libinterp/parse-tree/pt-bytecode-walk.cc: Compile anonymous functions. * libinterp/parse-tree/pt-bytecode-walk.h: New opcodes, flags * libinterp/parse-tree/pt-eval.cc: Set ignore outputs when calling into VM * test/compile/bytecode.tst: Update tests * test/compile/bytecode_anon_handles.m diff -r c52ae0cfd9fc -r 7d924d061e9d libinterp/corefcn/compile.cc --- a/libinterp/corefcn/compile.cc Mon Aug 07 10:10:49 2023 +0200 +++ b/libinterp/corefcn/compile.cc Thu Aug 10 17:07:19 2023 +0200 @@ -274,24 +274,32 @@ if (nargin != 1) print_usage (); - std::string fn_name = args(0).string_value (); - symbol_table& symtab = interp.get_symbol_table (); + octave_value ov; - octave_value ov = symtab.find_function (fn_name); + if (args (0).is_string ()) + { + std::string fn_name = args(0).string_value (); + symbol_table& symtab = interp.get_symbol_table (); - if (!ov.is_defined ()) - { - error ("Function not defined: %s", fn_name.c_str ()); + ov = symtab.find_function (fn_name); + + if (!ov.is_defined ()) + { + error ("Function not defined: %s", fn_name.c_str ()); + } } + else + ov = args (0); octave_user_function *ufn = ov.user_function_value (); + std::string fn_name = ufn->name (); if (!ufn || (!ufn->is_user_function () && !ufn->is_user_script ())) { error ("Function not a user function or script: %s", fn_name.c_str ()); } - if (!ufn->is_compiled () && V__enable_vm_eval__) + if (!ufn->is_compiled () && V__enable_vm_eval__ && !ov.is_anonymous_function ()) compile_user_function (*ufn, 0); else if (!ufn->is_compiled ()) error ("Function not compiled: %s", fn_name.c_str ()); diff -r c52ae0cfd9fc -r 7d924d061e9d libinterp/corefcn/stack-frame.cc --- a/libinterp/corefcn/stack-frame.cc Mon Aug 07 10:10:49 2023 +0200 +++ b/libinterp/corefcn/stack-frame.cc Thu Aug 10 17:07:19 2023 +0200 @@ -514,7 +514,7 @@ int start = entries[i].m_ip_start; int end = entries[i].m_ip_end; - if (start > m_ip || end < m_ip) + if (start > (m_ip - 1) || end < (m_ip - 1)) continue; if (best_match != -1) diff -r c52ae0cfd9fc -r 7d924d061e9d libinterp/octave-value/ov-fcn-handle.cc --- a/libinterp/octave-value/ov-fcn-handle.cc Mon Aug 07 10:10:49 2023 +0200 +++ b/libinterp/octave-value/ov-fcn-handle.cc Thu Aug 10 17:07:19 2023 +0200 @@ -64,6 +64,7 @@ #include "pt-misc.h" #include "pt-pr-code.h" #include "pt-stmt.h" +#include "pt-bytecode-walk.h" #include "stack-frame.h" #include "syminfo.h" #include "symscope.h" @@ -754,6 +755,9 @@ return m_stack_context; } + // Compile the underlying function to bytecode for the VM. + void compile (); + protected: // Pointer to closure stack frames. @@ -2931,17 +2935,54 @@ octave_value_list anonymous_fcn_handle::call (int nargout, const octave_value_list& args) { + bool is_compiled = false; + tree_evaluator& tw = __get_evaluator__ (); octave_user_function *oct_usr_fcn = m_fcn.user_function_value (); - tw.push_stack_frame (oct_usr_fcn, m_local_vars, m_stack_context); + if (oct_usr_fcn) + { + is_compiled = oct_usr_fcn->is_compiled (); + if (octave::V__enable_vm_eval__ && !is_compiled && !oct_usr_fcn->m_compilation_failed) + { + try + { + octave::compile_anon_user_function (*oct_usr_fcn, false, m_local_vars); + is_compiled = true; + } + catch (std::exception &e) + { + warning ("Auto-compilation of anonymous function failed with message %s", e.what ()); + oct_usr_fcn->m_compilation_failed = true; + } + } + } + + // Bytecode functions push their own stack frames in the vm + if (!is_compiled) + tw.push_stack_frame (oct_usr_fcn, m_local_vars, m_stack_context); unwind_action act ([&tw] () { tw.pop_stack_frame (); }); return oct_usr_fcn->execute (tw, nargout, args); } +void anonymous_fcn_handle::compile () +{ + octave_user_code *usr_code = user_function_value (); + + try + { + compile_anon_user_function (*usr_code, false, m_local_vars); + } + catch (std::exception &e) + { + warning ("Auto-compilation of anonymous function failed with message %s", e.what ()); + usr_code->m_compilation_failed = true; + } +} + octave_value anonymous_fcn_handle::workspace () const { octave_scalar_map local_vars_map; diff -r c52ae0cfd9fc -r 7d924d061e9d libinterp/octave-value/ov-fcn-handle.h --- a/libinterp/octave-value/ov-fcn-handle.h Mon Aug 07 10:10:49 2023 +0200 +++ b/libinterp/octave-value/ov-fcn-handle.h Thu Aug 10 17:07:19 2023 +0200 @@ -159,6 +159,8 @@ virtual bool has_function_cache (void) const { return false; } + virtual void compile () {} + protected: void warn_load (const char *file_type) const; @@ -375,6 +377,7 @@ get_cached_fcn (const octave_value_list& args) { return m_rep->get_cached_fcn (args); } bool has_function_cache (void) const { return m_rep->has_function_cache (); } + void compile () { m_rep->compile (); } private: std::shared_ptr m_rep; diff -r c52ae0cfd9fc -r 7d924d061e9d libinterp/parse-tree/pt-bytecode-vm-internal.h --- a/libinterp/parse-tree/pt-bytecode-vm-internal.h Mon Aug 07 10:10:49 2023 +0200 +++ b/libinterp/parse-tree/pt-bytecode-vm-internal.h Thu Aug 10 17:07:19 2023 +0200 @@ -313,7 +313,13 @@ /* Set the ip to 0 */ \ ip = code; \ int n_returns_callee = static_cast (*ip++); /* Negative for varargout */ \ -n_returns_callee = n_returns_callee >= 0 ? n_returns_callee : -n_returns_callee; \ +if (OCTAVE_UNLIKELY (n_returns_callee < 0)) \ + { \ + if (n_returns_callee == -128) /* Anonymous function */ \ + n_returns_callee = 1; \ + else \ + n_returns_callee = -n_returns_callee; \ + } \ int n_args_callee = static_cast (*ip++); /* Negative for varargin */ \ int n_locals_callee = POP_CODE_USHORT (); \ \ @@ -376,7 +382,6 @@ for (int i = 0; i < n_locals_to_ctor; i++) \ PUSH_OV (); \ \ - \ try \ { \ m_tw->push_stack_frame(*this, usr_fcn, nargout, n_args_on_callee_stack); \ @@ -384,18 +389,14 @@ CATCH_STACKPUSH_EXECUTION_EXCEPTION /* Sets m_could_not_push_frame to true */ \ CATCH_STACKPUSH_BAD_ALLOC \ \ +if (OCTAVE_UNLIKELY (m_output_ignore_data)) \ + { \ + /* Called fn needs to know about ignored outputs .e.g. [~, a] = foo() */ \ + m_output_ignore_data->push_frame (*this); \ + } \ + \ /* "auto var" in the frame object. This is needed if nargout() etc are called */ \ set_nargout (nargout); \ -/* Called fn needs to know about ignored outputs .e.g. [~, a] = foo() */ \ -if (m_output_ignore_data) \ - { \ - if (m_output_ignore_data->is_pending ()) \ - m_tw->set_auto_fcn_var (stack_frame::IGNORED, m_output_ignore_data->get_ignore_matrix ()); \ - else \ - m_tw->set_auto_fcn_var (stack_frame::IGNORED, {}); \ - m_output_ignore_data->m_v_lvalue_list.push_back (m_tw->lvalue_list ()); \ - m_tw->set_lvalue_list (nullptr); \ - } \ \ if (n_args_on_callee_stack > n_args_callee) \ { \ diff -r c52ae0cfd9fc -r 7d924d061e9d libinterp/parse-tree/pt-bytecode-vm.cc --- a/libinterp/parse-tree/pt-bytecode-vm.cc Mon Aug 07 10:10:49 2023 +0200 +++ b/libinterp/parse-tree/pt-bytecode-vm.cc Thu Aug 10 17:07:19 2023 +0200 @@ -33,6 +33,7 @@ #include "pt-bytecode-vm.h" #include "pt-bytecode-vm-internal.h" +#include "pt-bytecode-walk.h" #include "ov.h" #include "error.h" #include "symtab.h" @@ -47,6 +48,7 @@ #include "ov-inline.h" #include "ov-vm.h" +#include "ov-fcn-handle.h" //#pragma GCC optimize("O0") @@ -191,7 +193,8 @@ { switch (static_cast (*p)) { - + PRINT_OP (ANON_MAYBE_SET_IGNORE_OUTPUTS) + PRINT_OP (EXT_NARGOUT) PRINT_OP (POP) PRINT_OP (DUP) PRINT_OP (MUL) @@ -203,6 +206,7 @@ PRINT_OP (DIV) PRINT_OP (DIV_DBL) PRINT_OP (RET) + PRINT_OP (RET_ANON) PRINT_OP (LE) PRINT_OP (LE_DBL) PRINT_OP (LE_EQ) @@ -310,11 +314,13 @@ CASE_START (INDEX_ID_NARGOUT0) PSLOT () PCHAR () CASE_END () CASE_START (INDEX_ID_NARGOUT1) PSLOT () PCHAR () CASE_END () + CASE_START (INDEX_IDNX) PSLOT () PCHAR () CASE_END () CASE_START (INDEX_ID1_MAT_2D) PSLOT () PCHAR () CASE_END () CASE_START (INDEX_ID1_MAT_1D) PSLOT () PCHAR () CASE_END () CASE_START (INDEX_CELL_ID_NARGOUT0) PSLOT () PCHAR () CASE_END () CASE_START (INDEX_CELL_ID_NARGOUT1) PSLOT () PCHAR () CASE_END () + CASE_START (INDEX_CELL_IDNX) PSLOT () PCHAR () CASE_END () CASE_START (INDEX_CELL_ID_NARGOUTN) PSLOT () PCHAR () PCHAR () CASE_END () CASE_START (INDEX_IDN) PSLOT () PCHAR () PCHAR () CASE_END () @@ -334,6 +340,7 @@ CASE_START (INDEX_STRUCT_NARGOUTN) PCHAR () PWSLOT () PWSLOT () CASE_END () CASE_START (END_ID) PSLOT () PCHAR () PCHAR () CASE_END () + CASE_START (PUSH_SLOT_NX) PSLOT () PCHAR () CASE_END () CASE_START (PUSH_SLOT_NARGOUTN) PSLOT () PCHAR () CASE_END () CASE_START (BRAINDEAD_WARNING) PSLOT () PCHAR () CASE_END () CASE_START (SUBASSIGN_STRUCT) PSLOT () PWSLOT () CASE_END () @@ -352,6 +359,7 @@ PCHAR () PCHAR () PCHAR () + PCHAR () int nn = *p; s += " {"; for (int i = 0; i < nn; i++) @@ -368,6 +376,7 @@ CASE_START (END_OBJ) PSLOT () PCHAR () PCHAR () CASE_END () + CASE_START (WORDCMD_NX) PSLOT () PCHAR () CASE_END () CASE_START (WORDCMD) PSLOT () PCHAR () PCHAR () CASE_END () CASE_START (SET_IGNORE_OUTPUTS) @@ -842,6 +851,13 @@ &&subassign_id_mat_2d, &&enter_script_frame, &&exit_script_frame, + &&ret_anon, + &&index_idnx, + &&index_cell_idnx, + &&push_slot_nx, + &&ext_nargout, + &&wordcmd_nx, + &&anon_maybe_set_ignore_output, }; if (OCTAVE_UNLIKELY (m_profiler_enabled)) @@ -902,11 +918,16 @@ if (is_varargin) n_args = -n_args; - if (n_returns < 0) // Negative for varargout - n_returns = -n_returns; + if (OCTAVE_UNLIKELY (n_returns < 0)) // Negative for varargout and anonymous functions + { + if (n_returns != -128) + n_returns = -n_returns; + else + n_returns = 1; + } // The first return is always nargout, as a uint64 - (*sp++).u = 0; //TODO: nargout as arg to this func + (*sp++).u = root_nargout; // Construct nil octave_values for the return slots for (int i = 1; i < n_returns; i++) @@ -1033,6 +1054,15 @@ DISPATCH_1BYTEOP(); ret: { + // If we have any active ~/"black hole", e.g. [~] = foo() in the stack + // the m_output_ignore_data pointer is live. We need to pop and reset + // lvalue lists for the tree walker. + if (OCTAVE_UNLIKELY (m_output_ignore_data)) + { + m_output_ignore_data->pop_frame (*this); + output_ignore_data::maybe_delete_ignore_data (*this, 0); + } + // We need to tell the bytecode frame we are unwinding so that it can save // variables on the VM stack if it is referenced from somewhere else. m_tw->get_current_stack_frame ()->vm_unwinds (); @@ -1043,10 +1073,10 @@ int n_returns_callee = N_RETURNS (); bool is_varargout = n_returns_callee < 0; - if (n_returns_callee < 0) + if (OCTAVE_UNLIKELY (is_varargout)) n_returns_callee = -n_returns_callee; - assert (n_returns_callee > 0); + int n_locals_callee = N_LOCALS (); // Destroy locals @@ -1144,18 +1174,6 @@ // // Essentially do the same thing as in the call but in reverse order. - // If we have any active ~/"black hole", e.g. [~] = foo() in the stack - // the m_output_ignore_data pointer is live. We need to pop and reset - // lvalue lists for the tree walker. - if (m_output_ignore_data) - { - delete m_tw->lvalue_list (); - CHECK (!m_output_ignore_data->m_v_lvalue_list.empty ()); - - m_tw->set_lvalue_list (m_output_ignore_data->m_v_lvalue_list.back ()); - m_output_ignore_data->m_v_lvalue_list.pop_back (); - } - // The sp now points one past the last return value stack_element *caller_stack_end = sp - n_returns_callee; sp = caller_stack_end; // sp points to one past caller stack @@ -1533,6 +1551,7 @@ push_slot_nargout0: push_slot_nargout1: push_slot1_special: +push_slot_nx: { int slot = arg0; @@ -1709,6 +1728,10 @@ nargout = 0; ip += 2; // Skip the maybe command slot } + else if (opcode == INSTR::PUSH_SLOT_NX) + { + nargout = bsp[0].i; + } else PANIC ("Invalid opcode"); @@ -2013,6 +2036,13 @@ bool specialization_ok; if (0) { +index_idnx: + slot = arg0; + nargout = bsp[0].i; + specialization_ok = false; + } + else if (0) + { index_idn: slot = arg0; // Needed if we need a function lookup nargout = *ip++; @@ -2910,9 +2940,15 @@ { ignored = tmp.matrix_value (); - if (slot < N_RETURNS ()) + int n_returns = N_RETURNS (); + if (n_returns == -128) + n_returns = 1; + else if (n_returns < 0) + n_returns = -n_returns; + + if (slot < n_returns) { - int outputnum = N_RETURNS () - 1 - slot; + int outputnum = n_returns - 1 - slot; octave_idx_type idx = ignored.lookup (outputnum); is_ignored = idx > 0 && ignored (idx - 1) == outputnum; @@ -2956,9 +2992,15 @@ { ignored = tmp.matrix_value (); - if (slot < N_RETURNS ()) + int n_returns = N_RETURNS (); + if (n_returns == -128) + n_returns = 1; + else if (n_returns < 0) + n_returns = -n_returns; + + if (slot < n_returns) { - int outputnum = N_RETURNS () - 1 - slot; + int outputnum = n_returns - 1 - slot; octave_idx_type idx = ignored.lookup (outputnum); is_ignored = idx > 0 && ignored (idx - 1) == outputnum; @@ -3499,9 +3541,12 @@ auto it = unwind_data->m_ip_to_tree.find (tree_idx); CHECK (it != unwind_data->m_ip_to_tree.end ()); - tree_anon_fcn_handle *h = reinterpret_cast (it->second); - - octave_value ret = m_tw->evaluate_anon_fcn_handle (*h); + tree_anon_fcn_handle *tree_h = reinterpret_cast (it->second); + + octave_value ret = m_tw->evaluate_anon_fcn_handle (*tree_h); + octave_fcn_handle *fn_h = ret.fcn_handle_value (); + CHECK (fn_h); + fn_h->compile (); PUSH_OV (ret); } @@ -3729,11 +3774,27 @@ herm_ldiv: MAKE_BINOP(compound_binary_op::op_herm_ldiv) DISPATCH_1BYTEOP(); -wordcmd: + { - int slot = arg0; // Needed if we need a function lookup - int nargout = *ip++; - int n_args_on_stack = *ip++; + int slot; // Needed if we need a function lookup + int nargout; + int n_args_on_stack; + + if (0) + { +wordcmd_nx: + slot = arg0; + nargout = bsp[0].i; + n_args_on_stack = *ip++; + } + else if (0) + { +wordcmd: + slot = arg0; + nargout = *ip++; + n_args_on_stack = *ip++; + } + // The object to index is before the args on the stack octave_value &ov = (sp[-1 - n_args_on_stack]).ov; @@ -3961,6 +4022,12 @@ int nargout, slot; if (0) { +index_cell_idnx: + slot = arg0; // Needed if we need a function lookup + nargout = bsp[0].i; + } + else if (0) + { index_cell_idn: slot = arg0; // Needed if we need a function lookup nargout = *ip++; @@ -4185,8 +4252,13 @@ octave_user_function *usr_fcn = static_cast (sp[0].pv); int n_returns_callee = static_cast (ip[-4]); - if (n_returns_callee < 0) - n_returns_callee = -n_returns_callee; + if (OCTAVE_UNLIKELY (n_returns_callee < 0)) + { + if (n_returns_callee == -128) /* Anonymous function */ + n_returns_callee = 1; + else + n_returns_callee = -n_returns_callee; + } int n_args_callee = -static_cast (ip[-3]); // Note: Minus int n_locals_callee = ip[-2] | (ip[-1] << 8); @@ -4299,23 +4371,21 @@ PUSH_OV (); int nargin = n_args_on_callee_stack + idx_cell; // n_args_callee count includes varargin - try - { - m_tw->push_stack_frame(*this, usr_fcn, nargout, nargin); - } - CATCH_STACKPUSH_EXECUTION_EXCEPTION // Sets m_could_not_push_frame to true - CATCH_STACKPUSH_BAD_ALLOC - + +try + { + m_tw->push_stack_frame(*this, usr_fcn, nargout, n_args_on_callee_stack); + } +CATCH_STACKPUSH_EXECUTION_EXCEPTION /* Sets m_could_not_push_frame to true */ +CATCH_STACKPUSH_BAD_ALLOC + + m_tw->set_nargin (nargin); + +if (OCTAVE_UNLIKELY (m_output_ignore_data)) + { /* Called fn needs to know about ignored outputs .e.g. [~, a] = foo() */ - if (m_output_ignore_data) - { - if (m_output_ignore_data->is_pending ()) - m_tw->set_auto_fcn_var (stack_frame::IGNORED, m_output_ignore_data->get_ignore_matrix ()); - else - m_tw->set_auto_fcn_var (stack_frame::IGNORED, {}); - m_output_ignore_data->m_v_lvalue_list.push_back (m_tw->lvalue_list ()); - m_tw->set_lvalue_list (nullptr); - } +m_output_ignore_data->push_frame (*this); + } /* N_RETURNS is negative for varargout */ int n_returns = N_RETURNS () - 1; /* %nargout in N_RETURNS */ @@ -4491,14 +4561,19 @@ // ~ we need to restore stuff. if (m_output_ignore_data) { - delete m_tw->lvalue_list (); - CHECK (!m_output_ignore_data->m_v_lvalue_list.empty ()); - - m_tw->set_lvalue_list (m_output_ignore_data->m_v_lvalue_list.back ()); - m_output_ignore_data->m_v_lvalue_list.pop_back (); + m_output_ignore_data->pop_frame (*this); + output_ignore_data::maybe_delete_ignore_data (*this, 0); } } + if (m_output_ignore_data) + { + CHECK_PANIC (m_output_ignore_data->m_external_root_ignorer); + output_ignore_data::maybe_delete_ignore_data (*this, 1); + } + + CHECK_PANIC (!m_output_ignore_data); + CHECK_STACK (0); this->m_dbg_proper_return = true; @@ -4869,9 +4944,9 @@ index_struct_call: { - int slot = arg0; + int nargout = arg0; bool has_slot = *ip++; - int nargout = *ip++; + int slot = POP_CODE_USHORT (); int n_subs = POP_CODE (); @@ -5463,6 +5538,17 @@ DISPATCH(); } +anon_maybe_set_ignore_output: + { + if (m_output_ignore_data) + { + // We want to propagate the caller's ignore matrix. + octave_value current_ignore_matrix = m_tw->get_auto_fcn_var (stack_frame::auto_var_type::IGNORED); + m_output_ignore_data->set_ignore_anon (*this, current_ignore_matrix); + } + } + DISPATCH_1BYTEOP (); + set_ignore_outputs: { if (!m_output_ignore_data) @@ -5473,7 +5559,6 @@ int n_ignored = arg0; int n_total = POP_CODE (); auto *M = new Matrix {}; - m_output_ignore_data->m_v_matrixes.push_back (M); M->resize (1, n_ignored); std::set set_ignored; @@ -5485,45 +5570,29 @@ set_ignored.insert (ignore_idx); } + octave_value ignore_matrix {*M}; + // For calls into m-functions etc - auto *active_lvalue_list = new std::list {}; - - auto *saved_lvalue_list = m_tw->lvalue_list (); - m_output_ignore_data->m_v_lvalue_list.push_back (saved_lvalue_list); + auto *new_lvalue_list = new std::list {}; for (int i = 0; i < n_total; i++) { octave_lvalue lval ({}, m_tw->get_current_stack_frame ()); if (set_ignored.find (i + 1) != set_ignored.end ()) lval.mark_black_hole (); - active_lvalue_list->push_back (lval); - } - - m_tw->set_lvalue_list (active_lvalue_list); + new_lvalue_list->push_back (lval); + } + + m_output_ignore_data->set_ignore (*this, ignore_matrix, new_lvalue_list); } DISPATCH(); clear_ignore_outputs: { - CHECK (m_output_ignore_data); - - auto *active_lvalue_list = m_tw->lvalue_list (); - delete active_lvalue_list; - - // Restore the evaluators lvalue list - m_tw->set_lvalue_list (m_output_ignore_data->pop_lvalue_list ()); - - // Delete the matrix we used to set the autovar (if it has not been used an nulled) - delete m_output_ignore_data->m_v_matrixes.back (); - m_output_ignore_data->m_v_matrixes.pop_back (); - - // We want to null m_output_ignore_data if it is empty - if (m_output_ignore_data->m_v_matrixes.empty ()) - { - CHECK (m_output_ignore_data->m_v_lvalue_list.empty ()); - delete m_output_ignore_data; - m_output_ignore_data = nullptr; - } + if (m_output_ignore_data) + m_output_ignore_data->clear_ignore (*this); + + output_ignore_data::maybe_delete_ignore_data (*this, 1); // Clear any value written to the %~X slot(s) int n_slots = arg0; @@ -5656,6 +5725,198 @@ DISPATCH (); } + +ret_anon: + { + // We need to tell the bytecode frame we are unwinding so that it can save + // variables on the VM stack if it is referenced from somewhere else. + m_tw->get_current_stack_frame ()->vm_unwinds (); + + assert (N_RETURNS () == -128); + + int n_returns_callee = bsp[0].i + 1; // Nargout on stack, +1 to include %nargout + if (n_returns_callee == 1) + n_returns_callee = 2; + + int n_locals_callee = N_LOCALS (); // Amount of arguments, purely local variables and %nargout + + // Assert that the stack pointer is back where it should be + assert (bsp + n_locals_callee + n_returns_callee - 1 == sp); + + stack_element *first_ret = sp - n_returns_callee + 1; + + // Destroy locals + // + // Note that we destroy from the bottom towards + // the top of the stack to calls ctors in the same + // order as the treewalker. + + stack_element *first_pure_local = bsp + 1; + while (first_pure_local != first_ret) + { + (*first_pure_local++).ov.~octave_value (); + } + + if (OCTAVE_UNLIKELY (m_profiler_enabled)) + { + auto p = vm::m_vm_profiler; + if (p) + { + std::string fn_name = data[2].string_value (); // profiler_name () querried at compile time + p->exit_fn (fn_name); + } + } + + // If we have any active ~/"black hole", e.g. [~] = foo() in the stack + // the m_output_ignore_data pointer is live. We need to pop and reset + // lvalue lists for the tree walker. + if (OCTAVE_UNLIKELY (m_output_ignore_data)) + { + m_output_ignore_data->pop_frame (*this); + output_ignore_data::maybe_delete_ignore_data (*this, 0); + } + + // Are we at the root routine? + if (bsp == rsp) + { + CHECK (!m_output_ignore_data); // Should not be active + + // Collect return values in octave_value_list. + // Skip %nargout, the first value, which is an integer. + // n_returns_callee includes %nargout, but root_nargout doesn't. + octave_value_list ret; + + int j; + // nargout 0 should still give one return value, if there is one + int n_root_wanted = std::max (root_nargout, 1); + for (j = 1; j < n_returns_callee && j < (n_root_wanted + 1); j++) + { + int idx = (n_returns_callee - 1) - j; + ret.append (std::move (first_ret[idx].ov)); + first_ret[idx].ov.~octave_value (); + } + // Destroy rest of return values, if any + for (; j < n_returns_callee; j++) + { + int idx = (n_returns_callee - 1) - j; + first_ret[idx].ov.~octave_value (); + } + + //Note: Stack frame object popped by caller + CHECK_STACK (0); + this->m_dbg_proper_return = true; + + m_tw->set_lvalue_list (m_original_lvalue_list); + return ret; + } + + // If the root stack pointer is not the same as the base pointer, + // we are returning from a bytecode routine to another bytecode routine, + // so we have to restore the caller stack frame and cleanup the callee's. + // + // Essentially do the same thing as in the call but in reverse order. + + // Point sp one past the last return value + stack_element *caller_stack_end = bsp; + sp = caller_stack_end; // sp points to one past caller stack + + int callee_nargout = caller_stack_end[0].i; + //TODO: Check nargout + + // Restore ip + ip = (*--sp).puc; + + // Restore bsp + bsp = (*--sp).pse; + + // Restore id names + name_data = (*--sp).ps; + + // Restore data + data = (*--sp).pov; + + // Restore code + code = (*--sp).puc; + + // Restore unwind data + unwind_data = (*--sp).pud; + + // Restore the stack pointer. The stored address is the first arg + // on the caller stack, or where it would have been if there are no args. + // The args were moved to the callee stack and destroyed on the caller + // stack in the call. + sp = sp[-1].pse; + + // We now have the object that was called on the stack, destroy it + STACK_DESTROY (1); + + // Move the callee's return values to the top of the stack of the caller. + // Renaming variables to keep my sanity. + int n_args_caller_expects = callee_nargout; + int n_args_callee_has = n_returns_callee - 1; // Excludes %nargout + int n_args_to_move = std::min (n_args_caller_expects, n_args_callee_has); + int n_args_actually_moved = 0; + + // If no return values is requested but there exists return values, + // we need to push one to be able to write it to ans. + if (n_args_caller_expects == 0 && n_args_callee_has) + { + n_args_actually_moved++; + PUSH_OV (std::move (first_ret[0].ov)); + } + // If the callee aint returning anything, we need to push a + // nil object, since the caller always anticipates atleast + // one object, even for nargout == 0. + else if (n_args_caller_expects == 0 && !n_args_callee_has) + PUSH_OV(); + // If the stacks will overlap due to many returns, do copy via container + else if (sp + n_args_caller_expects >= caller_stack_end) + { + // This pushes 'n_args_to_move' number of return values and 'n_args_caller_expects - n_args_to_move' + // number of nils. + copy_many_args_to_caller (sp, first_ret, n_args_to_move, n_args_caller_expects); + n_args_actually_moved = n_args_caller_expects; + sp += n_args_actually_moved; + } + // Move 'n_args_to_move' return value from callee to caller + else + { + // If the caller wants '[a, b, ~]' and the callee has 'd e' + // we need to push 'nil' 'd' 'e' + for (int i = n_args_to_move; i < n_args_caller_expects; i++) + PUSH_OV (); + for (int i = 0; i < n_args_to_move; i++) + { + // Move into caller stack. Note that the order is reversed, such that + // a b c on the callee stack becomes c b a on the caller stack. + octave_value &arg = first_ret[i].ov; + + PUSH_OV (std::move (arg)); + } + n_args_actually_moved = n_args_caller_expects; + } + + // Destroy the unused return values on the callee stack + for (int i = 0; i < n_args_callee_has; i++) + { + int idx = n_args_callee_has - 1 - i; + first_ret[idx].ov.~octave_value (); // Destroy ov in callee + } + + // Pop the current dynamic stack frame + std::shared_ptr fp = m_tw->pop_return_stack_frame (); + // If the pointer is not shared, stash it in a cache which is used + // to avoid having to allocate shared pointers each frame push. + if (fp.unique () && m_frame_ptr_cache.size () < 8) + { + fp->vm_clear_for_cache (); + m_frame_ptr_cache.push_back (std::move (fp)); + } + + // Continue execution back in the caller + } + DISPATCH (); + /* Check whether we should enter the debugger on the next ip */ { bool onebyte_op; @@ -5767,7 +6028,7 @@ if (it == unwind_data->m_ip_to_tree.end ()) goto debug_check_end; - bool is_ret = *ip == static_cast (INSTR::RET); + bool is_ret = *ip == static_cast (INSTR::RET) || *ip == static_cast (INSTR::RET_ANON); m_sp = sp; m_bsp = bsp; @@ -5868,6 +6129,25 @@ ip += 2; // Forward ip so it points to after the widened argument goto *instr [opcode]; } + + ext_nargout: + { + // This opcode replaces the first opcode argument of the next opcode, with the + // current function's nargout. + // + // Anonymous functions need to have a dynamic "expression nargout" on the + // root expression since the "expression nargout" is decided by the caller. + // E.g. '[a b] = anon ()' yields 2 for the root expression in 'anon'. + // + // In a ordinary function "expression nargout" is decided by the source row. + + int opcode = arg0; // The opcode to execute next is in arg0, i.e. ip[-1] + // The next opcode needs its arg0, which is supposed to be a nargout value + arg0 = bsp[0].i; // %nargout is stored in the first slot in the stack frame + ip++; // Forward ip so it points to after the nargout argument in the next opcode + goto *instr [opcode]; // Execute the next opcode + } + enter_script_frame: { auto fp = m_tw->get_current_stack_frame (); @@ -6273,6 +6553,14 @@ m_tw->set_nargin (nargin); } +void vm::caller_ignores_output () +{ + m_output_ignore_data = new output_ignore_data; + m_output_ignore_data->m_v_lvalue_list.back () = m_tw->lvalue_list (); + m_output_ignore_data->m_v_owns_lvalue_list.back () = false; + m_output_ignore_data->m_external_root_ignorer = true; +} + void vm::set_nargout (int nargout) { m_tw->set_nargout (nargout); @@ -6915,6 +7203,94 @@ return; } +void +vm::output_ignore_data::push_frame (vm &vm) +{ + vm.m_tw->set_auto_fcn_var (stack_frame::IGNORED, m_ov_pending_ignore_matrix); + m_ov_pending_ignore_matrix = {}; // Clear ignore matrix so that the next call wont ignore anything + m_v_lvalue_list.push_back (vm.m_tw->lvalue_list ()); // Will be restored in output_ignore_data::pop_frame () + m_v_owns_lvalue_list.push_back (false); // Caller owns the current lvalue + + vm.m_tw->set_lvalue_list (nullptr); // There is not lvalue list set for the new frame +} + +void +vm::output_ignore_data::clear_ignore (vm &vm) +{ + CHECK_PANIC (m_v_lvalue_list.size ()); + CHECK_PANIC (m_v_owns_lvalue_list.size ()); + CHECK_PANIC (m_v_owns_lvalue_list.size () == m_v_lvalue_list.size ()); + + // If the output_ignore_data object owns the current lvalue list + // we need to free it. + auto *current_lval_list = vm.m_tw->lvalue_list (); + + bool owns_lval_list = m_v_owns_lvalue_list.back (); + m_v_owns_lvalue_list.back () = false; + + if (owns_lval_list) + delete current_lval_list; + + // Restore the prior lvalue list in the tree walker + vm.m_tw->set_lvalue_list (m_v_lvalue_list.back ()); + m_v_lvalue_list.back () = nullptr; + + m_ov_pending_ignore_matrix = {}; +} + +void +vm::output_ignore_data::pop_frame (vm &vm) +{ + CHECK_PANIC (m_v_lvalue_list.size ()); + CHECK_PANIC (m_v_owns_lvalue_list.size ()); + CHECK_PANIC (m_v_owns_lvalue_list.size () == m_v_lvalue_list.size ()); + + // If the output_ignore_data object owns the current lvalue list + // we need to free it. + auto *current_lval_list = vm.m_tw->lvalue_list (); + + bool owns_lval_list = m_v_owns_lvalue_list.back (); + m_v_owns_lvalue_list.pop_back (); + + if (owns_lval_list) + delete current_lval_list; + + // Restore the prior lvalue list in the tree walker + vm.m_tw->set_lvalue_list (m_v_lvalue_list.back ()); + m_v_lvalue_list.pop_back (); +} + +void +vm::output_ignore_data::set_ignore_anon (vm &vm, octave_value ignore_matrix) +{ + CHECK_PANIC (m_ov_pending_ignore_matrix.is_nil ()); + CHECK_PANIC (m_v_lvalue_list.size ()); + CHECK_PANIC (m_v_owns_lvalue_list.size ()); + CHECK_PANIC (m_v_owns_lvalue_list.size () == m_v_lvalue_list.size ()); + + // For anonymous functions we propagate the current ignore matrix and lvalue list to the callee. + + m_ov_pending_ignore_matrix = ignore_matrix; + // Since the caller owns the lvalue list, we need to note not to delete the lvalue list when popping + // the callee frame. + vm.m_tw->set_lvalue_list (m_v_lvalue_list.back ()); +} + +void +vm::output_ignore_data::set_ignore (vm &vm, octave_value ignore_matrix, + std::list *new_lval_list) +{ + CHECK_PANIC (m_ov_pending_ignore_matrix.is_nil ()); + CHECK_PANIC (m_v_lvalue_list.size ()); + CHECK_PANIC (m_v_owns_lvalue_list.size ()); + CHECK_PANIC (m_v_owns_lvalue_list.size () == m_v_lvalue_list.size ()); + + m_ov_pending_ignore_matrix = ignore_matrix; + m_v_owns_lvalue_list.back () = true; + m_v_lvalue_list.back () = vm.m_tw->lvalue_list (); + vm.m_tw->set_lvalue_list (new_lval_list); +} + // Debugging functions to be called from gdb void diff -r c52ae0cfd9fc -r 7d924d061e9d libinterp/parse-tree/pt-bytecode-vm.h --- a/libinterp/parse-tree/pt-bytecode-vm.h Mon Aug 07 10:10:49 2023 +0200 +++ b/libinterp/parse-tree/pt-bytecode-vm.h Thu Aug 10 17:07:19 2023 +0200 @@ -370,6 +370,15 @@ #include "pt-bytecode.h" +#if defined(__FILE_NAME__) +#define CHECK_PANIC(cond) \ +do { if (!(cond)) panic ("VM internal error at %s:%d, " #cond, __FILE_NAME__, __LINE__);} while ((0)) +#else +#define CHECK_PANIC(cond) \ +do { if (!(cond)) panic ("VM internal error at %d, " #cond, __LINE__);} while ((0)) +#endif + + OCTAVE_BEGIN_NAMESPACE(octave) class tree_evaluator; @@ -491,19 +500,54 @@ static int constexpr m_matrix_typeid = 4; static int constexpr m_bool_typeid = 10; + // If there are any ignored outputs, e.g. "[x, ~] = foo ()", we need to push a separate + // stack frame with the ignored outputs for isargout () to be able to querry for ignored + // outputs in the callees. + // + // struct output_ignore_data { - std::vector m_v_matrixes; + octave_value m_ov_pending_ignore_matrix; std::vector*> m_v_lvalue_list; + std::vector m_v_owns_lvalue_list; // If true, should call delete on active lvalue list + // A sanity check flag. Set to true if the first ignorer is calling from outside the VM + bool m_external_root_ignorer = false; - bool is_pending () {return m_v_matrixes.size () && m_v_matrixes.back () != nullptr; } + output_ignore_data () + { + m_v_lvalue_list.push_back (nullptr); + m_v_owns_lvalue_list.push_back (false); + } - Matrix get_ignore_matrix () + static void maybe_delete_ignore_data (vm &vm, unsigned target_depth) { - Matrix m = *m_v_matrixes.back (); - delete m_v_matrixes.back (); - m_v_matrixes.back () = nullptr; + if (!vm.m_output_ignore_data) + return; + if (vm.m_output_ignore_data->m_v_owns_lvalue_list.size () > target_depth) + return; + + delete vm.m_output_ignore_data; + vm.m_output_ignore_data = nullptr; + } - return m; + void push_frame (vm &vm); + void pop_frame (vm &vm); + void clear_ignore (vm &vm); + void set_ignore (vm &vm, octave_value ignore_matrix, + std::list *new_lval_list); + + void set_ignore_anon (vm &vm, octave_value ignore_matrix); + + octave_value get_and_null_ignore_matrix () + { + octave_value ret = m_ov_pending_ignore_matrix; + m_ov_pending_ignore_matrix = {}; + + return ret; + } + + octave_value get_ignore_matrix () + { + return m_ov_pending_ignore_matrix; } const std::list* pop_lvalue_list () @@ -556,6 +600,8 @@ void set_nargout (int nargout); + void caller_ignores_output (); + unwind_entry* find_unwind_entry_for_current_state (bool only_find_unwind_protect); int find_unwind_entry_for_forloop (int current_stack_depth); diff -r c52ae0cfd9fc -r 7d924d061e9d libinterp/parse-tree/pt-bytecode-walk.cc --- a/libinterp/parse-tree/pt-bytecode-walk.cc Mon Aug 07 10:10:49 2023 +0200 +++ b/libinterp/parse-tree/pt-bytecode-walk.cc Thu Aug 10 17:07:19 2023 +0200 @@ -37,6 +37,54 @@ using namespace octave; +// Compiles an anonymous function. +// +// The compilation need to happen at runtime since the values of the variables are embedded into the bytecode +// as constants. I.e. when the anonymous function is created by running e.g. "a = @() b;" +void octave::compile_anon_user_function (octave_user_code &ufn, bool do_print, stack_frame::local_vars_map &locals) +{ + try + { + if (!ufn.is_anonymous_function ()) + error ("compile_anon_user_function (): Function is not anonymous"); + + if (ufn.is_classdef_constructor ()) + error ("Classdef constructors are not supported by the VM yet"); // Needs special handling + if (ufn.is_inline_function () || ufn.is_nested_function ()) + error ("Inlined or scoped functions are not supported by the VM yet"); + + // Begin with clearing the old bytecode, if any + ufn.clear_bytecode (); + + bytecode_walker bw; + + // The value of each variable that is taken from the calling scope + bw.m_anon_local_values = &locals; + bw.m_is_anon = true; // Flag used during compilation + + ufn.accept (bw); + + if (do_print) + print_bytecode (bw.m_code); + + ufn.set_bytecode (bw.m_code); + + // Compile the subfunctions + auto subs = ufn.subfunctions (); + for (auto kv : subs) + { + octave_user_function *sub = kv.second.user_function_value (); + compile_user_function (*sub, do_print); + sub->get_bytecode ().m_unwind_data.m_file = ufn.fcn_file_name (); + } + } + catch(...) + { + ufn.clear_bytecode (); + throw; + } +} + void octave::compile_user_function (octave_user_code &ufn, bool do_print) { try @@ -282,6 +330,17 @@ PUSH_CODE (INSTR::WIDE);\ } while ((0)) +// Anonymous functions need dynamic nargout +// The extension opcode EXT_NARGOUT replaces the arg0 +// (the static opcode embedded 'nargout') of the following opcode +// with the %nargout on the stack. +// -1 is the marker value for the need of the dynamic nargout +#define MAYBE_PUSH_ANON_NARGOUT_OPEXT(nargout) \ +do {\ + if ((nargout) == -1)\ + PUSH_CODE (INSTR::EXT_NARGOUT);\ +} while ((0)) + #define PUSH_SLOT(slot) \ do {\ if (slot >= 256)\ @@ -419,7 +478,12 @@ { CHECK_NONNULL (elt); - PUSH_NARGOUT (0); + // For anonymous functions the nargout is dynamic, which is marked + // by setting nargout to -1 + if (m_is_anon) + PUSH_NARGOUT (-1); + else + PUSH_NARGOUT (0); elt->accept (*this); POP_NARGOUT (); @@ -1319,6 +1383,8 @@ POP_NARGOUT (); } + maybe_emit_anon_maybe_ignore_outputs (); + switch (op) { case octave_value::unary_op::op_not: @@ -1344,8 +1410,10 @@ PUSH_TREE_FOR_EVAL (&expr); int tree_idx = -CODE_SIZE (); // NB: Negative to not collide with debug data + int nargout = NARGOUT (); + MAYBE_PUSH_ANON_NARGOUT_OPEXT (nargout); PUSH_CODE (INSTR::EVAL); - PUSH_CODE (NARGOUT ()); + PUSH_CODE (nargout); PUSH_CODE_INT (tree_idx); } else @@ -1364,8 +1432,10 @@ PUSH_TREE_FOR_EVAL (&expr); int tree_idx = -CODE_SIZE (); // NB: Negative to not collide with debug data + int nargout = NARGOUT (); + MAYBE_PUSH_ANON_NARGOUT_OPEXT (nargout); PUSH_CODE (INSTR::EVAL); - PUSH_CODE (NARGOUT ()); + PUSH_CODE (nargout); PUSH_CODE_INT (tree_idx); } else @@ -1446,6 +1516,8 @@ POP_NARGOUT (); } + maybe_emit_anon_maybe_ignore_outputs (); + switch (op) { case octave_value::unary_op::op_not: @@ -1471,8 +1543,10 @@ PUSH_TREE_FOR_EVAL (&expr); int tree_idx = -CODE_SIZE (); // NB: Negative to not collide with debug data + int nargout = NARGOUT (); + MAYBE_PUSH_ANON_NARGOUT_OPEXT (nargout); PUSH_CODE (INSTR::EVAL); - PUSH_CODE (NARGOUT ()); + PUSH_CODE (nargout); PUSH_CODE_INT (tree_idx); } else @@ -1491,8 +1565,10 @@ PUSH_TREE_FOR_EVAL (&expr); int tree_idx = -CODE_SIZE (); // NB: Negative to not collide with debug data + int nargout = NARGOUT (); + MAYBE_PUSH_ANON_NARGOUT_OPEXT (nargout); PUSH_CODE (INSTR::EVAL); - PUSH_CODE (NARGOUT ()); + PUSH_CODE (nargout); PUSH_CODE_INT (tree_idx); } else @@ -1649,6 +1725,8 @@ CHECK_NONNULL (op2); op2->accept (*this); + maybe_emit_anon_maybe_ignore_outputs (); + switch (expr.cop_type ()) { case octave_value::compound_binary_op::op_trans_mul: @@ -1882,6 +1960,8 @@ op2->accept (*this); } + maybe_emit_anon_maybe_ignore_outputs (); + switch (expr.op_type ()) { case octave_value::binary_op::op_mul: @@ -2201,19 +2281,23 @@ } } - // TODO: The return_list is a nullptr for anonymous functions. - if (! returns) - error ("Compiling anonymous functions is not supported by the VM yet"); - // Does the function output varargout? - m_varargout = returns->takes_varargs (); + m_varargout = returns ? returns->takes_varargs () : false; // "varargout" is not in the 'returns' list (if in the proper last position) // so add one to size if 'm_varargout' is true int n_returns = returns ? returns->size () + m_varargout: 0; // The first instruction is the amount of return variables. Negative for varargout. - // +1 for native '%nargout' on the stack - PUSH_CODE (m_varargout ? -(n_returns + 1) : (n_returns + 1)); + // Anonymous functions have no return parameter list specified. For those we set + // the amount of returns to magic number -128. + if (!returns) + { + m_code.m_unwind_data.m_is_anon = true; + CHECK (m_is_anon); // m_is_anon is set by compile_anon_user_function () + PUSH_CODE (-128); + } + else + PUSH_CODE (m_varargout ? -(n_returns + 1) : (n_returns + 1)); // +1 for native '%nargout' on the stack. // Check if the last parameter is "varargin" // If that is the case, we need to mess with the stacks @@ -2242,12 +2326,15 @@ add_id_to_table("%nargout"); // Then the return values - for (auto it = returns->begin (); it != returns->end (); it++) + if (returns) { - std::string name = (*it)->name(); - tree_identifier *id = (*it)->ident (); - CHECK_NONNULL (id); - add_id_to_table (id->name ()); + for (auto it = returns->begin (); it != returns->end (); it++) + { + std::string name = (*it)->name(); + tree_identifier *id = (*it)->ident (); + CHECK_NONNULL (id); + add_id_to_table (id->name ()); + } } if (m_varargout) add_id_to_table ("varargout"); // Not in the returns list. Need to be last @@ -2344,13 +2431,16 @@ } } } - for (auto it = returns->begin (); it != returns->end (); it++) + if (returns) { - std::string name = (*it)->name(); - tree_identifier *id = (*it)->ident (); - int frame_offset = id->symbol ().data_offset (); - int slot = SLOT (name); - m_code.m_unwind_data.m_external_frame_offset_to_internal[frame_offset] = slot; + for (auto it = returns->begin (); it != returns->end (); it++) + { + std::string name = (*it)->name(); + tree_identifier *id = (*it)->ident (); + int frame_offset = id->symbol ().data_offset (); + int slot = SLOT (name); + m_code.m_unwind_data.m_external_frame_offset_to_internal[frame_offset] = slot; + } } // The function name should be in the frame as an id too aswell @@ -2406,6 +2496,32 @@ m_code.m_unwind_data.m_external_frame_offset_to_internal[offset] = SLOT ("ans"); } + // Add code to initialize variables in anonymous functions that took their value from + // the parent scope. + if (m_is_anon) + { + CHECK_NONNULL (m_anon_local_values); + + for (auto kv : *m_anon_local_values) + { + std::string name = kv.first; + CHECK (m_map_locals_to_slot.find (name) != m_map_locals_to_slot.end ()); // Ensure it is in the scope + + int slot = SLOT (name); + + int data_offset = DATA_SIZE (); + octave_value val = kv.second; + PUSH_DATA (val); + + // Push the value to the operand stack + PUSH_CODE_LOAD_CST (data_offset); + // Assign the value on the stack to the slot. + MAYBE_PUSH_WIDE_OPEXT (slot); + PUSH_CODE (INSTR::FORCE_ASSIGN); + PUSH_SLOT (slot); + } + } + // Add code to handle default arguments. If an argument is undefined or // "magic colon" it is to get its default value. if (paras) @@ -2460,6 +2576,10 @@ // Set the amount of locals that has a placeholder since earlier SET_CODE_SHORT (m_offset_n_locals, m_n_locals); + // Anonymous functions has no end marker, so push RET_ANON here + if (m_is_anon) + PUSH_CODE (INSTR::RET_ANON); + // When the last byte of opcode, a 'RET', is to be executed, the VM reads the // next byte of code and puts it in 'arg0'. So, we need to add a dummy // opcode afterwards to prevent out-of-bounds reads. @@ -2555,11 +2675,12 @@ PUSH_TREE_FOR_EVAL (&expr); int tree_idx = -CODE_SIZE (); // NB: Negative to not collide with debug data + MAYBE_PUSH_ANON_NARGOUT_OPEXT (outer_nargout); PUSH_CODE (INSTR::EVAL); PUSH_CODE (outer_nargout); PUSH_CODE_INT (tree_idx); - if (DEPTH () == 1) + if (!m_is_anon && DEPTH () == 1) PUSH_CODE (INSTR::POP); POP_NARGOUT (); @@ -2804,10 +2925,29 @@ void bytecode_walker:: +maybe_emit_anon_maybe_ignore_outputs () +{ + // The ignored output of the caller need to be propagated to the outer nested expression + // for anonymous functions. + // E.g.: + // anon = @() foo (bar (baz ())) + // [x, ~] = anon (); % The foo call will have isargout(2) set to false + if (m_is_anon && DEPTH () == 1) + PUSH_CODE (INSTR::ANON_MAYBE_SET_IGNORE_OUTPUTS); +} + +void +bytecode_walker:: maybe_emit_bind_ans_and_disp (tree_expression &expr, const std::string maybe_cmd_name) { bool print_result = expr.print_result (); + // Anonymous functions never print or write to ans. + // The value currently on the stack is used as the return value + // by RET_ANON. For normal functions the write to ans would pop it. + if (m_is_anon) + return; + // If this is an root expression we need to write the return value // to ans. if (DEPTH () == 1) @@ -2857,7 +2997,10 @@ if (m_is_script) PUSH_CODE (INSTR::EXIT_SCRIPT_FRAME); - PUSH_CODE (INSTR::RET); + if (m_is_anon) + PUSH_CODE (INSTR::RET_ANON); + else + PUSH_CODE (INSTR::RET); } void @@ -2947,7 +3090,7 @@ PUSH_CODE (0); // nargout PUSH_CODE_INT (tree_idx); - if (DEPTH () == 1) + if (!m_is_anon && DEPTH () == 1) PUSH_CODE (INSTR::POP); POP_NARGOUT (); @@ -3136,7 +3279,7 @@ PUSH_CODE (INSTR::DUP); emit_disp_obj (expr); } - if (DEPTH () == 1) + if (!m_is_anon && DEPTH () == 1) PUSH_CODE (INSTR::POP); } } @@ -3247,7 +3390,7 @@ // SUBASSIGN_OBJ puts the lhs back on the stack // but since lhs is not an id from a slot we just // pop it unless it is used by a chained assign. - if (DEPTH () == 1) + if (!m_is_anon && DEPTH () == 1) PUSH_CODE (INSTR::POP); } else if (type == '(') @@ -3445,7 +3588,7 @@ // SUBASSIGN_OBJ puts the lhs back on the stack // but since lhs is not an id from a slot we just // pop it, unless there are chained assignments. - if (DEPTH () == 1) + if (!m_is_anon && DEPTH () == 1) PUSH_CODE (INSTR::POP); } else //(is_dynamic_field && !is_id) @@ -3483,7 +3626,7 @@ // SUBASSIGN_OBJ puts the lhs back on the stack // but since lhs is not an id from a slot we just // pop it, unless there are chained assignments. - if (DEPTH () == 1) + if (!m_is_anon && DEPTH () == 1) PUSH_CODE (INSTR::POP); } } @@ -3714,9 +3857,11 @@ tree_expression *e = *it; CHECK_NONNULL (e); + PUSH_NARGOUT (1); INC_DEPTH (); e->accept (*this); DEC_DEPTH (); + POP_NARGOUT (); n_cols++; // We now need to expand the value (if it is an cs list) @@ -3750,6 +3895,8 @@ { INC_DEPTH(); + maybe_emit_anon_maybe_ignore_outputs (); + std::string name = id.name (); if (name == "__VM_DBG") { @@ -3837,7 +3984,7 @@ PUSH_CODE (INSTR::PUSH_SLOT_INDEXED); PUSH_SLOT (slot); } - else if (DEPTH () == 1) + else if (DEPTH () == 1 && NARGOUT () != -1) { CHECK (NARGOUT () == 0); @@ -3876,6 +4023,12 @@ PUSH_CODE (INSTR::PUSH_SLOT_NARGOUT1); PUSH_SLOT (slot); } + else if (NARGOUT () == -1) + { + MAYBE_PUSH_WIDE_OPEXT (slot); + PUSH_CODE (INSTR::PUSH_SLOT_NX); + PUSH_SLOT (slot); + } else if (NARGOUT() > 1) { MAYBE_PUSH_WIDE_OPEXT (slot); @@ -4381,11 +4534,15 @@ tree_expression *e = expr.expression (); CHECK_NONNULL(e); + maybe_emit_anon_maybe_ignore_outputs (); + PUSH_TREE_FOR_EVAL (&expr); int tree_idx = -CODE_SIZE (); // NB: Negative to not collide with debug data + int nargout = NARGOUT (); + MAYBE_PUSH_ANON_NARGOUT_OPEXT (nargout); PUSH_CODE (INSTR::EVAL); - PUSH_CODE (NARGOUT ()); + PUSH_CODE (nargout); PUSH_CODE_INT (tree_idx); maybe_emit_bind_ans_and_disp (expr); @@ -4465,6 +4622,8 @@ m_ignored_ip_start = CODE_SIZE (); // visit_multi_assignment () need the code offset to set the proper range for the unwind protect } + maybe_emit_anon_maybe_ignore_outputs (); + int loc_id = N_LOC (); PUSH_LOC (); LOC (loc_id).m_ip_start = CODE_SIZE (); @@ -4480,12 +4639,24 @@ std::string id_name = e->name (); int slot = SLOT (id_name); MAYBE_PUSH_WIDE_OPEXT (slot); - PUSH_CODE (INSTR::WORDCMD); - // The vm need the name of the identifier for function lookups - PUSH_SLOT (slot); - PUSH_CODE (nargout); - // Push nargin - PUSH_CODE (args ? args->size () : 0); + + if (nargout == -1) // Anonymous functions need dynamic nargout + { + PUSH_CODE (INSTR::WORDCMD_NX); + // The vm need the name of the identifier for function lookups + PUSH_SLOT (slot); + // Push nargin + PUSH_CODE (args ? args->size () : 0); + } + else + { + PUSH_CODE (INSTR::WORDCMD); + // The vm need the name of the identifier for function lookups + PUSH_SLOT (slot); + PUSH_CODE (nargout); + // Push nargin + PUSH_CODE (args ? args->size () : 0); + } } else if (e->is_identifier () && !(type == '.' && !struct_is_id_dot_id)) { @@ -4523,6 +4694,13 @@ PUSH_SLOT (slot); } + // Anonymous functions need to have dynamic nargout + else if (nargout == -1) + { + MAYBE_PUSH_WIDE_OPEXT (slot); + PUSH_CODE (INSTR::INDEX_IDNX); + PUSH_SLOT (slot); + } else { MAYBE_PUSH_WIDE_OPEXT (slot); @@ -4549,6 +4727,12 @@ PUSH_CODE (INSTR::INDEX_CELL_ID_NARGOUT1); PUSH_SLOT (slot); } + else if (nargout == -1) + { + MAYBE_PUSH_WIDE_OPEXT (slot); + PUSH_CODE (INSTR::INDEX_CELL_IDNX); + PUSH_SLOT (slot); + } else { MAYBE_PUSH_WIDE_OPEXT (slot); @@ -4562,6 +4746,7 @@ } else if (type == '.') { + MAYBE_PUSH_ANON_NARGOUT_OPEXT (nargout); PUSH_CODE (INSTR::INDEX_STRUCT_NARGOUTN); PUSH_CODE (nargout); @@ -4580,8 +4765,9 @@ // We are not indexing an id, but e.g.: // (foo).() // I.e. a temporary object. + MAYBE_PUSH_ANON_NARGOUT_OPEXT (nargout); PUSH_CODE (INSTR::INDEX_OBJ); - PUSH_CODE (nargout); + PUSH_CODE (nargout); // This is arg0, not used if EXT_NARGOUT is used PUSH_CODE (0); // "has slot" PUSH_WSLOT (0); // The w/e slot TODO: Remove? // Push nargin @@ -4732,6 +4918,8 @@ m_ignored_ip_start = CODE_SIZE (); // visit_multi_assignment () need the code offset to set the proper range for the unwind protect } + maybe_emit_anon_maybe_ignore_outputs (); + int nargout = NARGOUT (); arg_name_entry.m_ip_start = CODE_SIZE (); @@ -4739,18 +4927,19 @@ if (first_expression && first_expression->is_identifier ()) { int slot = SLOT (first_expression->name ()); - MAYBE_PUSH_WIDE_OPEXT (slot); + MAYBE_PUSH_ANON_NARGOUT_OPEXT (nargout); PUSH_CODE (INSTR::INDEX_STRUCT_CALL); - PUSH_SLOT (slot); // the slot + PUSH_CODE (nargout); PUSH_CODE (1); // has slot - PUSH_CODE (nargout); + PUSH_WSLOT (slot); // the slot } else { + MAYBE_PUSH_ANON_NARGOUT_OPEXT (nargout); PUSH_CODE (INSTR::INDEX_STRUCT_CALL); - PUSH_SLOT (0); // slot - PUSH_CODE (0); // has slot PUSH_CODE (nargout); + PUSH_SLOT (0); // has slot + PUSH_WSLOT (0); // slot ignored } PUSH_CODE (v_n_args.size ()); @@ -5042,6 +5231,8 @@ // slot for the handle function cache int slot = add_id_to_table(aname); + maybe_emit_anon_maybe_ignore_outputs (); + MAYBE_PUSH_WIDE_OPEXT (slot); PUSH_CODE (INSTR::PUSH_FCN_HANDLE); PUSH_SLOT (slot); @@ -5072,6 +5263,8 @@ CHECK_NONNULL (op3); op3->accept (*this); + maybe_emit_anon_maybe_ignore_outputs (); + // Colon expressions have some different semantics // in command expressions. if (expr.is_for_cmd_expr ()) diff -r c52ae0cfd9fc -r 7d924d061e9d libinterp/parse-tree/pt-bytecode-walk.h --- a/libinterp/parse-tree/pt-bytecode-walk.h Mon Aug 07 10:10:49 2023 +0200 +++ b/libinterp/parse-tree/pt-bytecode-walk.h Thu Aug 10 17:07:19 2023 +0200 @@ -43,6 +43,7 @@ namespace octave { void compile_user_function (octave_user_code &fn, bool print); + void compile_anon_user_function (octave_user_code &fn, bool print, stack_frame::local_vars_map &locals); // No separate visitor needed // Base classes only, so no need to include them. @@ -153,6 +154,7 @@ bool m_varargout = false; bool m_is_script = false; + bool m_is_anon = false; std::vector> m_continue_target; std::vector> m_need_break_target; @@ -187,6 +189,11 @@ std::vector m_v_ignored; int m_ignored_ip_start = 0; + // The values that the locals of an anonymous function are supposed + // to be set to. The field is set before calling the first accept() + // for anonymous functions. + stack_frame::local_vars_map *m_anon_local_values = nullptr; + // bool m_is_folding = false; std::vector m_v_trees_to_fold; @@ -210,6 +217,7 @@ void emit_load_2_cst (tree_expression *lhs, tree_expression *rhs); + void maybe_emit_anon_maybe_ignore_outputs (); void maybe_emit_bind_ans_and_disp (tree_expression &expr, const std::string maybe_cmd_name = ""); void maybe_emit_disp_id (tree_expression &expr, const std::string &name, const std::string maybe_cmd_name = "" ); void maybe_emit_push_and_disp_id (tree_expression &expr, const std::string &name, const std::string maybe_cmd_name = ""); diff -r c52ae0cfd9fc -r 7d924d061e9d libinterp/parse-tree/pt-bytecode.h --- a/libinterp/parse-tree/pt-bytecode.h Mon Aug 07 10:10:49 2023 +0200 +++ b/libinterp/parse-tree/pt-bytecode.h Thu Aug 10 17:07:19 2023 +0200 @@ -184,6 +184,13 @@ SUBASSIGN_ID_MAT_2D, ENTER_SCRIPT_FRAME, EXIT_SCRIPT_FRAME, + RET_ANON, + INDEX_IDNX, + INDEX_CELL_IDNX, + PUSH_SLOT_NX, + EXT_NARGOUT, + WORDCMD_NX, + ANON_MAYBE_SET_IGNORE_OUTPUTS, }; enum class unwind_entry_type @@ -234,7 +241,8 @@ unsigned m_code_size; unsigned m_ids_size; - bool m_is_script; + bool m_is_script = false; + bool m_is_anon = false; }; struct bytecode diff -r c52ae0cfd9fc -r 7d924d061e9d libinterp/parse-tree/pt-eval.cc --- a/libinterp/parse-tree/pt-eval.cc Mon Aug 07 10:10:49 2023 +0200 +++ b/libinterp/parse-tree/pt-eval.cc Thu Aug 10 17:07:19 2023 +0200 @@ -73,6 +73,7 @@ #include "utils.h" #include "variables.h" #include "pt-bytecode-vm.h" +#include "pt-bytecode-walk.h" OCTAVE_BEGIN_NAMESPACE(octave) @@ -3566,7 +3567,11 @@ // The caller can be bytecode if evalin("caller", ...) is used in some uncompiled function. if (!caller_is_bytecode) set_auto_fcn_var (stack_frame::ARG_NAMES, Cell (xargs.name_tags ())); - set_auto_fcn_var (stack_frame::IGNORED, ignored_outputs); + if (ignored_outputs.numel()) + { + vm.caller_ignores_output (); + set_auto_fcn_var (stack_frame::IGNORED, ignored_outputs); + } octave_value_list ret; diff -r c52ae0cfd9fc -r 7d924d061e9d test/compile/bytecode.tst --- a/test/compile/bytecode.tst Mon Aug 07 10:10:49 2023 +0200 +++ b/test/compile/bytecode.tst Thu Aug 10 17:07:19 2023 +0200 @@ -621,14 +621,18 @@ %!test %! __enable_vm_eval__ (0, "local"); %! clear all -%! key = "1 2 12 3 4 1 2 3 1 2 11 12 1 "; +%! key = "1 2 12 3 4 4 4 1 2 3 1 2 3 1 2 1 2 11 12 11 12 1 4 4 1 1 1 2 1 2 1 2 3 1 2 3 1 3 1 3 9 0 1 fooo ~101 103 ~101 103 ~110 123 ~101 123 ~010 123 ~000 123 ~011 123 "; %! __compile__ bytecode_anon_handles clear; %! bytecode_anon_handles; +%! +%! global __assert_printf__; +%! %! assert (__prog_output_assert__ (key)); %! %! __enable_vm_eval__ (1, "local"); %! assert (__compile__ ("bytecode_anon_handles")); %! bytecode_anon_handles; +%! global __assert_printf__; %! assert (__prog_output_assert__ (key)); ## Test compling a function named differently from its diff -r c52ae0cfd9fc -r 7d924d061e9d test/compile/bytecode_anon_handles.m --- a/test/compile/bytecode_anon_handles.m Mon Aug 07 10:10:49 2023 +0200 +++ b/test/compile/bytecode_anon_handles.m Thu Aug 10 17:07:19 2023 +0200 @@ -13,17 +13,113 @@ h3 = @(a,b,c) a + b + c; __printf_assert__ ("%d ", h3 (1, 2, 1)); + __printf_assert__ ("%d ", h3 (1, 2, 1)); + + h3 (1, 2, 1); + __printf_assert__ ("%d ", ans); h4 = @() {1,2,3}{:}; [a b c] = h4(); __printf_assert__ ("%d %d %d ", a, b, c); + [a b c] = h4(); + __printf_assert__ ("%d %d %d ", a, b, c); + [a b] = h4(); + __printf_assert__ ("%d %d ", a, b); [a b] = h4(); __printf_assert__ ("%d %d ", a, b); h5 = @(x) @(y) __printf_assert__ ("%d %d ", x, y); h5(11)(12) + h5(11)(12) % max not in parent scope h6 = @(x, y) max (x, y); __printf_assert__ ("%d ", h6 (-1, 1)); + + % Mess with the anon function's stackframe + a = 3; + h7 = @() foo () + a; + __printf_assert__ ("%d ", h7 ()); % 4 + __printf_assert__ ("%d ", h7 ()); % also 4 + + % Nargout + h8 = @() expression_nargout (); + a = h8 (); + __printf_assert__ ("%d ", a); + a = h8 (); + __printf_assert__ ("%d ", a); + + [a b] = h8 (); + __printf_assert__ ("%d ", a, b); + [a b] = h8 (); + __printf_assert__ ("%d ", a, b); + + [a b c] = h8 (); + __printf_assert__ ("%d ", a, b, c); + [a b c] = h8 (); + __printf_assert__ ("%d ", a, b, c); + + [a, ~, c] = h8 (); + __printf_assert__ ("%d ", a, c); + h8 = @() expression_nargout (); + [a, ~, c] = h8 (); + __printf_assert__ ("%d ", a, c); + + % ans + h9 = @() 9; + h9 (); + __printf_assert__ ("%d ", ans); + + % word command + h10 = @() nargout; + h10 (); + __printf_assert__ ("%d ", ans); + a = h10 (); + __printf_assert__ ("%d ", a); + + % inputname + h11 = @(x) inputname (1); + fooo = 123; + __printf_assert__ ("%s ", h11 (fooo)); + + % Ignored outputs are propagated to nested calls + h12 = @() try_isargout (); + x = y = z = 0; + [x, ~, z] = h12 (); __printf_assert__ ("%d%d%d ", x, y, z); + [x, ~, z] = h12 (); __printf_assert__ ("%d%d%d ", x, y, z); + [x, y, ~] = h12 (); __printf_assert__ ("%d%d%d ", x, y, z); + [x, ~, z] = h12 (); __printf_assert__ ("%d%d%d ", x, y, z); + [~, y, ~] = h12 (); __printf_assert__ ("%d%d%d ", x, y, z); + [~, ~, ~] = h12 (); __printf_assert__ ("%d%d%d ", x, y, z); + [~, y, z] = h12 (); __printf_assert__ ("%d%d%d ", x, y, z); + + % The optim package exposed a bug with EXPAND_CS_LIST during development + h1 = @ (p) - (p(1)^2 + 1 - p(2)); + h2 = @ (p) {[], h1(p)}{:}; + [~, a] = h2 ([-2 5]) + assert (a == 0) + [~, a] = h2 ([-2 5]) + assert (a == 0) end + +function [x, y, z] = try_isargout () + __printf_assert__ ("~%d%d%d ", isargout (1), isargout (2), isargout (3)) + x = 1; y = 2; z = 3; + + bar (); % Does nothing. Check return from nested subfunction works with active ignore +endfunction + +function bar +end + +function ret = foo + evalin ("caller", "a++;"); % Should not change 'a' for the next time h7 is called + ret = 0; +end + +function varargout = expression_nargout () + varargout = cell (1, nargout); + for i = 1:nargout + varargout{i} = i; + end +end