[turbofan] Regalloc was assuming "blocked" register can't be "used"

src/compiler/register-allocator.cc

Index: src/compiler/register-allocator.cc
diff --git a/src/compiler/register-allocator.cc b/src/compiler/register-allocator.cc
index 36d670350a7d6f6726076a2b36752c2c68fbc141..99a1101583d23bbf68d787e0cf6bd515429822fe 100644
--- a/src/compiler/register-allocator.cc
+++ b/src/compiler/register-allocator.cc
@@ -3175,6 +3175,9 @@ void LinearScanAllocator::AllocateBlockedReg(LiveRange* current) {
                              rep == MachineRepresentation::kSimd128))
     GetFPRegisterSet(rep, &num_regs, &num_codes, &codes);
 
+  // use_pos keeps track of positions a register/alias is used at.
+  // block_pos keeps track of positions where a register/alias is blocked
+  // from.
   LifetimePosition use_pos[RegisterConfiguration::kMaxFPRegisters];
   LifetimePosition block_pos[RegisterConfiguration::kMaxFPRegisters];
   for (int i = 0; i < num_regs; i++) {
@@ -3190,8 +3193,9 @@ void LinearScanAllocator::AllocateBlockedReg(LiveRange* current) {
         block_pos[cur_reg] = use_pos[cur_reg] =
             LifetimePosition::GapFromInstructionIndex(0);
       } else {
-        use_pos[cur_reg] =
-            range->NextLifetimePositionRegisterIsBeneficial(current->Start());
+        use_pos[cur_reg] = Min(
+            block_pos[cur_reg],
+            range->NextLifetimePositionRegisterIsBeneficial(current->Start()));

[bbudge, 2017/01/18 00:45:13]

Is this necessary here, since there is either simple aliasing, or there is no FP
aliasing?

[Mircea Trofin, 2017/01/18 01:10:12]

It's not, but I'd prefer it be there for consistency with the rest of the code.

Alternatively, I could DCHECK there and also in the line above.

[bbudge, 2017/01/18 01:14:51]

DCHECK seems clearer to me than calling Min which always returns
NextLifetimePosition...

[Mircea Trofin, 2017/01/18 06:51:25]

Done.

       }
     } else {
       int alias_base_index = -1;
@@ -3205,7 +3209,9 @@ void LinearScanAllocator::AllocateBlockedReg(LiveRange* current) {
               LifetimePosition::GapFromInstructionIndex(0);
         } else {
           use_pos[aliased_reg] =
-              range->NextLifetimePositionRegisterIsBeneficial(current->Start());
+              Min(block_pos[aliased_reg],
+                  range->NextLifetimePositionRegisterIsBeneficial(
+                      current->Start()));
         }
       }
     }
@@ -3219,10 +3225,12 @@ void LinearScanAllocator::AllocateBlockedReg(LiveRange* current) {
     // Don't perform costly intersections if they are guaranteed to not update
     // block_pos or use_pos.
     // TODO(mtrofin): extend to aliased ranges, too.
-    if ((kSimpleFPAliasing || !check_fp_aliasing()) && is_fixed) {
-      if (block_pos[cur_reg] < range->Start()) continue;
-    } else {
-      if (use_pos[cur_reg] < range->Start()) continue;
+    if ((kSimpleFPAliasing || !check_fp_aliasing())) {
+      if (is_fixed) {
+        if (block_pos[cur_reg] < range->Start()) continue;
+      } else {
+        if (use_pos[cur_reg] < range->Start()) continue;
+      }
     }
 
     LifetimePosition next_intersection = range->FirstIntersection(current);