[turbofan] Change handling of empty basic blocks

This CL inserts NOP instructions a little bit earlier into empty
blocks; this ensures that instructions keep their initial position.

Bug: v8:7327
Change-Id: Idee5269f4fd7fc15c44bda83a2be74e8cff62df8
Reviewed-on: https://chromium-review.googlesource.com/1097078
Commit-Queue: Sigurd Schneider <sigurds@chromium.org>
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#53672}

src/compiler/instruction-selector.cc

@@ -1104,7 +1104,11 @@ void InstructionSelector::VisitBlock(BasicBlock* block) {
   // We're done with the block.
   InstructionBlock* instruction_block =
       sequence()->InstructionBlockAt(RpoNumber::FromInt(block->rpo_number()));
-  instruction_block->set_code_start(static_cast<int>(instructions_.size()));
+  if (current_num_instructions() == current_block_end) {
+    // Avoid empty block: insert a {kArchNop} instruction.
+    Emit(Instruction::New(sequence()->zone(), kArchNop));
+  }
+  instruction_block->set_code_start(current_num_instructions());
   instruction_block->set_code_end(current_block_end);
 
   current_block_ = nullptr;

src/compiler/instruction.cc

@@ -869,12 +869,8 @@ void InstructionSequence::StartBlock(RpoNumber rpo) {
 void InstructionSequence::EndBlock(RpoNumber rpo) {
   int end = static_cast<int>(instructions_.size());
   DCHECK_EQ(current_block_->rpo_number(), rpo);
-  if (current_block_->code_start() == end) {  // Empty block.  Insert a nop.
+  CHECK(current_block_->code_start() >= 0 &&
-    AddInstruction(Instruction::New(zone(), kArchNop));
+        current_block_->code_start() < end);
-    end = static_cast<int>(instructions_.size());
-  }
-  DCHECK(current_block_->code_start() >= 0 &&
-         current_block_->code_start() < end);
   current_block_->set_code_end(end);
   current_block_ = nullptr;
 }

test/cctest/compiler/test-instruction.cc

@@ -82,6 +82,11 @@ class InstructionTester : public HandleAndZoneScope {
     return code->AddInstruction(instr);
   }
 
+  int NewNop() {
+    TestInstr* instr = TestInstr::New(zone(), kArchNop);
+    return code->AddInstruction(instr);
+  }
+
   UnallocatedOperand Unallocated(int vreg) {
     return UnallocatedOperand(UnallocatedOperand::REGISTER_OR_SLOT, vreg);
   }
@@ -163,6 +168,7 @@ TEST(InstructionGetBasicBlock) {
   int i8 = R.NewInstr();
   R.code->EndBlock(R.RpoFor(b2));
   R.code->StartBlock(R.RpoFor(b3));
+  R.NewNop();
   R.code->EndBlock(R.RpoFor(b3));
 
   CHECK_EQ(b0, R.GetBasicBlock(i0));

test/cctest/compiler/test-jump-threading.cc

@@ -77,6 +77,10 @@ class TestCode : public HandleAndZoneScope {
   }
   void End() {
     Start();
+    int end = static_cast<int>(sequence_.instructions().size());
+    if (current_->code_start() == end) {  // Empty block.  Insert a nop.
+      sequence_.AddInstruction(Instruction::New(main_zone(), kArchNop));
+    }
     sequence_.EndBlock(current_->rpo_number());
     current_ = nullptr;
     rpo_number_ = RpoNumber::FromInt(rpo_number_.ToInt() + 1);

test/unittests/compiler/instruction-sequence-unittest.cc

@@ -159,6 +159,10 @@ Instruction* InstructionSequenceTest::EndBlock(BlockCompletion completion) {
   }
   completions_.push_back(completion);
   CHECK_NOT_NULL(current_block_);
+  int end = static_cast<int>(sequence()->instructions().size());
+  if (current_block_->code_start() == end) {  // Empty block.  Insert a nop.
+    sequence()->AddInstruction(Instruction::New(zone(), kArchNop));
+  }
   sequence()->EndBlock(current_block_->rpo_number());
   current_block_ = nullptr;
   return result;
@@ -507,6 +511,7 @@ void InstructionSequenceTest::WireBlocks() {
   CHECK(loop_blocks_.empty());
   // Wire in end block to look like a scheduler produced cfg.
   auto end_block = NewBlock();
+  Emit(kArchNop);
   current_block_ = nullptr;
   sequence()->EndBlock(end_block->rpo_number());
   size_t offset = 0;