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

src/compiler/register-allocator.cc

 // Copyright 2014 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "src/base/adapters.h"
 #include "src/compiler/linkage.h"
 #include "src/compiler/register-allocator.h"
 #include "src/string-stream.h"
 
 namespace v8 {
@@ skipping 3157 matching lines @@
   }
 
   int num_regs = num_registers();
   int num_codes = num_allocatable_registers();
   const int* codes = allocatable_register_codes();
   MachineRepresentation rep = current->representation();
   if (!kSimpleFPAliasing && (rep == MachineRepresentation::kFloat32 ||
                              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++) {
     use_pos[i] = block_pos[i] = LifetimePosition::MaxPosition();
   }
 
   for (LiveRange* range : active_live_ranges()) {
     int cur_reg = range->assigned_register();
     bool is_fixed_or_cant_spill =
         range->TopLevel()->IsFixed() || !range->CanBeSpilled(current->Start());
     if (kSimpleFPAliasing || !check_fp_aliasing()) {
       if (is_fixed_or_cant_spill) {
         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;
       int aliases = data()->config()->GetAliases(
           range->representation(), cur_reg, rep, &alias_base_index);
       DCHECK(aliases > 0 || (aliases == 0 && alias_base_index == -1));
       while (aliases--) {
         int aliased_reg = alias_base_index + aliases;
         if (is_fixed_or_cant_spill) {
           block_pos[aliased_reg] = use_pos[aliased_reg] =
               LifetimePosition::GapFromInstructionIndex(0);
         } else {
           use_pos[aliased_reg] =
-              range->NextLifetimePositionRegisterIsBeneficial(current->Start());
+              Min(block_pos[aliased_reg],
+                  range->NextLifetimePositionRegisterIsBeneficial(
+                      current->Start()));
         }
       }
     }
   }
 
   for (LiveRange* range : inactive_live_ranges()) {
     DCHECK(range->End() > current->Start());
     int cur_reg = range->assigned_register();
     bool is_fixed = range->TopLevel()->IsFixed();
 
     // 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);
     if (!next_intersection.IsValid()) continue;
 
     if (kSimpleFPAliasing || !check_fp_aliasing()) {
       if (is_fixed) {
         block_pos[cur_reg] = Min(block_pos[cur_reg], next_intersection);
         use_pos[cur_reg] = Min(block_pos[cur_reg], use_pos[cur_reg]);
       } else {
@@ skipping 762 matching lines @@
         }
       }
     }
   }
 }
 
 
 }  // namespace compiler
 }  // namespace internal
 }  // namespace v8