[turbofan] Fix phi-hinting problem with deferred blocks

Previously, turbofan selected the gap use from first predecessor block when
hinting a phi, unless that block was deferred, in which case the gap move from
the first non-deferred predecessor block was chosen.

This strategy didn't guarantee that an important invariant was maintained: the
predecessor blocks chosen for hinting phis must preceed the phi's block in the
rpo ordering. In most cases the strategy worked, since graphs generated by the
AstGraphBuilder and existing stubs just happened to always generate schedules
where this rpo ordering property for the first predecessor block, but it is
quite possible to generate a code stub by hand that doesn't have this property
(see included test case).

After this CL, the allocator chooses either the the first non-deferred
"rpo-preceeding" block to be the hinting block, or the first deferred
"rpo-preceeding" block if that doesn't exist. In all previously-existing code,
this behavior is the same as the original algorithm, but has the benefit of not
failing in the register allocator in hand-crafted stubs where all the
"rpo-preceeding" predecessors are all in deferred code.

Review-Url: https://codereview.chromium.org/2030463003
Cr-Commit-Position: refs/heads/master@{#36689}

src/compiler/register-allocator.cc

@@ -2184,23 +2184,24 @@ void LiveRangeBuilder::ProcessPhis(const InstructionBlock* block,
     // block.
     int phi_vreg = phi->virtual_register();
     live->Remove(phi_vreg);
-    InstructionOperand* hint = nullptr;
+    // Select the hint from the first predecessor block that preceeds this block
+    // in the rpo ordering. Prefer non-deferred blocks. The enforcement of
+    // hinting in rpo order is required because hint resolution that happens
+    // later in the compiler pipeline visits instructions in reverse rpo,
+    // relying on the fact that phis are encountered before their hints.
+    const Instruction* instr = nullptr;
     const InstructionBlock::Predecessors& predecessors = block->predecessors();
+    for (size_t i = 0; i < predecessors.size(); ++i) {
       const InstructionBlock* predecessor_block =
-        code()->InstructionBlockAt(predecessors[0]);
-    const Instruction* instr = GetLastInstruction(code(), predecessor_block);
-    if (predecessor_block->IsDeferred()) {
-      // "Prefer the hint from the first non-deferred predecessor, if any.
-      for (size_t i = 1; i < predecessors.size(); ++i) {
-        predecessor_block = code()->InstructionBlockAt(predecessors[i]);
-        if (!predecessor_block->IsDeferred()) {
+          code()->InstructionBlockAt(predecessors[i]);
+      if (predecessor_block->rpo_number() < block->rpo_number()) {
         instr = GetLastInstruction(code(), predecessor_block);
-          break;
-        }
+        if (!predecessor_block->IsDeferred()) break;
       }
     }
     DCHECK_NOT_NULL(instr);
 
+    InstructionOperand* hint = nullptr;
     for (MoveOperands* move : *instr->GetParallelMove(Instruction::END)) {
       InstructionOperand& to = move->destination();
       if (to.IsUnallocated() &&

test/cctest/test-code-stub-assembler.cc

@@ -1065,5 +1065,30 @@ TEST(TryLookupElement) {
   }
 }
 
+TEST(DeferredCodePhiHints) {
+  typedef compiler::Node Node;
+  typedef CodeStubAssembler::Label Label;
+  typedef CodeStubAssembler::Variable Variable;
+  Isolate* isolate(CcTest::InitIsolateOnce());
+  VoidDescriptor descriptor(isolate);
+  CodeStubAssemblerTester m(isolate, descriptor);
+  Label block1(&m, Label::kDeferred);
+  m.Goto(&block1);
+  m.Bind(&block1);
+  {
+    Variable var_object(&m, MachineRepresentation::kTagged);
+    Label loop(&m, &var_object);
+    var_object.Bind(m.IntPtrConstant(0));
+    m.Goto(&loop);
+    m.Bind(&loop);
+    {
+      Node* map = m.LoadMap(var_object.value());
+      var_object.Bind(map);
+      m.Goto(&loop);
+    }
+  }
+  CHECK(!m.GenerateCode().is_null());
+}
+
 }  // namespace internal
 }  // namespace v8