[dart/vm] Fix dominance violation

Rationale:
The flow graph checker still had a few ugly exceptions
due to dominance violation after inlining. This CL fixes
the violation, removes the exceptions from the flow graph
checker, and also adds a new utility that can be used in
some other places.

dart-lang#36899

Change-Id: I42766af22a3696fe050b0e5c4a2dfe930933fc40
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/111502
Reviewed-by: Martin Kustermann <[email protected]>
Commit-Queue: Aart Bik <[email protected]>

Author: aartbik

runtime/vm/compiler/backend/branch_optimizer.cc

@@ -169,16 +169,8 @@ void BranchSimplifier::Simplify(FlowGraph* flow_graph) {
           new_branch->comparison()->SetDeoptId(*comparison);
           // The phi can be used in the branch's environment.  Rename such
           // uses.
-          for (Environment::DeepIterator it(new_branch->env()); !it.Done();
-               it.Advance()) {
-            Value* use = it.CurrentValue();
-            if (use->definition() == phi) {
-              Definition* replacement = phi->InputAt(i)->definition();
-              use->RemoveFromUseList();
-              use->set_definition(replacement);
-              replacement->AddEnvUse(use);
-            }
-          }
+          Definition* replacement = phi->InputAt(i)->definition();
+          new_branch->ReplaceInEnvironment(phi, replacement);
         }
 
         new_branch->InsertBefore(old_goto);

runtime/vm/compiler/backend/flow_graph_checker.cc

@@ -226,7 +226,7 @@ void FlowGraphChecker::VisitInstruction(Instruction* instruction) {
   for (intptr_t i = 0, n = instruction->InputCount(); i < n; ++i) {
     VisitUseDef(instruction, instruction->InputAt(i), i, /*is_env*/ false);
   }
-  // Check all environment inputs.
+  // Check all environment inputs (including outer ones).
   intptr_t i = 0;
   for (Environment::DeepIterator it(instruction->env()); !it.Done();
        it.Advance()) {
@@ -271,14 +271,15 @@ void FlowGraphChecker::VisitUseDef(Instruction* instruction,
   // Make sure each input is properly defined in the graph by something
   // that dominates the input (note that the proper dominance relation
   // on the input values of Phis is checked by the Phi visitor below).
-  bool test_def = def->HasSSATemp();
   if (def->IsPhi()) {
     ASSERT(def->GetBlock()->IsJoinEntry());
     // Phis are never linked into graph.
     ASSERT(def->next() == nullptr);
     ASSERT(def->previous() == nullptr);
   } else if (def->IsConstant() || def->IsParameter() ||
              def->IsSpecialParameter()) {
+    // Special constants reside outside the IR.
+    if (IsSpecialConstant(def)) return;
     // Initial definitions are partially linked into graph, but some
     // constants are fully linked into graph (so no next() assert).
     ASSERT(def->previous() != nullptr);
@@ -287,14 +288,10 @@ void FlowGraphChecker::VisitUseDef(Instruction* instruction,
     ASSERT(def->next() != nullptr);
     ASSERT(def->previous() != nullptr);
   }
-  if (test_def) {
-    ASSERT(is_env ||  // TODO(dartbug.com/36899)
-           DefDominatesUse(def, instruction));
-    if (is_env) {
-      ASSERT(IsInUseList(def->env_use_list(), instruction));
-    } else {
-      ASSERT(IsInUseList(def->input_use_list(), instruction));
-    }
+  if (def->HasSSATemp()) {
+    ASSERT(DefDominatesUse(def, instruction));
+    ASSERT(IsInUseList(is_env ? def->env_use_list() : def->input_use_list(),
+                       instruction));
   }
 }
 

runtime/vm/compiler/backend/il.cc

@@ -1172,6 +1172,18 @@ void Instruction::RemoveEnvironment() {
   env_ = NULL;
 }
 
+void Instruction::ReplaceInEnvironment(Definition* current,
+                                       Definition* replacement) {
+  for (Environment::DeepIterator it(env()); !it.Done(); it.Advance()) {
+    Value* use = it.CurrentValue();
+    if (use->definition() == current) {
+      use->RemoveFromUseList();
+      use->set_definition(replacement);
+      replacement->AddEnvUse(use);
+    }
+  }
+}
+
 Instruction* Instruction::RemoveFromGraph(bool return_previous) {
   ASSERT(!IsBlockEntry());
   ASSERT(!IsBranch());

runtime/vm/compiler/backend/il.h

@@ -830,6 +830,7 @@ class Instruction : public ZoneAllocated {
   Environment* env() const { return env_; }
   void SetEnvironment(Environment* deopt_env);
   void RemoveEnvironment();
+  void ReplaceInEnvironment(Definition* current, Definition* replacement);
 
   intptr_t lifetime_position() const { return lifetime_position_; }
   void set_lifetime_position(intptr_t pos) { lifetime_position_ = pos; }

runtime/vm/compiler/backend/inliner.cc

@@ -635,6 +635,11 @@ static void ReplaceParameterStubs(Zone* zone,
       }
       redefinition->InsertAfter(callee_entry);
       defn = redefinition;
+      // Since the redefinition does not dominate the callee entry, replace
+      // uses of the receiver argument in this entry with the redefined value.
+      callee_entry->ReplaceInEnvironment(
+          call_data->parameter_stubs->At(first_arg_stub_index + i),
+          actual->definition());
     } else if (actual != NULL) {
       defn = actual->definition();
     }

runtime/vm/compiler/backend/redundancy_elimination.cc

@@ -3128,15 +3128,7 @@ void AllocationSinking::CreateMaterializationAt(
   // MaterializeObject instruction.
   // We must preserve the identity: all mentions are replaced by the same
   // materialization.
-  for (Environment::DeepIterator env_it(exit->env()); !env_it.Done();
-       env_it.Advance()) {
-    Value* use = env_it.CurrentValue();
-    if (use->definition() == alloc) {
-      use->RemoveFromUseList();
-      use->set_definition(mat);
-      mat->AddEnvUse(use);
-    }
-  }
+  exit->ReplaceInEnvironment(alloc, mat);
 
   // Mark MaterializeObject as an environment use of this allocation.
   // This will allow us to discover it when we are looking for deoptimization