[vm/compiler] Rename Comparison to Condition, introduce proper Comparison

Previously, ComparisonInstr base class represented arbitrary conditions
used in Branch, IfThenElse and CheckCondition instructions and included
subclasses TestInt, TestCids, TestRange and unary DoubleTestOp which are
not comparisons. So this refactoring renames ComparisonInstr to
ConditionInstr.

In addition, a new Comparison instruction is added as a base class for
StrictCompare, EqualityCompare and RelationalOp.

TEST=ci (pure refactoring)

Change-Id: Ic8756ee5913ff2bc974c95cea8004370c9f5527f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/393420
Reviewed-by: Slava Egorov <[email protected]>
Commit-Queue: Alexander Markov <[email protected]>

Author: alexmarkov

runtime/vm/compiler/aot/aot_call_specializer.cc

@@ -1127,7 +1127,7 @@ bool AotCallSpecializer::TryReplaceInstanceOfWithRangeCheck(
       new (Z) LoadClassIdInstr(new (Z) Value(left), kUnboxedUword);
   InsertBefore(call, load_cid, nullptr, FlowGraph::kValue);
 
-  ComparisonInstr* check_range;
+  ConditionInstr* check_range;
   if (lower_limit == upper_limit) {
     ConstantInstr* cid_constant = flow_graph()->GetConstant(
         Smi::Handle(Z, Smi::New(lower_limit)), kUnboxedUword);

runtime/vm/compiler/backend/block_builder.h

@@ -132,11 +132,11 @@ class BlockBuilder : public ValueObject {
     return AddUnboxInstr(rep, new Value(boxed), value_mode);
   }
 
-  BranchInstr* AddBranch(ComparisonInstr* comp,
+  BranchInstr* AddBranch(ConditionInstr* cond,
                          TargetEntryInstr* true_successor,
                          TargetEntryInstr* false_successor) {
     auto branch =
-        new BranchInstr(comp, CompilerState::Current().GetNextDeoptId());
+        new BranchInstr(cond, CompilerState::Current().GetNextDeoptId());
     // Some graph transformations use environments from branches.
     branch->SetEnvironment(dummy_env_);
     current_->AppendInstruction(branch);

runtime/vm/compiler/backend/branch_optimizer.cc

@@ -30,27 +30,27 @@ static bool PhiHasSingleUse(PhiInstr* phi, Value* use) {
 }
 
 bool BranchSimplifier::Match(JoinEntryInstr* block) {
-  // Match the pattern of a branch on a comparison whose left operand is a
+  // Match the pattern of a branch on a condition whose left operand is a
   // phi from the same block, and whose right operand is a constant.
   //
-  //   Branch(Comparison(kind, Phi, Constant))
+  //   Branch(Condition(kind, Phi, Constant))
   //
   // These are the branches produced by inlining in a test context.  Also,
   // the phi has no other uses so they can simply be eliminated.  The block
   // has no other phis and no instructions intervening between the phi and
   // branch so the block can simply be eliminated.
   BranchInstr* branch = block->last_instruction()->AsBranch();
   ASSERT(branch != nullptr);
-  ComparisonInstr* comparison = branch->comparison();
-  if (comparison->InputCount() != 2) {
+  ConditionInstr* condition = branch->condition();
+  if (condition->InputCount() != 2) {
     return false;
   }
-  if (comparison->CanDeoptimize() || comparison->MayThrow()) {
+  if (condition->CanDeoptimize() || condition->MayThrow()) {
     return false;
   }
-  Value* left = comparison->left();
+  Value* left = condition->InputAt(0);
   PhiInstr* phi = left->definition()->AsPhi();
-  Value* right = comparison->right();
+  Value* right = condition->InputAt(1);
   ConstantInstr* constant =
       (right == nullptr) ? nullptr : right->definition()->AsConstant();
   return (phi != nullptr) && (constant != nullptr) &&
@@ -87,11 +87,11 @@ BranchInstr* BranchSimplifier::CloneBranch(Zone* zone,
                                            BranchInstr* branch,
                                            Value* new_left,
                                            Value* new_right) {
-  ComparisonInstr* comparison = branch->comparison();
-  ComparisonInstr* new_comparison =
-      comparison->CopyWithNewOperands(new_left, new_right);
+  ConditionInstr* condition = branch->condition();
+  ConditionInstr* new_condition =
+      condition->CopyWithNewOperands(new_left, new_right);
   BranchInstr* new_branch =
-      new (zone) BranchInstr(new_comparison, DeoptId::kNone);
+      new (zone) BranchInstr(new_condition, DeoptId::kNone);
   return new_branch;
 }
 
@@ -140,9 +140,10 @@ void BranchSimplifier::Simplify(FlowGraph* flow_graph) {
       JoinEntryInstr* join_true = ToJoinEntry(zone, branch->true_successor());
       JoinEntryInstr* join_false = ToJoinEntry(zone, branch->false_successor());
 
-      ComparisonInstr* comparison = branch->comparison();
-      PhiInstr* phi = comparison->left()->definition()->AsPhi();
-      ConstantInstr* constant = comparison->right()->definition()->AsConstant();
+      ConditionInstr* condition = branch->condition();
+      PhiInstr* phi = condition->InputAt(0)->definition()->AsPhi();
+      ConstantInstr* constant =
+          condition->InputAt(1)->definition()->AsConstant();
       ASSERT(constant != nullptr);
       // Copy the constant and branch and push it to all the predecessors.
       for (intptr_t i = 0, count = block->PredecessorCount(); i < count; ++i) {
@@ -161,10 +162,10 @@ void BranchSimplifier::Simplify(FlowGraph* flow_graph) {
         } else {
           // Take the environment from the branch if it has one.
           new_branch->InheritDeoptTarget(zone, branch);
-          // InheritDeoptTarget gave the new branch's comparison the same
+          // InheritDeoptTarget gave the new branch's condition the same
           // deopt id that it gave the new branch.  The id should be the
-          // deopt id of the original comparison.
-          new_branch->comparison()->SetDeoptId(*comparison);
+          // deopt id of the original condition.
+          new_branch->condition()->SetDeoptId(*condition);
           // The phi can be used in the branch's environment.  Rename such
           // uses.
           Definition* replacement = phi->InputAt(i)->definition();
@@ -249,11 +250,11 @@ void IfConverter::Simplify(FlowGraph* flow_graph) {
     JoinEntryInstr* join = block->AsJoinEntry();
 
     // Detect diamond control flow pattern which materializes a value depending
-    // on the result of the comparison:
+    // on the result of the condition:
     //
     // B_pred:
     //   ...
-    //   Branch if COMP goto (B_pred1, B_pred2)
+    //   Branch if COND goto (B_pred1, B_pred2)
     // B_pred1: -- trivial block that contains at most one definition
     //   v1 = Constant(...)
     //   goto B_block
@@ -266,7 +267,7 @@ void IfConverter::Simplify(FlowGraph* flow_graph) {
     // and replace it with
     //
     // Ba:
-    //   v3 = IfThenElse(COMP ? v1 : v2)
+    //   v3 = IfThenElse(COND ? v1 : v2)
     //
     if ((join != nullptr) && (join->phis() != nullptr) &&
         (join->phis()->length() == 1) && (block->PredecessorCount() == 2)) {
@@ -291,19 +292,20 @@ void IfConverter::Simplify(FlowGraph* flow_graph) {
           continue;
         }
 
-        ComparisonInstr* comparison = branch->comparison();
+        ConditionInstr* condition = branch->condition();
 
         // Check if the platform supports efficient branchless IfThenElseInstr
-        // for the given combination of comparison and values flowing from
+        // for the given combination of condition and values flowing from
         // false and true paths.
-        if (IfThenElseInstr::Supports(comparison, v1, v2)) {
+        if (IfThenElseInstr::Supports(condition, v1, v2)) {
           Value* if_true = (pred1 == branch->true_successor()) ? v1 : v2;
           Value* if_false = (pred2 == branch->true_successor()) ? v1 : v2;
 
-          ComparisonInstr* new_comparison = comparison->CopyWithNewOperands(
-              comparison->left()->Copy(zone), comparison->right()->Copy(zone));
+          ConditionInstr* new_condition =
+              condition->CopyWithNewOperands(condition->InputAt(0)->Copy(zone),
+                                             condition->InputAt(1)->Copy(zone));
           IfThenElseInstr* if_then_else =
-              new (zone) IfThenElseInstr(new_comparison, if_true->Copy(zone),
+              new (zone) IfThenElseInstr(new_condition, if_true->Copy(zone),
                                          if_false->Copy(zone), DeoptId::kNone);
           flow_graph->InsertBefore(branch, if_then_else, nullptr,
                                    FlowGraph::kValue);

runtime/vm/compiler/backend/constant_propagator.cc

@@ -239,7 +239,7 @@ void ConstantPropagator::VisitIndirectGoto(IndirectGotoInstr* instr) {
 }
 
 void ConstantPropagator::VisitBranch(BranchInstr* instr) {
-  instr->comparison()->Accept(this);
+  instr->condition()->Accept(this);
 
   // The successors may be reachable, but only if this instruction is.  (We
   // might be analyzing it because the constant value of one of its inputs
@@ -250,7 +250,7 @@ void ConstantPropagator::VisitBranch(BranchInstr* instr) {
              (instr->constant_target() == instr->false_successor()));
       SetReachable(instr->constant_target());
     } else {
-      const Object& value = instr->comparison()->constant_value();
+      const Object& value = instr->condition()->constant_value();
       if (IsNonConstant(value)) {
         SetReachable(instr->true_successor());
         SetReachable(instr->false_successor());
@@ -557,8 +557,8 @@ void ConstantPropagator::VisitStoreLocal(StoreLocalInstr* instr) {
 }
 
 void ConstantPropagator::VisitIfThenElse(IfThenElseInstr* instr) {
-  instr->comparison()->Accept(this);
-  const Object& value = instr->comparison()->constant_value();
+  instr->condition()->Accept(this);
+  const Object& value = instr->condition()->constant_value();
   ASSERT(!value.IsNull());
   if (IsUnknown(value)) {
     return;
@@ -656,8 +656,6 @@ static bool CompareIntegers(Token::Kind kind,
   }
 }
 
-// Comparison instruction that is equivalent to the (left & right) == 0
-// comparison pattern.
 void ConstantPropagator::VisitTestInt(TestIntInstr* instr) {
   const Object& left = instr->left()->definition()->constant_value();
   const Object& right = instr->right()->definition()->constant_value();
@@ -1614,10 +1612,11 @@ static RedefinitionInstr* InsertRedefinition(FlowGraph* graph,
 void ConstantPropagator::InsertRedefinitionsAfterEqualityComparisons() {
   for (auto block : graph_->reverse_postorder()) {
     if (auto branch = block->last_instruction()->AsBranch()) {
-      auto comparison = branch->comparison();
-      if (comparison->IsStrictCompare() ||
-          (comparison->IsEqualityCompare() &&
-           comparison->operation_cid() != kDoubleCid)) {
+      auto comparison = branch->condition()->AsComparison();
+      if (comparison != nullptr &&
+          (comparison->IsStrictCompare() ||
+           (comparison->IsEqualityCompare() &&
+            comparison->operation_cid() != kDoubleCid))) {
         Value* value;
         ConstantInstr* constant_defn;
         if (comparison->IsComparisonWithConstant(&value, &constant_defn) &&

runtime/vm/compiler/backend/flow_graph.cc

@@ -2818,9 +2818,9 @@ static GotoInstr* NewGoto(FlowGraph* graph,
 }
 
 static BranchInstr* NewBranch(FlowGraph* graph,
-                              ComparisonInstr* cmp,
+                              ConditionInstr* cond,
                               Instruction* inherit) {
-  BranchInstr* bra = new (graph->zone()) BranchInstr(cmp, DeoptId::kNone);
+  BranchInstr* bra = new (graph->zone()) BranchInstr(cond, DeoptId::kNone);
   bra->InheritDeoptTarget(graph->zone(), inherit);
   return bra;
 }
@@ -2841,7 +2841,7 @@ static BranchInstr* NewBranch(FlowGraph* graph,
 //
 JoinEntryInstr* FlowGraph::NewDiamond(Instruction* instruction,
                                       Instruction* inherit,
-                                      ComparisonInstr* compare,
+                                      ConditionInstr* condition,
                                       TargetEntryInstr** b_true,
                                       TargetEntryInstr** b_false) {
   BlockEntryInstr* entry = instruction->GetBlock();
@@ -2851,7 +2851,7 @@ JoinEntryInstr* FlowGraph::NewDiamond(Instruction* instruction,
   JoinEntryInstr* join = NewJoin(this, inherit);
   GotoInstr* gotot = NewGoto(this, join, inherit);
   GotoInstr* gotof = NewGoto(this, join, inherit);
-  BranchInstr* bra = NewBranch(this, compare, inherit);
+  BranchInstr* bra = NewBranch(this, condition, inherit);
 
   instruction->AppendInstruction(bra);
   entry->set_last_instruction(bra);

runtime/vm/compiler/backend/flow_graph.h

@@ -527,9 +527,9 @@ class FlowGraph : public ZoneAllocated {
 
   // Logical-AND (for use in short-circuit diamond).
   struct LogicalAnd {
-    LogicalAnd(ComparisonInstr* x, ComparisonInstr* y) : oper1(x), oper2(y) {}
-    ComparisonInstr* oper1;
-    ComparisonInstr* oper2;
+    LogicalAnd(ConditionInstr* x, ConditionInstr* y) : oper1(x), oper2(y) {}
+    ConditionInstr* oper1;
+    ConditionInstr* oper2;
   };
 
   // Constructs a diamond control flow at the instruction, inheriting
@@ -538,7 +538,7 @@ class FlowGraph : public ZoneAllocated {
   // relation, but not the succ/pred ordering on block.
   JoinEntryInstr* NewDiamond(Instruction* instruction,
                              Instruction* inherit,
-                             ComparisonInstr* compare,
+                             ConditionInstr* condition,
                              TargetEntryInstr** block_true,
                              TargetEntryInstr** block_false);
 

runtime/vm/compiler/backend/flow_graph_compiler.cc

@@ -229,7 +229,7 @@ void FlowGraphCompiler::InitCompiler() {
       for (ForwardInstructionIterator it(entry); !it.Done(); it.Advance()) {
         Instruction* current = it.Current();
         if (auto* branch = current->AsBranch()) {
-          current = branch->comparison();
+          current = branch->condition();
         }
         if (auto* instance_call = current->AsInstanceCall()) {
           const ICData* ic_data = instance_call->ic_data();