[dart/vm] enhanced graph checker

Rationale:
Refactored existing verify uses (with some overlap and some
additional checks) into the graph checker. Also added more
verification code and repaired some inconsistencies in IR
found by new checker. However, some checks are disabled with
a TODO, since the IR does not currently meet all the stricter
assumptions. Fixes will follow.

https://github.com/dart-lang/sdk/issues/36893
https://github.com/dart-lang/sdk/issues/36894
https://github.com/dart-lang/sdk/issues/36895
https://github.com/dart-lang/sdk/issues/36899

Change-Id: Ic0395208da38ecb6fc8ca2551efe819e6458a731
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/101922
Commit-Queue: Aart Bik <ajcbik@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
diff --git a/runtime/vm/compiler/aot/precompiler.cc b/runtime/vm/compiler/aot/precompiler.cc
index 2355495..29b1fad 100644
--- a/runtime/vm/compiler/aot/precompiler.cc
+++ b/runtime/vm/compiler/aot/precompiler.cc
@@ -72,7 +72,6 @@
 DECLARE_FLAG(bool, trace_compiler);
 DECLARE_FLAG(bool, trace_optimizing_compiler);
 DECLARE_FLAG(bool, trace_bailout);
-DECLARE_FLAG(bool, verify_compiler);
 DECLARE_FLAG(bool, huge_method_cutoff_in_code_size);
 DECLARE_FLAG(bool, trace_failed_optimization_attempts);
 DECLARE_FLAG(bool, trace_inlining_intervals);
diff --git a/runtime/vm/compiler/backend/block_builder.h b/runtime/vm/compiler/backend/block_builder.h
index 22557d7..94b566a 100644
--- a/runtime/vm/compiler/backend/block_builder.h
+++ b/runtime/vm/compiler/backend/block_builder.h
@@ -54,8 +54,9 @@
   }
 
   Definition* AddParameter(intptr_t index, bool with_frame) {
-    return AddToInitialDefinitions(new ParameterInstr(
-        index, flow_graph_->graph_entry(), with_frame ? FPREG : SPREG));
+    auto normal_entry = flow_graph_->graph_entry()->normal_entry();
+    return AddToInitialDefinitions(
+        new ParameterInstr(index, normal_entry, with_frame ? FPREG : SPREG));
   }
 
   TokenPosition TokenPos() { return flow_graph_->function().token_pos(); }
diff --git a/runtime/vm/compiler/backend/flow_graph.cc b/runtime/vm/compiler/backend/flow_graph.cc
index 40eac02..406c5dd 100644
--- a/runtime/vm/compiler/backend/flow_graph.cc
+++ b/runtime/vm/compiler/backend/flow_graph.cc
@@ -27,7 +27,6 @@
 DEFINE_FLAG(bool, trace_smi_widening, false, "Trace Smi->Int32 widening pass.");
 #endif
 DEFINE_FLAG(bool, prune_dead_locals, true, "optimize dead locals away");
-DECLARE_FLAG(bool, verify_compiler);
 
 // Quick access to the current zone.
 #define Z (zone())
@@ -299,80 +298,6 @@
   if (changed) DiscoverBlocks();
 }
 
-// Debugging code to verify the construction of use lists.
-static intptr_t MembershipCount(Value* use, Value* list) {
-  intptr_t count = 0;
-  while (list != NULL) {
-    if (list == use) ++count;
-    list = list->next_use();
-  }
-  return count;
-}
-
-static void VerifyUseListsInInstruction(Instruction* instr) {
-  ASSERT(instr != NULL);
-  ASSERT(!instr->IsJoinEntry());
-  for (intptr_t i = 0; i < instr->InputCount(); ++i) {
-    Value* use = instr->InputAt(i);
-    ASSERT(use->definition() != NULL);
-    ASSERT((use->definition() != instr) || use->definition()->IsPhi() ||
-           use->definition()->IsMaterializeObject());
-    ASSERT(use->instruction() == instr);
-    ASSERT(use->use_index() == i);
-    ASSERT(!FLAG_verify_compiler ||
-           (1 == MembershipCount(use, use->definition()->input_use_list())));
-  }
-  if (instr->env() != NULL) {
-    intptr_t use_index = 0;
-    for (Environment::DeepIterator it(instr->env()); !it.Done(); it.Advance()) {
-      Value* use = it.CurrentValue();
-      ASSERT(use->definition() != NULL);
-      ASSERT((use->definition() != instr) || use->definition()->IsPhi());
-      ASSERT(use->instruction() == instr);
-      ASSERT(use->use_index() == use_index++);
-      ASSERT(!FLAG_verify_compiler ||
-             (1 == MembershipCount(use, use->definition()->env_use_list())));
-    }
-  }
-  Definition* defn = instr->AsDefinition();
-  if (defn != NULL) {
-    // Used definitions must have an SSA name.  We use the name to index
-    // into bit vectors during analyses.  Some definitions without SSA names
-    // (e.g., PushArgument) have environment uses.
-    ASSERT((defn->input_use_list() == NULL) || defn->HasSSATemp());
-    Value* prev = NULL;
-    Value* curr = defn->input_use_list();
-    while (curr != NULL) {
-      ASSERT(prev == curr->previous_use());
-      ASSERT(defn == curr->definition());
-      Instruction* instr = curr->instruction();
-      // The instruction should not be removed from the graph.
-      ASSERT((instr->IsPhi() && instr->AsPhi()->is_alive()) ||
-             (instr->previous() != NULL));
-      ASSERT(curr == instr->InputAt(curr->use_index()));
-      prev = curr;
-      curr = curr->next_use();
-    }
-
-    prev = NULL;
-    curr = defn->env_use_list();
-    while (curr != NULL) {
-      ASSERT(prev == curr->previous_use());
-      ASSERT(defn == curr->definition());
-      Instruction* instr = curr->instruction();
-      ASSERT(curr == instr->env()->ValueAtUseIndex(curr->use_index()));
-      // BlockEntry instructions have environments attached to them but
-      // have no reliable way to verify if they are still in the graph.
-      // Thus we just assume they are.
-      ASSERT(instr->IsBlockEntry() ||
-             (instr->IsPhi() && instr->AsPhi()->is_alive()) ||
-             (instr->previous() != NULL));
-      prev = curr;
-      curr = curr->next_use();
-    }
-  }
-}
-
 void FlowGraph::ComputeIsReceiverRecursive(
     PhiInstr* phi,
     GrowableArray<PhiInstr*>* unmark) const {
@@ -574,31 +499,6 @@
   InsertBefore(call, guard, call->env(), FlowGraph::kEffect);
 }
 
-bool FlowGraph::VerifyUseLists() {
-  // Verify the initial definitions.
-  for (intptr_t i = 0; i < graph_entry_->initial_definitions()->length(); ++i) {
-    VerifyUseListsInInstruction((*graph_entry_->initial_definitions())[i]);
-  }
-
-  // Verify phis in join entries and the instructions in each block.
-  for (intptr_t i = 0; i < preorder_.length(); ++i) {
-    BlockEntryInstr* entry = preorder_[i];
-    JoinEntryInstr* join = entry->AsJoinEntry();
-    if (join != NULL) {
-      for (PhiIterator it(join); !it.Done(); it.Advance()) {
-        PhiInstr* phi = it.Current();
-        ASSERT(phi != NULL);
-        VerifyUseListsInInstruction(phi);
-      }
-    }
-    for (ForwardInstructionIterator it(entry); !it.Done(); it.Advance()) {
-      VerifyUseListsInInstruction(it.Current());
-    }
-  }
-
-  return true;  // Return true so we can ASSERT validation.
-}
-
 // Verify that a redefinition dominates all uses of the redefined value.
 bool FlowGraph::VerifyRedefinitions() {
   for (BlockIterator block_it = reverse_postorder_iterator(); !block_it.Done();
@@ -1240,7 +1140,7 @@
 
     param->set_ssa_temp_index(alloc_ssa_temp_index());  // New SSA temp.
     (*env)[i] = param;
-    catch_entry->initial_definitions()->Add(param);
+    AddToInitialDefinitions(catch_entry, param);
   }
 }
 
diff --git a/runtime/vm/compiler/backend/flow_graph.h b/runtime/vm/compiler/backend/flow_graph.h
index 67f47ce..abbfcac 100644
--- a/runtime/vm/compiler/backend/flow_graph.h
+++ b/runtime/vm/compiler/backend/flow_graph.h
@@ -264,8 +264,7 @@
   void ComputeSSA(intptr_t next_virtual_register_number,
                   ZoneGrowableArray<Definition*>* inlining_parameters);
 
-  // Verification methods for debugging.
-  bool VerifyUseLists();
+  // Verification method for debugging.
   bool VerifyRedefinitions();
 
   void DiscoverBlocks();
diff --git a/runtime/vm/compiler/backend/flow_graph_checker.cc b/runtime/vm/compiler/backend/flow_graph_checker.cc
index 8872e8a..242e3e2 100644
--- a/runtime/vm/compiler/backend/flow_graph_checker.cc
+++ b/runtime/vm/compiler/backend/flow_graph_checker.cc
@@ -13,6 +13,8 @@
 
 namespace dart {
 
+DECLARE_FLAG(bool, trace_compiler);
+
 // Returns true if block is a predecessor of succ.
 static bool IsPred(BlockEntryInstr* block, BlockEntryInstr* succ) {
   for (intptr_t i = 0, n = succ->PredecessorCount(); i < n; ++i) {
@@ -44,10 +46,9 @@
   return false;
 }
 
-// Returns true if instruction appears on def's use list.
-static bool IsInDefUseList(Definition* def, Instruction* instruction) {
-  for (Value* use = def->input_use_list(); use != nullptr;
-       use = use->next_use()) {
+// Returns true if instruction appears in use list.
+static bool IsInUseList(Value* use, Instruction* instruction) {
+  for (; use != nullptr; use = use->next_use()) {
     if (use->instruction() == instruction) {
       return true;
     }
@@ -55,6 +56,32 @@
   return false;
 }
 
+// Returns true if definition dominates instruction. Note that this
+// helper is required to account for some situations that are not
+// accounted for in the IR methods that compute dominance.
+static bool DefDominatesUse(Definition* def, Instruction* instruction) {
+  if (instruction->IsPhi()) {
+    // A phi use is not necessarily dominated by a definition.
+    // Proper dominance relation on the input values of Phis is
+    // checked by the Phi visitor below.
+    return true;
+  } else if (def->IsMaterializeObject() || instruction->IsMaterializeObject()) {
+    // These instructions resides outside the IR.
+    return true;
+  } else if (auto entry =
+                 instruction->GetBlock()->AsBlockEntryWithInitialDefs()) {
+    // An initial definition in the same block.
+    // TODO(ajcbik): use an initial def too?
+    for (auto idef : *entry->initial_definitions()) {
+      if (idef == def) {
+        return true;
+      }
+    }
+  }
+  // Use the standard IR method for dominance.
+  return instruction->IsDominatedBy(def);
+}
+
 // Returns true if instruction forces control flow.
 static bool IsControlFlow(Instruction* instruction) {
   return instruction->IsBranch() || instruction->IsGoto() ||
@@ -108,10 +135,6 @@
     // Visit all instructions in this block.
     VisitInstructions(block);
   }
-
-  // Flow graph built-in verification.
-  // TODO(ajcbik): migrate actual code into checker too?
-  ASSERT(flow_graph_->VerifyUseLists());
 }
 
 void FlowGraphChecker::VisitInstructions(BlockEntryInstr* block) {
@@ -122,13 +145,39 @@
   }
   // Give all visitors quick access.
   current_block_ = block;
+  // Visit initial definitions.
+  if (auto entry = block->AsBlockEntryWithInitialDefs()) {
+    GrowableArray<Definition*>* initial_defs = entry->initial_definitions();
+    for (intptr_t i = 0, n = initial_defs->length(); i < n; i++) {
+      Definition* def = (*initial_defs)[i];
+      ASSERT(def != nullptr);
+      ASSERT(def->IsConstant() || def->IsParameter() ||
+             def->IsSpecialParameter());
+      // TODO(dartbug.com/36895) fix this bail-out
+      // All null/optimized constants misbehave, parameters are moved
+      // around without updating block_field.
+      if (def->IsConstant() || def->GetBlock() != entry) {
+        continue;
+      }
+      // Make sure block lookup agrees.
+      ASSERT(def->GetBlock() == entry);
+      // Initial definitions are partially linked into graph.
+      ASSERT(def->next() == nullptr);
+      ASSERT(def->previous() == entry);
+      // Visit the initial definition as instruction.
+      VisitInstruction(def);
+    }
+  }
   // Visit phis in join.
-  if (auto join_entry = block->AsJoinEntry()) {
-    for (PhiIterator it(join_entry); !it.Done(); it.Advance()) {
+  if (auto entry = block->AsJoinEntry()) {
+    for (PhiIterator it(entry); !it.Done(); it.Advance()) {
       PhiInstr* phi = it.Current();
       // Make sure block lookup agrees.
-      ASSERT(phi->GetBlock() == join_entry);
-      // Visit phi as instruction.
+      ASSERT(phi->GetBlock() == entry);
+      // Phis are never linked into graph.
+      ASSERT(phi->next() == nullptr);
+      ASSERT(phi->previous() == nullptr);
+      // Visit the phi as instruction.
       VisitInstruction(phi);
     }
   }
@@ -147,9 +196,11 @@
     prev = instruction;
     // Make sure control flow makes sense.
     ASSERT(IsControlFlow(instruction) == (instruction == last));
-    // Perform instruction specific checks.
+    ASSERT(!instruction->IsPhi());
+    // Visit the instruction.
     VisitInstruction(instruction);
   }
+  ASSERT(prev->next() == nullptr);
   ASSERT(prev == last);
   // Make sure loop information, when up-to-date, agrees.
   if (flow_graph_->loop_hierarchy_ != nullptr) {
@@ -161,29 +212,17 @@
 }
 
 void FlowGraphChecker::VisitInstruction(Instruction* instruction) {
-  // Make sure all inputs are properly defined by something that
-  // dominates the use that is not a Phi instruction (note that the
-  // proper dominance relation on the input values of Phis are checked
-  // by the Phi visitor below). Also verify some graph sanity.
+  ASSERT(!instruction->IsBlockEntry());
+  // Check all regular inputs.
   for (intptr_t i = 0, n = instruction->InputCount(); i < n; ++i) {
-    Definition* def = instruction->InputAt(i)->definition();
-    bool test_def = def->HasSSATemp();
-    if (def->IsPhi()) {
-      // Phis are never linked into graph.
-      ASSERT(def->next() == nullptr);
-      ASSERT(def->previous() == nullptr);
-    } else if (def->IsConstant() || def->IsParameter() ||
-               def->IsSpecialParameter()) {
-      test_def = false;
-    } else {
-      ASSERT(def->next() != nullptr);
-      ASSERT(def->previous() != nullptr);
-    }
-    if (test_def) {
-      ASSERT(IsInDefUseList(def, instruction));  // proper def-use
-      ASSERT(instruction->IsPhi() ||
-             instruction->IsMaterializeObject() ||  // does not have dominance
-             instruction->IsDominatedBy(def));
+    VisitUseDef(instruction, instruction->InputAt(i), i, /*is_env*/ false);
+  }
+  // Check all environment inputs.
+  if (instruction->env() != nullptr) {
+    intptr_t i = 0;
+    for (Environment::DeepIterator it(instruction->env()); !it.Done();
+         it.Advance()) {
+      VisitUseDef(instruction, it.CurrentValue(), i++, /*is_env*/ true);
     }
   }
   // Visit specific instructions (definitions and anything with Visit()).
@@ -194,52 +233,123 @@
 }
 
 void FlowGraphChecker::VisitDefinition(Definition* def) {
-  // Make sure each outgoing use is dominated by this def, or is a
-  // Phi instruction (note that the proper dominance relation on
-  // the input values of Phis are checked by the Phi visitor below).
-  // Also verify some graph sanity.
+  // Used definitions must have an SSA name.
+  ASSERT(def->HasSSATemp() || def->input_use_list() == nullptr);
+  // Check all regular uses.
+  Value* prev = nullptr;
   for (Value* use = def->input_use_list(); use != nullptr;
        use = use->next_use()) {
-    ASSERT(use->definition() == def);  // proper use-def
-    Instruction* use_instr = use->instruction();
-    ASSERT(use_instr != nullptr);
-    if (use_instr->IsPhi()) {
-      ASSERT(use_instr->next() == nullptr);
-      ASSERT(use_instr->previous() == nullptr);
-      ASSERT(use_instr->GetBlock()->IsJoinEntry());
+    VisitDefUse(def, use, prev, /*is_env*/ false);
+    prev = use;
+  }
+  // Check all environment uses.
+  prev = nullptr;
+  for (Value* use = def->env_use_list(); use != nullptr;
+       use = use->next_use()) {
+    VisitDefUse(def, use, prev, /*is_env*/ true);
+    prev = use;
+  }
+}
+
+void FlowGraphChecker::VisitUseDef(Instruction* instruction,
+                                   Value* use,
+                                   intptr_t index,
+                                   bool is_env) {
+  ASSERT(use->instruction() == instruction);
+  ASSERT(use->use_index() == index);
+  // Get definition.
+  Definition* def = use->definition();
+  ASSERT(def != nullptr);
+  ASSERT(def != instruction || def->IsPhi() || def->IsMaterializeObject());
+  // 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()) {
+    test_def = false;  // TODO(dartbug.com/36895) fix this bail-out
+  } else {
+    if (is_env) return;  // TODO(dartbug.com/36893)
+    // Others are fully linked into graph.
+    ASSERT(def->next() != nullptr);
+    ASSERT(def->previous() != nullptr);
+  }
+  if (test_def) {
+    ASSERT(DefDominatesUse(def, instruction));
+    if (is_env) {
+      ASSERT(IsInUseList(def->env_use_list(), instruction));
     } else {
-      ASSERT(IsControlFlow(use_instr) || use_instr->next() != nullptr);
-      ASSERT(use_instr->previous() != nullptr);
-      ASSERT(use_instr->IsMaterializeObject() ||  // does not have dominance
-             use_instr->IsDominatedBy(def));
+      ASSERT(IsInUseList(def->input_use_list(), instruction));
     }
   }
 }
 
+void FlowGraphChecker::VisitDefUse(Definition* def,
+                                   Value* use,
+                                   Value* prev,
+                                   bool is_env) {
+  ASSERT(use->definition() == def);
+  ASSERT(use->previous_use() == prev);
+  // Get using instruction.
+  Instruction* instruction = use->instruction();
+  ASSERT(instruction != nullptr);
+  ASSERT(def != instruction || def->IsPhi() || def->IsMaterializeObject());
+  if (is_env) {
+    ASSERT(instruction->env()->ValueAtUseIndex(use->use_index()) == use);
+  } else {
+    ASSERT(instruction->InputAt(use->use_index()) == use);
+  }
+  // Make sure each use appears in the graph and is properly dominated
+  // by the defintion (note that the proper dominance relation on the
+  // input values of Phis is checked by the Phi visitor below).
+  if (instruction->IsPhi()) {
+    ASSERT(instruction->AsPhi()->is_alive());
+    ASSERT(instruction->GetBlock()->IsJoinEntry());
+    // Phis are never linked into graph.
+    ASSERT(instruction->next() == nullptr);
+    ASSERT(instruction->previous() == nullptr);
+  } else if (instruction->IsBlockEntry()) {
+    // BlockEntry instructions have environments attached to them but
+    // have no reliable way to verify if they are still in the graph.
+    ASSERT(is_env);
+  } else {
+    // Others are fully linked into graph.
+    ASSERT(IsControlFlow(instruction) || instruction->next() != nullptr);
+    ASSERT(instruction->previous() != nullptr);
+    ASSERT(is_env ||  // TODO(dartbug.com/36899)
+           DefDominatesUse(def, instruction));
+  }
+}
+
 void FlowGraphChecker::VisitConstant(ConstantInstr* constant) {
+  // Range check on smi.
   const Object& value = constant->value();
   if (value.IsSmi()) {
     const int64_t smi_value = Integer::Cast(value).AsInt64Value();
     ASSERT(kSmiMin <= smi_value);
     ASSERT(smi_value <= kSmiMax);
   }
-  // TODO(ajcbik): Is this a property we eventually want (all constants
-  // generated by utility that queries pool and put in the graph entry
-  // when seen first)? The inliner still creates some direct constants.
+  // Any constant involved in SSA should appear in the entry (making it more
+  // likely it was inserted by the utility that avoids duplication).
+  //
+  // TODO(dartbug.com/36894)
+  //
   // ASSERT(constant->GetBlock() == flow_graph_->graph_entry());
 }
 
 void FlowGraphChecker::VisitPhi(PhiInstr* phi) {
-  // Phis are never linked into graph.
-  ASSERT(phi->next() == nullptr);
-  ASSERT(phi->previous() == nullptr);
   // Make sure each incoming input value of a Phi is dominated
   // on the corresponding incoming edge, as defined by order.
   ASSERT(phi->InputCount() == current_block_->PredecessorCount());
   for (intptr_t i = 0, n = phi->InputCount(); i < n; ++i) {
     Definition* input_def = phi->InputAt(i)->definition();
     BlockEntryInstr* edge = current_block_->PredecessorAt(i);
-    ASSERT(input_def->IsConstant() ||  // some constants are in initial defs
+    ASSERT(input_def->IsConstant() ||  // TODO(dartbug.com/36894 and 36895)
            edge->last_instruction()->IsDominatedBy(input_def));
   }
 }
@@ -256,8 +366,15 @@
   ASSERT(branch->SuccessorCount() == 2);
 }
 
+void FlowGraphChecker::VisitRedefinition(RedefinitionInstr* def) {
+  ASSERT(def->value()->definition() != def);
+}
+
 // Main entry point of graph checker.
-void FlowGraphChecker::Check() {
+void FlowGraphChecker::Check(const char* pass_name) {
+  if (FLAG_trace_compiler) {
+    THR_Print("Running checker after %s\n", pass_name);
+  }
   ASSERT(flow_graph_ != nullptr);
   VisitBlocks();
 }
diff --git a/runtime/vm/compiler/backend/flow_graph_checker.h b/runtime/vm/compiler/backend/flow_graph_checker.h
index fba159f..088ad3e 100644
--- a/runtime/vm/compiler/backend/flow_graph_checker.h
+++ b/runtime/vm/compiler/backend/flow_graph_checker.h
@@ -35,7 +35,7 @@
         current_block_(nullptr) {}
 
   // Performs a sanity check on the flow graph.
-  void Check();
+  void Check(const char* pass_name);
 
  private:
   // Custom-made visitors.
@@ -43,6 +43,11 @@
   void VisitInstructions(BlockEntryInstr* block);
   void VisitInstruction(Instruction* instruction);
   void VisitDefinition(Definition* def);
+  void VisitUseDef(Instruction* instruction,
+                   Value* use,
+                   intptr_t index,
+                   bool is_env);
+  void VisitDefUse(Definition* def, Value* use, Value* prev, bool is_env);
 
   // Instruction visitors.
   void VisitConstant(ConstantInstr* constant) override;
@@ -50,6 +55,7 @@
   void VisitGoto(GotoInstr* jmp) override;
   void VisitIndirectGoto(IndirectGotoInstr* jmp) override;
   void VisitBranch(BranchInstr* branch) override;
+  void VisitRedefinition(RedefinitionInstr* def) override;
 
   FlowGraph* const flow_graph_;
   BlockEntryInstr* current_block_;
diff --git a/runtime/vm/compiler/backend/inliner.cc b/runtime/vm/compiler/backend/inliner.cc
index 634729f8..d3b03a9 100644
--- a/runtime/vm/compiler/backend/inliner.cc
+++ b/runtime/vm/compiler/backend/inliner.cc
@@ -101,7 +101,6 @@
 DECLARE_FLAG(int, max_deoptimization_counter_threshold);
 DECLARE_FLAG(bool, print_flow_graph);
 DECLARE_FLAG(bool, print_flow_graph_optimized);
-DECLARE_FLAG(bool, verify_compiler);
 
 // Quick access to the current zone.
 #define Z (zone())
@@ -738,9 +737,6 @@
       }
     }
   }
-
-  // Check that inlining maintains use lists.
-  DEBUG_ASSERT(!FLAG_verify_compiler || caller_graph->VerifyUseLists());
 }
 
 class CallSiteInliner : public ValueObject {
@@ -1062,11 +1058,9 @@
             entry_kind == Code::EntryKind::kUnchecked);
         {
           callee_graph = builder.BuildGraph();
-
 #if defined(DEBUG)
-          FlowGraphChecker(callee_graph).Check();
+          FlowGraphChecker(callee_graph).Check("Builder (callee)");
 #endif
-
           CalleeGraphValidator::Validate(callee_graph);
         }
 #if defined(DART_PRECOMPILER) && !defined(TARGET_ARCH_DBC) &&                  \
@@ -1149,7 +1143,9 @@
           // Compute SSA on the callee graph, catching bailouts.
           callee_graph->ComputeSSA(caller_graph_->max_virtual_register_number(),
                                    param_stubs);
-          DEBUG_ASSERT(callee_graph->VerifyUseLists());
+#if defined(DEBUG)
+          FlowGraphChecker(callee_graph).Check("SSA (callee)");
+#endif
         }
 
         if (FLAG_support_il_printer && trace_inlining() &&
@@ -4148,12 +4144,16 @@
             FunctionEntryInstr(graph_entry, flow_graph->allocate_block_id(),
                                call->GetBlock()->try_index(), DeoptId::kNone);
         (*entry)->InheritDeoptTarget(Z, call);
-        *last = new (Z) ConstantInstr(type);
+        ConstantInstr* ctype = flow_graph->GetConstant(type);
+        // Create a synthetic (re)definition for return to flag insertion.
+        // TODO(ajcbik): avoid this mechanism altogether
+        RedefinitionInstr* redef =
+            new (Z) RedefinitionInstr(new (Z) Value(ctype));
         flow_graph->AppendTo(
-            *entry, *last,
+            *entry, redef,
             call->deopt_id() != DeoptId::kNone ? call->env() : NULL,
             FlowGraph::kValue);
-        *result = (*last)->AsDefinition();
+        *last = *result = redef;
         return true;
       }
       return false;
diff --git a/runtime/vm/compiler/backend/range_analysis.cc b/runtime/vm/compiler/backend/range_analysis.cc
index c2bd37b..d619f96 100644
--- a/runtime/vm/compiler/backend/range_analysis.cc
+++ b/runtime/vm/compiler/backend/range_analysis.cc
@@ -1679,7 +1679,6 @@
                 replacement->ToCString());
     }
     defn->ReplaceWith(replacement, NULL);
-    ASSERT(flow_graph_->VerifyUseLists());
   }
 }
 
diff --git a/runtime/vm/compiler/compiler_pass.cc b/runtime/vm/compiler/compiler_pass.cc
index 6232f8f..340b149 100644
--- a/runtime/vm/compiler/compiler_pass.cc
+++ b/runtime/vm/compiler/compiler_pass.cc
@@ -178,7 +178,7 @@
       repeat = DoBody(state);
       thread->CheckForSafepoint();
 #if defined(DEBUG)
-      FlowGraphChecker(state->flow_graph).Check();
+      FlowGraphChecker(state->flow_graph).Check(name());
 #endif
     }
     PrintGraph(state, kTraceAfter, round);
diff --git a/runtime/vm/compiler/jit/compiler.cc b/runtime/vm/compiler/jit/compiler.cc
index 2f7e059..e8543ae 100644
--- a/runtime/vm/compiler/jit/compiler.cc
+++ b/runtime/vm/compiler/jit/compiler.cc
@@ -84,10 +84,6 @@
             false,
             "Trace only optimizing compiler operations.");
 DEFINE_FLAG(bool, trace_bailout, false, "Print bailout from ssa compiler.");
-DEFINE_FLAG(bool,
-            verify_compiler,
-            false,
-            "Enable compiler verification assertions");
 
 DECLARE_FLAG(bool, enable_interpreter);
 DECLARE_FLAG(bool, huge_method_cutoff_in_code_size);