[vm/compiler] Rename PushArgument to MoveArgument

This is follow up to 65f4a73,
which switched optimized code to use fixed frame for outgoing
arguments.

Change Kernel to IL translation to handle null-checks in
invocations differently: this code used to duplicate receiver on
the stack to accomodate for PushArgument in unoptimized code, but
PushArgument has not been inserted since f4e61ea,
which means duplication of the receiver is no longer necessary.

TEST=ci

Cq-Include-Trybots: luci.dart.try:vm-kernel-precomp-linux-debug-x64-try,vm-kernel-precomp-dwarf-linux-product-x64-try,vm-kernel-precomp-linux-release-simarm-try,vm-kernel-precomp-linux-release-x64-try,vm-kernel-precomp-nnbd-linux-debug-simarm_x64-try,vm-kernel-precomp-nnbd-linux-release-simarm64-try
Change-Id: I6c1f1e8c354f9ea92424b6602b83b9e9ebce8b69
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/284184
Commit-Queue: Slava Egorov <[email protected]>
Reviewed-by: Martin Kustermann <[email protected]>

Author: mraleph

runtime/tests/vm/dart/records_allocation_sinking_il_test.dart

@@ -35,14 +35,14 @@ void matchIL$test(FlowGraph graph) {
       'bar' << match.Parameter(index: 3),
       'baz' << match.Parameter(index: 4),
       match.CheckStackOverflow(),
-      match.PushArgument('x'),
+      match.MoveArgument('x'),
       match.StaticCall(),
-      match.PushArgument('y'),
+      match.MoveArgument('y'),
       match.StaticCall(),
       'baz_boxed' << match.BoxInt64('baz'),
-      match.PushArgument('foo'),
+      match.MoveArgument('foo'),
       match.StaticCall(),
-      match.PushArgument('baz_boxed'),
+      match.MoveArgument('baz_boxed'),
       match.StaticCall(),
       match.Return(),
     ]),

runtime/tests/vm/dart/records_return_value_unboxing_il_test.dart

@@ -103,42 +103,42 @@ void matchIL$test(FlowGraph graph) {
       'obj2' << match.Parameter(index: 5),
       match.CheckStackOverflow(),
 
-      match.PushArgument('x'),
-      match.PushArgument('z'),
+      match.MoveArgument('x'),
+      match.MoveArgument('z'),
       'r1' << match.StaticCall(),
       'r1_0' << match.ExtractNthOutput('r1', index: 0),
       'r1_1' << match.ExtractNthOutput('r1', index: 1),
-      match.PushArgument('r1_0'),
+      match.MoveArgument('r1_0'),
       match.StaticCall(),
-      match.PushArgument('r1_1'),
+      match.MoveArgument('r1_1'),
       match.StaticCall(),
 
-      match.PushArgument('foo'),
-      match.PushArgument('bar'),
+      match.MoveArgument('foo'),
+      match.MoveArgument('bar'),
       'r2' << match.StaticCall(),
       'r2_bar' << match.ExtractNthOutput('r2', index: 0),
       'r2_foo' << match.ExtractNthOutput('r2', index: 1),
-      match.PushArgument('r2_foo'),
+      match.MoveArgument('r2_foo'),
       match.StaticCall(),
-      match.PushArgument('r2_bar'),
+      match.MoveArgument('r2_bar'),
       match.StaticCall(),
 
-      match.PushArgument('obj1'),
+      match.MoveArgument('obj1'),
       'r3' << match.StaticCall(),
       'r3_0' << match.ExtractNthOutput('r3', index: 0),
       'r3_y' << match.ExtractNthOutput('r3', index: 1),
-      match.PushArgument('r3_0'),
+      match.MoveArgument('r3_0'),
       match.StaticCall(),
-      match.PushArgument('r3_y'),
+      match.MoveArgument('r3_y'),
       match.StaticCall(),
 
       'obj2_cid' << match.LoadClassId('obj2'),
-      match.PushArgument('obj2'),
+      match.MoveArgument('obj2'),
       'r4' << match.DispatchTableCall('obj2_cid'),
       'r4_0' << match.ExtractNthOutput('r4', index: 0),
       'r4_y' << match.ExtractNthOutput('r4', index: 1),
       'r4_boxed' << match.AllocateSmallRecord('r4_0', 'r4_y'),
-      match.PushArgument('r4_boxed'),
+      match.MoveArgument('r4_boxed'),
       match.StaticCall(),
 
       match.Return(),

runtime/vm/compiler/backend/constant_propagator.cc

@@ -432,8 +432,8 @@ void ConstantPropagator::VisitNativeParameter(NativeParameterInstr* instr) {
   SetValue(instr, non_constant_);
 }
 
-void ConstantPropagator::VisitPushArgument(PushArgumentInstr* instr) {
-  UNREACHABLE();
+void ConstantPropagator::VisitMoveArgument(MoveArgumentInstr* instr) {
+  UNREACHABLE();  // Inserted right before register allocation.
 }
 
 void ConstantPropagator::VisitAssertAssignable(AssertAssignableInstr* instr) {
@@ -1753,7 +1753,6 @@ bool ConstantPropagator::TransformDefinition(Definition* defn) {
   // Replace constant-valued instructions without observable side
   // effects.  Do this for smis and old objects only to avoid having to
   // copy other objects into the heap's old generation.
-  ASSERT((defn == nullptr) || !defn->IsPushArgument());
   if ((defn != nullptr) && IsConstant(defn->constant_value()) &&
       (defn->constant_value().IsSmi() || defn->constant_value().IsOld()) &&
       !defn->IsConstant() && !defn->IsStoreIndexed() && !defn->IsStoreField() &&
@@ -1770,7 +1769,7 @@ bool ConstantPropagator::TransformDefinition(Definition* defn) {
       ASSERT(!constant_value_.IsNull());
     }
     if (auto call = defn->AsStaticCall()) {
-      ASSERT(!call->HasPushArguments());
+      ASSERT(!call->HasMoveArguments());
     }
     Definition* replacement =
         graph_->TryCreateConstantReplacementFor(defn, constant_value_);

runtime/vm/compiler/backend/flow_graph.cc

@@ -158,7 +158,7 @@ void FlowGraph::ReplaceCurrentInstruction(ForwardInstructionIterator* iterator,
     }
   }
   if (current->ArgumentCount() != 0) {
-    ASSERT(!current->HasPushArguments());
+    ASSERT(!current->HasMoveArguments());
   }
   iterator->RemoveCurrentFromGraph();
 }
@@ -1558,7 +1558,7 @@ void FlowGraph::RenameRecursive(
         break;
       }
 
-      case Instruction::kPushArgument:
+      case Instruction::kMoveArgument:
         UNREACHABLE();
         break;
 
@@ -1628,7 +1628,7 @@ void FlowGraph::RenameRecursive(
           // Rename input operand.
           Definition* input = (*env)[i];
           ASSERT(input != nullptr);
-          ASSERT(!input->IsPushArgument());
+          ASSERT(!input->IsMoveArgument());
           Value* use = new (zone()) Value(input);
           phi->SetInputAt(pred_index, use);
         }
@@ -2414,7 +2414,7 @@ void FlowGraph::WidenSmiToInt32() {
         if (use_defn == NULL) {
           // We assume that tagging before returning or pushing argument costs
           // very little compared to the cost of the return/call itself.
-          ASSERT(!instr->IsPushArgument());
+          ASSERT(!instr->IsMoveArgument());
           if (!instr->IsReturn() &&
               (use->use_index() >= instr->ArgumentCount())) {
             gain--;
@@ -2998,7 +2998,7 @@ PhiInstr* FlowGraph::AddPhi(JoinEntryInstr* join,
   return phi;
 }
 
-void FlowGraph::InsertPushArguments() {
+void FlowGraph::InsertMoveArguments() {
   intptr_t max_argument_slot_count = 0;
   for (BlockIterator block_it = reverse_postorder_iterator(); !block_it.Done();
        block_it.Advance()) {
@@ -3010,37 +3010,37 @@ void FlowGraph::InsertPushArguments() {
       if (arg_count == 0) {
         continue;
       }
-      PushArgumentsArray* arguments =
-          new (Z) PushArgumentsArray(zone(), arg_count);
+      MoveArgumentsArray* arguments =
+          new (Z) MoveArgumentsArray(zone(), arg_count);
       arguments->EnsureLength(arg_count, nullptr);
 
-      intptr_t top_of_stack_relative_index = 0;
+      intptr_t sp_relative_index = 0;
       for (intptr_t i = arg_count - 1; i >= 0; --i) {
         Value* arg = instruction->ArgumentValueAt(i);
         const auto rep = instruction->RequiredInputRepresentation(i);
-        (*arguments)[i] = new (Z) PushArgumentInstr(
-            arg->CopyWithType(Z), rep, top_of_stack_relative_index);
+        (*arguments)[i] = new (Z)
+            MoveArgumentInstr(arg->CopyWithType(Z), rep, sp_relative_index);
 
         static_assert(compiler::target::kIntSpillFactor ==
                           compiler::target::kDoubleSpillFactor,
                       "double and int are expected to be of the same size");
         RELEASE_ASSERT(rep == kTagged || rep == kUnboxedDouble ||
                        rep == kUnboxedInt64);
-        top_of_stack_relative_index +=
+        sp_relative_index +=
             (rep == kTagged) ? 1 : compiler::target::kIntSpillFactor;
       }
       max_argument_slot_count =
-          Utils::Maximum(max_argument_slot_count, top_of_stack_relative_index);
+          Utils::Maximum(max_argument_slot_count, sp_relative_index);
 
-      for (auto push_arg : *arguments) {
-        // Insert all PushArgument instructions immediately before call.
-        // PushArgumentInstr::EmitNativeCode may generate more efficient
-        // code for subsequent PushArgument instructions (ARM, ARM64).
-        InsertBefore(instruction, push_arg, /*env=*/nullptr, kEffect);
+      for (auto move_arg : *arguments) {
+        // Insert all MoveArgument instructions immediately before call.
+        // MoveArgumentInstr::EmitNativeCode may generate more efficient
+        // code for subsequent MoveArgument instructions (ARM, ARM64).
+        InsertBefore(instruction, move_arg, /*env=*/nullptr, kEffect);
       }
-      instruction->ReplaceInputsWithPushArguments(arguments);
+      instruction->ReplaceInputsWithMoveArguments(arguments);
       if (instruction->env() != nullptr) {
-        instruction->RepairPushArgsInEnvironment();
+        instruction->RepairArgumentUsesInEnvironment();
       }
     }
   }

runtime/vm/compiler/backend/flow_graph.h

@@ -382,11 +382,11 @@ class FlowGraph : public ZoneAllocated {
   // Remove the redefinition instructions inserted to inhibit code motion.
   void RemoveRedefinitions(bool keep_checks = false);
 
-  // Insert PushArgument instructions and remove explicit def-use
+  // Insert MoveArgument instructions and remove explicit def-use
   // relations between calls and their arguments.
   //
   // Compute the maximum number of arguments.
-  void InsertPushArguments();
+  void InsertMoveArguments();
 
   // Copy deoptimization target from one instruction to another if we still
   // have to keep deoptimization environment at gotos for LICM purposes.

runtime/vm/compiler/backend/flow_graph_checker.cc

@@ -140,10 +140,10 @@ static void AssertArgumentsInEnv(FlowGraph* flow_graph, Definition* call) {
     ASSERT1((arg_count + after_args_input_count) <= env_count, call);
     const intptr_t env_base = env_count - arg_count - after_args_input_count;
     for (intptr_t i = 0; i < arg_count; i++) {
-      if (call->HasPushArguments()) {
+      if (call->HasMoveArguments()) {
         ASSERT1(call->ArgumentAt(i) == env->ValueAt(env_base + i)
                                            ->definition()
-                                           ->AsPushArgument()
+                                           ->AsMoveArgument()
                                            ->value()
                                            ->definition(),
                 call);

runtime/vm/compiler/backend/flow_graph_compiler.cc

@@ -95,16 +95,16 @@ compiler::LRState ComputeInnerLRState(const FlowGraph& flow_graph) {
 }
 #endif
 
-// Assign locations to incoming arguments, i.e., values pushed above spill slots
-// with PushArgument.  Recursively allocates from outermost to innermost
-// environment.
-void CompilerDeoptInfo::AllocateIncomingParametersRecursive(Environment* env) {
+// Assign locations to outgoing arguments. Note that MoveArgument
+// can only occur in the innermost environment because we insert
+// them immediately before the call instruction and right before
+// register allocation.
+void CompilerDeoptInfo::AllocateOutgoingArguments(Environment* env) {
   if (env == NULL) return;
-  AllocateIncomingParametersRecursive(env->outer());
   for (Environment::ShallowIterator it(env); !it.Done(); it.Advance()) {
     if (it.CurrentLocation().IsInvalid()) {
-      if (auto push_arg = it.CurrentValue()->definition()->AsPushArgument()) {
-        it.SetCurrentLocation(push_arg->locs()->out(0));
+      if (auto move_arg = it.CurrentValue()->definition()->AsMoveArgument()) {
+        it.SetCurrentLocation(move_arg->locs()->out(0));
       }
     }
   }
@@ -592,8 +592,7 @@ static bool IsPusher(Instruction* instr) {
 static bool IsPopper(Instruction* instr) {
   // TODO(ajcbik): even allow deopt targets by making environment aware?
   if (!instr->CanBecomeDeoptimizationTarget()) {
-    return !instr->IsPushArgument() && instr->ArgumentCount() == 0 &&
-           instr->InputCount() > 0;
+    return instr->ArgumentCount() == 0 && instr->InputCount() > 0;
   }
   return false;
 }
@@ -1040,21 +1039,21 @@ void FlowGraphCompiler::RecordSafepoint(LocationSummary* locs,
     RELEASE_ASSERT(args_count == 0 || is_optimizing());
 
     for (intptr_t i = 0; i < args_count; i++) {
-      const auto push_arg =
-          instr->ArgumentValueAt(i)->instruction()->AsPushArgument();
-      const auto rep = push_arg->representation();
+      const auto move_arg =
+          instr->ArgumentValueAt(i)->instruction()->AsMoveArgument();
+      const auto rep = move_arg->representation();
 
       ASSERT(rep == kTagged || rep == kUnboxedInt64 || rep == kUnboxedDouble);
       static_assert(compiler::target::kIntSpillFactor ==
                         compiler::target::kDoubleSpillFactor,
                     "int and double are of the same size");
-      const bool is_tagged = push_arg->representation() == kTagged;
+      const bool is_tagged = move_arg->representation() == kTagged;
       const intptr_t num_bits =
           is_tagged ? 1 : compiler::target::kIntSpillFactor;
 
       // Note: bits are reversed so higher bit corresponds to lower word.
       const intptr_t last_arg_bit =
-          (spill_area_size - 1) - push_arg->top_of_stack_relative_index();
+          (spill_area_size - 1) - move_arg->sp_relative_index();
       bitmap.SetRange(last_arg_bit - (num_bits - 1), last_arg_bit, is_tagged);
     }
     ASSERT(slow_path_argument_count == 0 || !using_shared_stub);
@@ -1731,7 +1730,7 @@ void FlowGraphCompiler::AllocateRegistersLocally(Instruction* instr) {
   }
 
   // Allocate all unallocated input locations.
-  RELEASE_ASSERT(!instr->IsPushArgument());
+  ASSERT(!instr->IsMoveArgument());
   Register fpu_unboxing_temp = kNoRegister;
   for (intptr_t i = locs->input_count() - 1; i >= 0; i--) {
     Location loc = locs->in(i);
@@ -3292,10 +3291,6 @@ void FlowGraphCompiler::FrameStateUpdateWith(Instruction* instr) {
   ASSERT(!is_optimizing());
 
   switch (instr->tag()) {
-    case Instruction::kPushArgument:
-      // Do nothing.
-      break;
-
     case Instruction::kDropTemps:
       FrameStatePop(instr->locs()->input_count() +
                     instr->AsDropTemps()->num_temps());

runtime/vm/compiler/backend/flow_graph_compiler.h

@@ -199,7 +199,7 @@ class CompilerDeoptInfo : public ZoneAllocated {
  private:
   void EmitMaterializations(Environment* env, DeoptInfoBuilder* builder);
 
-  void AllocateIncomingParametersRecursive(Environment* env);
+  void AllocateOutgoingArguments(Environment* env);
 
   intptr_t pc_offset_;
   const intptr_t deopt_id_;

runtime/vm/compiler/backend/flow_graph_compiler_arm.cc

@@ -98,7 +98,7 @@ TypedDataPtr CompilerDeoptInfo::CreateDeoptInfo(FlowGraphCompiler* compiler,
     return TypedData::null();
   }
 
-  AllocateIncomingParametersRecursive(deopt_env_);
+  AllocateOutgoingArguments(deopt_env_);
 
   intptr_t slot_ix = 0;
   Environment* current = deopt_env_;

runtime/vm/compiler/backend/flow_graph_compiler_arm64.cc

@@ -92,7 +92,7 @@ TypedDataPtr CompilerDeoptInfo::CreateDeoptInfo(FlowGraphCompiler* compiler,
     return TypedData::null();
   }
 
-  AllocateIncomingParametersRecursive(deopt_env_);
+  AllocateOutgoingArguments(deopt_env_);
 
   intptr_t slot_ix = 0;
   Environment* current = deopt_env_;

runtime/vm/compiler/backend/flow_graph_compiler_ia32.cc

@@ -70,7 +70,7 @@ TypedDataPtr CompilerDeoptInfo::CreateDeoptInfo(FlowGraphCompiler* compiler,
     return TypedData::null();
   }
 
-  AllocateIncomingParametersRecursive(deopt_env_);
+  AllocateOutgoingArguments(deopt_env_);
 
   intptr_t slot_ix = 0;
   Environment* current = deopt_env_;

runtime/vm/compiler/backend/flow_graph_compiler_riscv.cc

@@ -79,7 +79,7 @@ TypedDataPtr CompilerDeoptInfo::CreateDeoptInfo(FlowGraphCompiler* compiler,
     return TypedData::null();
   }
 
-  AllocateIncomingParametersRecursive(deopt_env_);
+  AllocateOutgoingArguments(deopt_env_);
 
   intptr_t slot_ix = 0;
   Environment* current = deopt_env_;

runtime/vm/compiler/backend/flow_graph_compiler_x64.cc

@@ -93,7 +93,7 @@ TypedDataPtr CompilerDeoptInfo::CreateDeoptInfo(FlowGraphCompiler* compiler,
     return TypedData::null();
   }
 
-  AllocateIncomingParametersRecursive(deopt_env_);
+  AllocateOutgoingArguments(deopt_env_);
 
   intptr_t slot_ix = 0;
   Environment* current = deopt_env_;

runtime/vm/compiler/backend/il.cc

@@ -1475,18 +1475,18 @@ void Instruction::UnuseAllInputs() {
   }
 }
 
-void Instruction::RepairPushArgsInEnvironment() const {
+void Instruction::RepairArgumentUsesInEnvironment() const {
   // Some calls (e.g. closure calls) have more inputs than actual arguments.
   // Those extra inputs will be consumed from the stack before the call.
   const intptr_t after_args_input_count = env()->LazyDeoptPruneCount();
-  PushArgumentsArray* push_arguments = GetPushArguments();
-  ASSERT(push_arguments != nullptr);
+  MoveArgumentsArray* move_arguments = GetMoveArguments();
+  ASSERT(move_arguments != nullptr);
   const intptr_t arg_count = ArgumentCount();
   ASSERT((arg_count + after_args_input_count) <= env()->Length());
   const intptr_t env_base =
       env()->Length() - arg_count - after_args_input_count;
   for (intptr_t i = 0; i < arg_count; ++i) {
-    env()->ValueAt(env_base + i)->BindToEnvironment(push_arguments->At(i));
+    env()->ValueAt(env_base + i)->BindToEnvironment(move_arguments->At(i));
   }
 }