[regalloc] Validator: handle aliasing first class.

The validator was trying to finalize virtual register assignments in
phi cases, however, since phis may create aliases, we ended up with
an unnecessarily complex design that was the source of pretty much all
validator bugs since its introduction.

This change embraces the fact that phis may create aliases: pending
assessments (==phis) carry a bag of aliased virtual registers. 

Bug: chromium:758778
Change-Id: Ib7ded350a726fbc77e9d0ff3eeda7f00acc4de13
Reviewed-on: https://chromium-review.googlesource.com/639530
Commit-Queue: Mircea Trofin <mtrofin@chromium.org>
Reviewed-by: Brad Nelson <bradnelson@chromium.org>
Reviewed-by: Bill Budge <bbudge@chromium.org>
Cr-Commit-Position: refs/heads/master@{#47657}

src/compiler/register-allocator-verifier.cc

@@ -359,7 +359,7 @@ BlockAssessments* RegisterAllocatorVerifier::CreateForBlock(
         InstructionOperand operand = pair.first;
         if (ret->map().find(operand) == ret->map().end()) {
           ret->map().insert(std::make_pair(
-              operand, new (zone()) PendingAssessment(block, operand)));
+              operand, new (zone()) PendingAssessment(zone(), block, operand)));
         }
       }
     }
@@ -369,13 +369,14 @@ BlockAssessments* RegisterAllocatorVerifier::CreateForBlock(
 
 void RegisterAllocatorVerifier::ValidatePendingAssessment(
     RpoNumber block_id, InstructionOperand op,
-    BlockAssessments* current_assessments, const PendingAssessment* assessment,
-    int virtual_register) {
+    const BlockAssessments* current_assessments,
+    PendingAssessment* const assessment, int virtual_register) {
+  if (assessment->IsAliasOf(virtual_register)) return;
+
   // When validating a pending assessment, it is possible some of the
-  // assessments
-  // for the original operand (the one where the assessment was created for
-  // first) are also pending. To avoid recursion, we use a work list. To
-  // deal with cycles, we keep a set of seen nodes.
+  // assessments for the original operand (the one where the assessment was
+  // created for first) are also pending. To avoid recursion, we use a work
+  // list. To deal with cycles, we keep a set of seen nodes.
   Zone local_zone(zone()->allocator(), ZONE_NAME);
   ZoneQueue<std::pair<const PendingAssessment*, int>> worklist(&local_zone);
   ZoneSet<RpoNumber> seen(&local_zone);
@@ -433,9 +434,8 @@ void RegisterAllocatorVerifier::ValidatePendingAssessment(
 
       switch (contribution->kind()) {
         case Final:
-          ValidateFinalAssessment(
-              block_id, current_operand, current_assessments,
-              FinalAssessment::cast(contribution), expected);
+          CHECK_EQ(FinalAssessment::cast(contribution)->virtual_register(),
+                   expected);
           break;
         case Pending: {
           // This happens if we have a diamond feeding into another one, and
@@ -448,42 +448,13 @@ void RegisterAllocatorVerifier::ValidatePendingAssessment(
           // Note that we do not want to finalize pending assessments at the
           // beginning of a block - which is the information we'd have
           // available here. This is because this operand may be reused to
-          // define
-          // duplicate phis.
+          // define duplicate phis.
           break;
         }
       }
     }
   }
-  // If everything checks out, we may make the assessment, unless the operand
-  // was redefined in this block: it was initially used for the phi, but then
-  // also used as an output operand.
-  if (current_assessments->map()[op] == nullptr ||
-      current_assessments->map()[op]->kind() == Pending) {
-    current_assessments->map()[op] =
-        new (zone()) FinalAssessment(virtual_register, assessment);
-  }
-}
-
-void RegisterAllocatorVerifier::ValidateFinalAssessment(
-    RpoNumber block_id, InstructionOperand op,
-    BlockAssessments* current_assessments, const FinalAssessment* assessment,
-    int virtual_register) {
-  if (assessment->virtual_register() == virtual_register) return;
-  // If we have 2 phis with the exact same operand list, and the first phi is
-  // used before the second one, via the operand incoming to the block,
-  // and the second one's operand is defined (via a parallel move) after the
-  // use, then the original operand will be assigned to the first phi. We
-  // then look at the original pending assessment to ascertain if op
-  // is virtual_register.
-  const PendingAssessment* old = assessment->original_pending_assessment();
-  CHECK_NOT_NULL(old);
-  RpoNumber old_block = old->origin()->rpo_number();
-  DCHECK_LE(old_block, block_id);
-  BlockAssessments* old_block_assessments =
-      old_block == block_id ? current_assessments : assessments_[old_block];
-  ValidatePendingAssessment(old_block, op, old_block_assessments, old,
-                            virtual_register);
+  assessment->AddAlias(virtual_register);
 }
 
 void RegisterAllocatorVerifier::ValidateUse(
@@ -496,12 +467,11 @@ void RegisterAllocatorVerifier::ValidateUse(
 
   switch (assessment->kind()) {
     case Final:
-      ValidateFinalAssessment(block_id, op, current_assessments,
-                              FinalAssessment::cast(assessment),
-                              virtual_register);
+      CHECK_EQ(FinalAssessment::cast(assessment)->virtual_register(),
+               virtual_register);
       break;
     case Pending: {
-      const PendingAssessment* pending = PendingAssessment::cast(assessment);
+      PendingAssessment* pending = PendingAssessment::cast(assessment);
       ValidatePendingAssessment(block_id, op, current_assessments, pending,
                                 virtual_register);
       break;
@@ -571,17 +541,13 @@ void RegisterAllocatorVerifier::VerifyGapMoves() {
       CHECK(found_op != block_assessments->map().end());
       switch (found_op->second->kind()) {
         case Final:
-          ValidateFinalAssessment(block->rpo_number(), op, block_assessments,
-                                  FinalAssessment::cast(found_op->second),
-                                  vreg);
+          CHECK_EQ(FinalAssessment::cast(found_op->second)->virtual_register(),
+                   vreg);
           break;
         case Pending:
-          const PendingAssessment* pending =
-              PendingAssessment::cast(found_op->second);
           ValidatePendingAssessment(block->rpo_number(), op, block_assessments,
-                                    pending, vreg);
-          block_assessments->map()[op] =
-              new (zone()) FinalAssessment(vreg, pending);
+                                    PendingAssessment::cast(found_op->second),
+                                    vreg);
           break;
       }
     }

src/compiler/register-allocator-verifier.h

@@ -36,7 +36,8 @@ class InstructionSequence;
 // site matches the definition of this pending operand: either the phi inputs
 // match, or, if it's not a phi, all the predecessors at the point the pending
 // assessment was defined have that operand assigned to the given virtual
-// register.
+// register. If all checks out, we record in the assessment that the virtual
+// register is aliased by the specific operand.
 // If a block is a loop header - so one or more of its predecessors are it or
 // below - we still treat uses of operands as above, but we record which operand
 // assessments haven't been made yet, and what virtual register they must
@@ -66,26 +67,38 @@ class Assessment : public ZoneObject {
 // PendingAssessments are associated to operands coming from the multiple
 // predecessors of a block. We only record the operand and the block, and
 // will determine if the way the operand is defined (from the predecessors)
-// matches a particular use. This handles scenarios where multiple phis are
+// matches a particular use. We allow more than one vreg association with
+// an operand - this handles scenarios where multiple phis are
 // defined with identical operands, and the move optimizer moved down the moves
 // separating the 2 phis in the block defining them.
 class PendingAssessment final : public Assessment {
  public:
-  explicit PendingAssessment(const InstructionBlock* origin,
+  explicit PendingAssessment(Zone* zone, const InstructionBlock* origin,
                              InstructionOperand operand)
-      : Assessment(Pending), origin_(origin), operand_(operand) {}
+      : Assessment(Pending),
+        origin_(origin),
+        operand_(operand),
+        aliases_(zone) {}
 
   static const PendingAssessment* cast(const Assessment* assessment) {
     CHECK(assessment->kind() == Pending);
     return static_cast<const PendingAssessment*>(assessment);
   }
 
+  static PendingAssessment* cast(Assessment* assessment) {
+    CHECK(assessment->kind() == Pending);
+    return static_cast<PendingAssessment*>(assessment);
+  }
+
   const InstructionBlock* origin() const { return origin_; }
   InstructionOperand operand() const { return operand_; }
+  bool IsAliasOf(int vreg) const { return aliases_.count(vreg) > 0; }
+  void AddAlias(int vreg) { aliases_.insert(vreg); }
 
  private:
   const InstructionBlock* const origin_;
   InstructionOperand operand_;
+  ZoneSet<int> aliases_;
 
   DISALLOW_COPY_AND_ASSIGN(PendingAssessment);
 };
@@ -94,11 +107,8 @@ class PendingAssessment final : public Assessment {
 // virtual register.
 class FinalAssessment final : public Assessment {
  public:
-  explicit FinalAssessment(int virtual_register,
-                           const PendingAssessment* original_pending = nullptr)
-      : Assessment(Final),
-        virtual_register_(virtual_register),
-        original_pending_assessment_(original_pending) {}
+  explicit FinalAssessment(int virtual_register)
+      : Assessment(Final), virtual_register_(virtual_register) {}
 
   int virtual_register() const { return virtual_register_; }
   static const FinalAssessment* cast(const Assessment* assessment) {
@@ -106,13 +116,8 @@ class FinalAssessment final : public Assessment {
     return static_cast<const FinalAssessment*>(assessment);
   }
 
-  const PendingAssessment* original_pending_assessment() const {
-    return original_pending_assessment_;
-  }
-
  private:
   int virtual_register_;
-  const PendingAssessment* original_pending_assessment_;
 
   DISALLOW_COPY_AND_ASSIGN(FinalAssessment);
 };
@@ -240,14 +245,12 @@ class RegisterAllocatorVerifier final : public ZoneObject {
                        const OperandConstraint* constraint);
   BlockAssessments* CreateForBlock(const InstructionBlock* block);
 
+  // Prove that this operand is an alias of this virtual register in the given
+  // block. Update the assessment if that's the case.
   void ValidatePendingAssessment(RpoNumber block_id, InstructionOperand op,
-                                 BlockAssessments* current_assessments,
-                                 const PendingAssessment* assessment,
+                                 const BlockAssessments* current_assessments,
+                                 PendingAssessment* const assessment,
                                  int virtual_register);
-  void ValidateFinalAssessment(RpoNumber block_id, InstructionOperand op,
-                               BlockAssessments* current_assessments,
-                               const FinalAssessment* assessment,
-                               int virtual_register);
   void ValidateUse(RpoNumber block_id, BlockAssessments* current_assessments,
                    InstructionOperand op, int virtual_register);