# HG changeset patch # User Petter T. # Date 1707950328 -3600 # Wed Feb 14 23:38:48 2024 +0100 # Branch bytecode-interpreter # Node ID f1e32a79b7691916d60600aab2c0675d9431cb85 # Parent 024ec6194932e7fbb931813ea09a6abdce9df28a Support more than 255 named arguments and returns in the bytecode interpreter (bug #65304) Use two bytes to store the amount of argument symbols and return symbols, to allow for 2^15-1 symbols. Replace magic anon. fn. marker -128 with -32768. * pt-bytecode-vm-internal.h: Change size to two bytes * pt-bytecode-vm.cc: Change size to two bytes * pt-bytecode-walk.cc: Emit two bytes Error check new limits * bytecode_subfuncs.m: Update tests diff -r 024ec6194932 -r f1e32a79b769 libinterp/parse-tree/pt-bytecode-vm-internal.h --- a/libinterp/parse-tree/pt-bytecode-vm-internal.h Wed Feb 14 23:38:48 2024 +0100 +++ b/libinterp/parse-tree/pt-bytecode-vm-internal.h Wed Feb 14 23:38:48 2024 +0100 @@ -379,15 +379,15 @@ \ /* Set the ip to 0 */ \ ip = code; \ -int n_returns_callee = static_cast (*ip++); /* Negative for varargout */ \ +int n_returns_callee = POP_CODE_SHORT (); /* Negative for varargout */ \ if (OCTAVE_UNLIKELY (n_returns_callee < 0)) \ { \ - if (n_returns_callee == -128) /* Anonymous function */ \ + if (n_returns_callee == -32768) /* 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_args_callee = POP_CODE_SHORT (); /* Negative for varargin */ \ int n_locals_callee = POP_CODE_USHORT (); \ \ if (n_args_callee < 0) \ diff -r 024ec6194932 -r f1e32a79b769 libinterp/parse-tree/pt-bytecode-vm.cc --- a/libinterp/parse-tree/pt-bytecode-vm.cc Wed Feb 14 23:38:48 2024 +0100 +++ b/libinterp/parse-tree/pt-bytecode-vm.cc Wed Feb 14 23:38:48 2024 +0100 @@ -76,6 +76,7 @@ static void copy_many_args_to_caller (octave::stack_element *sp, octave::stack_element *caller_stack_end, int n_args_to_move, int n_args_caller_expects); static int lhs_assign_numel (octave_value &ov, const std::string& type, const std::list& idx); +static int pop_code_short (unsigned char *ip); #define TODO(msg) error("Not done yet %d: " msg, __LINE__) #define ERR(msg) error("VM error %d: " msg, __LINE__) @@ -122,7 +123,7 @@ bool wide_opext_active = false; // Skip some framedata - p += 4; + p += 6; std::vector> v_pair_row_str; @@ -574,9 +575,9 @@ cout << "\t" << bc.m_data[1].string_value () << "\n\n"; // function type cout << "frame:\n"; - cout << "\t.n_return " << to_string (*p++) << "\n"; - cout << "\t.n_args " << to_string (*p++) << "\n"; - cout << "\t.n_locals " << to_string (*p++) << "\n\n"; + cout << "\t.n_return " << to_string (pop_code_short (p + 2)) << "\n"; + cout << "\t.n_args " << to_string (pop_code_short (p + 4)) << "\n"; + cout << "\t.n_locals " << to_string (pop_code_short (p + 6)) << "\n\n"; cout << "slots:\n"; int idx = 0; @@ -649,7 +650,19 @@ return ans; } - +static int pop_code_short (unsigned char *ip) +{ + short ans; + ip -= 2; +#ifdef WORDS_BIGENDIAN + ans = *ip++ << 8; + ans |= *ip++; +#else + ans = *ip++; + ans |= *ip++ << 8; +#endif + return ans; +} // Debug functions easy to break out into in gdb. Called by __dummy_mark_1__() in Octave extern "C" void dummy_mark_1 (void); @@ -658,6 +671,7 @@ #define POP_CODE() *ip++ #define POP_CODE_INT() (ip++,ip++,ip++,ip++,pop_code_int (ip)) #define POP_CODE_USHORT() (ip++, ip++, pop_code_ushort (ip)) +#define POP_CODE_SHORT() (ip++, ip++, pop_code_short (ip)) #define PUSH_OV(ov) \ do { \ @@ -1139,13 +1153,13 @@ // Read the meta data for constructing a stack frame. { -#define N_RETURNS() static_cast (code[0]) -#define N_ARGS() static_cast (code[1]) -#define N_LOCALS() USHORT_FROM_UCHAR_PTR (code + 2) - - int n_returns = static_cast (*ip++); +#define N_RETURNS() pop_code_short (&code[2]) +#define N_ARGS() pop_code_short (&code[4]) +#define N_LOCALS() USHORT_FROM_UCHAR_PTR (code + 4) + + int n_returns = POP_CODE_SHORT (); // n_args is negative for varargin calls - int n_args = static_cast (*ip++); + int n_args = POP_CODE_SHORT (); int n_locals = POP_CODE_USHORT (); // Note: An arg and return can share slot bool is_varargin = n_args < 0; @@ -1157,7 +1171,7 @@ n_args = -n_args; if (OCTAVE_UNLIKELY (n_returns < 0)) // Negative for varargout and anonymous functions { - if (n_returns != -128) + if (n_returns != -32768) n_returns = -n_returns; else n_returns = 1; @@ -3369,7 +3383,7 @@ ignored = tmp.matrix_value (); int n_returns = N_RETURNS (); - if (n_returns == -128) + if (n_returns == -32768) n_returns = 1; else if (n_returns < 0) n_returns = -n_returns; @@ -3421,7 +3435,7 @@ ignored = tmp.matrix_value (); int n_returns = N_RETURNS (); - if (n_returns == -128) + if (n_returns == -32768) n_returns = 1; else if (n_returns < 0) n_returns = -n_returns; @@ -4765,15 +4779,15 @@ octave_user_function *usr_fcn = static_cast (sp[0].pv); - int n_returns_callee = static_cast (ip[-4]); + int n_returns_callee = pop_code_short (&ip[-4]); if (OCTAVE_UNLIKELY (n_returns_callee < 0)) { - if (n_returns_callee == -128) /* Anonymous function */ + if (n_returns_callee == -32768) /* 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_args_callee = -pop_code_short (&ip[-2]);; // Note: Minus to make it positive int n_locals_callee = USHORT_FROM_UCHAR_PTR (ip - 2); int nargout = sp[-1].i; @@ -5011,15 +5025,15 @@ /* Set the ip to 0 */ ip = code; - int n_returns_callee = static_cast (*ip++); /* Negative for varargout */ + int n_returns_callee = POP_CODE_SHORT (); /* Negative for varargout */ if (OCTAVE_UNLIKELY (n_returns_callee < 0)) { - if (n_returns_callee == -128) /* Anonymous function */ + if (n_returns_callee == -32768) /* 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_args_callee = POP_CODE_SHORT (); /* Negative for varargin */ int n_locals_callee = POP_CODE_USHORT (); if (n_args_callee < 0) @@ -6419,7 +6433,7 @@ // variables on the VM stack if it is referenced from somewhere else. m_tw->get_current_stack_frame ()->vm_unwinds (); - assert (N_RETURNS () == -128); + assert (N_RETURNS () == -32768); int n_returns_callee = bsp[0].i; // Nargout on stack if (n_returns_callee == 0) diff -r 024ec6194932 -r f1e32a79b769 libinterp/parse-tree/pt-bytecode-walk.cc --- a/libinterp/parse-tree/pt-bytecode-walk.cc Wed Feb 14 23:38:48 2024 +0100 +++ b/libinterp/parse-tree/pt-bytecode-walk.cc Wed Feb 14 23:38:48 2024 +0100 @@ -2411,10 +2411,10 @@ tree_statement_list *cmd_list = fcn.body (); // The first instruction is the amount of return variables. - PUSH_CODE (1); // Only the dummy return '%nargout' + PUSH_CODE_SHORT (1); // Only the dummy return '%nargout' // The second instruction is the amount of arguments - PUSH_CODE (0); + PUSH_CODE_SHORT (0); // The third is the amount of locals, which need to be set // after compiling the function. So we need to store the offset @@ -2597,17 +2597,20 @@ // so add one to size if 'm_varargout' is true int n_returns = returns ? returns->size () + m_varargout: 0; + if (n_returns > 32767) + error ("The bytecode compiler only supports up to 32767 named return values. Consider using varargout"); + // The first instruction is the amount of return variables. Negative for varargout. // Anonymous functions have no return parameter list specified. For those we set - // the amount of returns to magic number -128. + // the amount of returns to magic number -32768. 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); + PUSH_CODE_SHORT (-32768); } else - PUSH_CODE (m_varargout ? -(n_returns + 1) : (n_returns + 1)); // +1 for native '%nargout' on the stack. + PUSH_CODE_SHORT (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 @@ -2622,7 +2625,9 @@ // The second instruction is the amount of arguments int n_paras = v_paras.size (); - PUSH_CODE (is_varargin ? -n_paras : n_paras); + if (n_paras > 32767) + error ("The bytecode compiler only supports up to 32767 named arguments. Consider using varargin"); + PUSH_CODE_SHORT (is_varargin ? -n_paras : n_paras); // The third is the amount of locals, which need to be set // after compiling the function. So we need to store the offset diff -r 024ec6194932 -r f1e32a79b769 test/compile/bytecode_subfuncs.m --- a/test/compile/bytecode_subfuncs.m Wed Feb 14 23:38:48 2024 +0100 +++ b/test/compile/bytecode_subfuncs.m Wed Feb 14 23:38:48 2024 +0100 @@ -63,6 +63,17 @@ takeXp32retXp32 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, ... 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63, 64); __printf_assert__ ("%d ", a01, a18, a59, a64); + + # Test calling a function with more then 255 arguments (which use another opcode to among other things handle "many args") + + # Uses INDEX_OBJ + a = ret1take300 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63, 64, 65, 66, 67, 68, 69, 70, 71, 72, 73, 74, 75, 76, 77, 78, 79, 80, 81, 82, 83, 84, 85, 86, 87, 88, 89, 90, 91, 92, 93, 94, 95, 96, 97, 98, 99, 100, 101, 102, 103, 104, 105, 106, 107, 108, 109, 110, 111, 112, 113, 114, 115, 116, 117, 118, 119, 120, 121, 122, 123, 124, 125, 126, 127, 128, 129, 130, 131, 132, 133, 134, 135, 136, 137, 138, 139, 140, 141, 142, 143, 144, 145, 146, 147, 148, 149, 150, 151, 152, 153, 154, 155, 156, 157, 158, 159, 160, 161, 162, 163, 164, 165, 166, 167, 168, 169, 170, 171, 172, 173, 174, 175, 176, 177, 178, 179, 180, 181, 182, 183, 184, 185, 186, 187, 188, 189, 190, 191, 192, 193, 194, 195, 196, 197, 198, 199, 200, 201, 202, 203, 204, 205, 206, 207, 208, 209, 210, 211, 212, 213, 214, 215, 216, 217, 218, 219, 220, 221, 222, 223, 224, 225, 226, 227, 228, 229, 230, 231, 232, 233, 234, 235, 236, 237, 238, 239, 240, 241, 242, 243, 244, 245, 246, 247, 248, 249, 250, 251, 252, 253, 254, 255, 256, 257, 258, 259, 260, 261, 262, 263, 264, 265, 266, 267, 268, 269, 270, 271, 272, 273, 274, 275, 276, 277, 278, 279, 280, 281, 282, 283, 284, 285, 286, 287, 288, 289, 290, 291, 292, 293, 294, 295, 296, 297, 298, 299, 300); + assert (a == sum (1:300)) + + # Uses INDEX_STRUCT_CALL and INDEX_STRUCT_SUBCALL + a = ret1take300 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63, 64, 65, 66, 67, 68, 69, 70, 71, 72, 73, 74, 75, 76, 77, 78, 79, 80, 81, 82, 83, 84, 85, 86, 87, 88, 89, 90, 91, 92, 93, 94, 95, 96, 97, 98, 99, 100, 101, 102, 103, 104, 105, 106, 107, 108, 109, 110, 111, 112, 113, 114, 115, 116, 117, 118, 119, 120, 121, 122, 123, 124, 125, 126, 127, 128, 129, 130, 131, 132, 133, 134, 135, 136, 137, 138, 139, 140, 141, 142, 143, 144, 145, 146, 147, 148, 149, 150, 151, 152, 153, 154, 155, 156, 157, 158, 159, 160, 161, 162, 163, 164, 165, 166, 167, 168, 169, 170, 171, 172, 173, 174, 175, 176, 177, 178, 179, 180, 181, 182, 183, 184, 185, 186, 187, 188, 189, 190, 191, 192, 193, 194, 195, 196, 197, 198, 199, 200, 201, 202, 203, 204, 205, 206, 207, 208, 209, 210, 211, 212, 213, 214, 215, 216, 217, 218, 219, 220, 221, 222, 223, 224, 225, 226, 227, 228, 229, 230, 231, 232, 233, 234, 235, 236, 237, 238, 239, 240, 241, 242, 243, 244, 245, 246, 247, 248, 249, 250, 251, 252, 253, 254, 255, 256, 257, 258, 259, 260, 261, 262, 263, 264, 265, 266, 267, 268, 269, 270, 271, 272, 273, 274, 275, 276, 277, 278, 279, 280, 281, 282, 283, 284, 285, 286, 287, 288, 289, 290, 291, 292, 293, 294, 295, 296, 297, 298, 299, ... + 300) (1); # <--- Note extra index operation + assert (a == sum (1:300)) end function [a01, a02, a03, a04, a05, a06, a07, a08, a09, a10, a11, a12, a13, a14, a15, a16, a17, a18, a19, a20, a21, a22, a23, a24, a25, a26, a27, a28, a29, a30, a31, a32, varargout] = takeXp32retXp32 (b01, b02, b03, b04, b05, b06, b07, b08, b09, b10, b11, b12, b13, b14, b15, b16, b17, b18, b19, b20, b21, b22, b23, b24, b25, b26, b27, b28, b29, b30, b31, b32, varargin) @@ -126,3 +137,13 @@ __printf_assert__ ("%d ", size(d)); __printf_assert__ ("%s ", class(d)); end + +function a = ret1take300 (a1, a2, a3, a4, a5, a6, a7, a8, a9, a10, a11, a12, a13, a14, a15, a16, a17, a18, a19, a20, a21, a22, a23, a24, a25, a26, a27, a28, a29, a30, a31, a32, a33, a34, a35, a36, a37, a38, a39, a40, a41, a42, a43, a44, a45, a46, a47, a48, a49, a50, a51, a52, a53, a54, a55, a56, a57, a58, a59, a60, a61, a62, a63, a64, a65, a66, a67, a68, a69, a70, a71, a72, a73, a74, a75, a76, a77, a78, a79, a80, a81, a82, a83, a84, a85, a86, a87, a88, a89, a90, a91, a92, a93, a94, a95, a96, a97, a98, a99, a100, a101, a102, a103, a104, a105, a106, a107, a108, a109, a110, a111, a112, a113, a114, a115, a116, a117, a118, a119, a120, a121, a122, a123, a124, a125, a126, a127, a128, a129, a130, a131, a132, a133, a134, a135, a136, a137, a138, a139, a140, a141, a142, a143, a144, a145, a146, a147, a148, a149, a150, a151, a152, a153, a154, a155, a156, a157, a158, a159, a160, a161, a162, a163, a164, a165, a166, a167, a168, a169, a170, a171, a172, a173, a174, a175, a176, a177, a178, a179, a180, a181, a182, a183, a184, a185, a186, a187, a188, a189, a190, a191, a192, a193, a194, a195, a196, a197, a198, a199, a200, a201, a202, a203, a204, a205, a206, a207, a208, a209, a210, a211, a212, a213, a214, a215, a216, a217, a218, a219, a220, a221, a222, a223, a224, a225, a226, a227, a228, a229, a230, a231, a232, a233, a234, a235, a236, a237, a238, a239, a240, a241, a242, a243, a244, a245, a246, a247, a248, a249, a250, a251, a252, a253, a254, a255, a256, a257, a258, a259, a260, a261, a262, a263, a264, a265, a266, a267, a268, a269, a270, a271, a272, a273, a274, a275, a276, a277, a278, a279, a280, a281, a282, a283, a284, a285, a286, a287, a288, a289, a290, a291, a292, a293, a294, a295, + a296, a297, a298, a299, a300, a1234 = 1234) + assert (a1234 == 1234) + a = 0; + for i = 1:300 + eval (sprintf ("a += a%d;", i)) + end +end +