Mercurial > octave

changeset 32317:906ea9c9c5d9

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
VM Fix chained subsassigns keeping a value live on the stack (Bug #64557) Give SUBASSIGN_CHAINED access to slot number, so that it can assign to a identifier inplace, to not waste memory. * libinterp/parse-tree/pt-bytecode-vm.cc: Restructure how SUBASSIGN_CHAINED works * libinterp/parse-tree/pt-bytecode-walk.cc: Slot for SUBASSIGN_CHAINED Nullptr check for args
author Petter T. <petter.vilhelm@gmail.com>
date Wed, 20 Sep 2023 20:00:21 -0400
parents 8d3b15dd7505
children ddf7c5f4ddde
files libinterp/parse-tree/pt-bytecode-vm.cc libinterp/parse-tree/pt-bytecode-walk.cc
diffstat 2 files changed, 35 insertions(+), 10 deletions(-) [+]
line wrap: on
line diff
--- a/libinterp/parse-tree/pt-bytecode-vm.cc	Wed Sep 20 19:56:02 2023 -0400
+++ b/libinterp/parse-tree/pt-bytecode-vm.cc	Wed Sep 20 20:00:21 2023 -0400
@@ -433,6 +433,7 @@
           CASE_END ()
 
           CASE_START (SUBASSIGN_CHAINED)
+            PSLOT ();
             PCHAR (); // op
             PCHAR (); // nchained
             int nn = *p;
@@ -5639,7 +5640,8 @@
 
 subassign_chained:
   {
-    octave_value::assign_op op = static_cast<octave_value::assign_op> (arg0);
+    int slot = arg0;
+    octave_value::assign_op op = static_cast<octave_value::assign_op> (*ip++);
     int n_chained = POP_CODE ();
     std::vector<int> v_n_args;
     std::string type (n_chained, 0);
@@ -5684,15 +5686,36 @@
         if (type.size () && type.back () != '(' && lhs_assign_numel (lhs, type, idx) != 1)
           err_invalid_structure_assignment ();
 
-        lhs.assign (op, type, idx, rhs);
+        if (slot)
+          {
+            octave_value &lhs_slot = bsp[slot].ov;
+
+            // We don't need to he lhs value put on the stack since we are working on a slot.
+            // Clear it to make assigns not need a new copy.
+            lhs = octave_value {};
+
+            bool is_ref = lhs_slot.is_ref ();
+
+            if (is_ref)
+              lhs_slot.ref_rep ()->ref ().make_unique ();
+
+            lhs_slot.assign (op, type, idx, rhs);
+
+            // Push a dummy octave_value. Always poped by a POP opcode. 
+            PUSH_OV (octave_value {});
+          }
+        else
+          {
+            lhs.assign (op, type, idx, rhs);
+            // The value is pushed and used for further chaining.
+            PUSH_OV (lhs);
+          }
       }
     CATCH_INTERRUPT_EXCEPTION
     CATCH_INDEX_EXCEPTION
     CATCH_EXECUTION_EXCEPTION
     CATCH_BAD_ALLOC
     CATCH_EXIT_EXCEPTION
-
-    PUSH_OV (lhs);
   }
   DISPATCH ();
 
--- a/libinterp/parse-tree/pt-bytecode-walk.cc	Wed Sep 20 19:56:02 2023 -0400
+++ b/libinterp/parse-tree/pt-bytecode-walk.cc	Wed Sep 20 20:00:21 2023 -0400
@@ -3359,7 +3359,7 @@
 
               n_args_in_part++;
             }
-          else
+          else if (args)
             {
               // Push all the args to the stack
 
@@ -3413,7 +3413,13 @@
       // We want to put them in lists and feed them to a subsassgn() call. We use
       // a special op-code for this.
 
+      int slot = 0;
+      if (e->is_identifier ())
+        slot = SLOT (e->name ());
+
+      MAYBE_PUSH_WIDE_OPEXT (slot);
       PUSH_CODE (INSTR::SUBASSIGN_CHAINED);
+      PUSH_SLOT (slot);
       PUSH_CODE (op); // =, += etc.
       PUSH_CODE (n_chained);
       for (unsigned i = 0; i < n_chained; i++)
@@ -3426,11 +3432,7 @@
       // Now we got the value that is subassigned to, on the stack
       if (e->is_identifier ())
         {
-          // Write the subassigned value back to the slot
-          int slot = SLOT (e->name ());
-          MAYBE_PUSH_WIDE_OPEXT (slot);
-          PUSH_CODE (INSTR::FORCE_ASSIGN);
-          PUSH_SLOT (slot);
+          PUSH_CODE (INSTR::POP);
 
           maybe_emit_push_and_disp_id (expr, e->name ());
         }