Revert of [turbofan] simplify gap ordering (patchset #2 id:20001 of...

Revert of [turbofan] simplify gap ordering (patchset #2 id:20001 of https://codereview.chromium.org/810013002/)

Reason for revert:
Revert for breaking emscripten bullet with turbofan on android arm64.

Original issue's description:
> [turbofan] simplify gap ordering
>
> BUG=
>
> Committed: https://crrev.com/70b5eb47b39acbf31746f4a116a9b3ce2730218a
> Cr-Commit-Position: refs/heads/master@{#25865}

TBR=bmeurer@chromium.org,dcarney@chromium.org
NOTREECHECKS=true
NOTRY=true
BUG=

Review URL: https://codereview.chromium.org/815743002

Cr-Commit-Position: refs/heads/master@{#25888}

src/compiler/instruction.cc

@@ -287,7 +287,7 @@ std::ostream& operator<<(std::ostream& os,
 
   if (instr.IsGapMoves()) {
     const GapInstruction* gap = GapInstruction::cast(&instr);
-    os << "gap ";
+    os << (instr.IsBlockStart() ? " block-start" : "gap ");
     for (int i = GapInstruction::FIRST_INNER_POSITION;
          i <= GapInstruction::LAST_INNER_POSITION; i++) {
       os << "(";
@@ -458,10 +458,10 @@ InstructionSequence::InstructionSequence(Zone* instruction_zone,
 }
 
 
-GapInstruction* InstructionSequence::GetBlockStart(
+BlockStartInstruction* InstructionSequence::GetBlockStart(
     BasicBlock::RpoNumber rpo) const {
   const InstructionBlock* block = InstructionBlockAt(rpo);
-  return GapInstruction::cast(InstructionAt(block->code_start()));
+  return BlockStartInstruction::cast(InstructionAt(block->code_start()));
 }
 
 
@@ -471,26 +471,26 @@ void InstructionSequence::StartBlock(BasicBlock::RpoNumber rpo) {
   int code_start = static_cast<int>(instructions_.size());
   block->set_code_start(code_start);
   block_starts_.push_back(code_start);
+  BlockStartInstruction* block_start = BlockStartInstruction::New(zone());
+  AddInstruction(block_start);
 }
 
 
 void InstructionSequence::EndBlock(BasicBlock::RpoNumber rpo) {
   int end = static_cast<int>(instructions_.size());
   InstructionBlock* block = InstructionBlockAt(rpo);
-  if (block->code_start() == end) {  // Empty block.  Insert a nop.
-    AddInstruction(Instruction::New(zone(), kArchNop));
-    end = static_cast<int>(instructions_.size());
-  }
   DCHECK(block->code_start() >= 0 && block->code_start() < end);
   block->set_code_end(end);
 }
 
 
 int InstructionSequence::AddInstruction(Instruction* instr) {
+  // TODO(titzer): the order of these gaps is a holdover from Lithium.
   GapInstruction* gap = GapInstruction::New(zone());
-  instructions_.push_back(gap);
+  if (instr->IsControl()) instructions_.push_back(gap);
   int index = static_cast<int>(instructions_.size());
   instructions_.push_back(instr);
+  if (!instr->IsControl()) instructions_.push_back(gap);
   if (instr->NeedsPointerMap()) {
     DCHECK(instr->pointer_map() == NULL);
     PointerMap* pointer_map = new (zone()) PointerMap(zone());

src/compiler/instruction.h

@@ -25,7 +25,8 @@ namespace compiler {
 
 // A couple of reserved opcodes are used for internal use.
 const InstructionCode kGapInstruction = -1;
-const InstructionCode kSourcePositionInstruction = -2;
+const InstructionCode kBlockStartInstruction = -2;
+const InstructionCode kSourcePositionInstruction = -3;
 
 #define INSTRUCTION_OPERAND_LIST(V)                                  \
   V(Constant, CONSTANT, 0)                                           \
@@ -491,7 +492,10 @@ class Instruction : public ZoneObject {
   bool NeedsPointerMap() const { return IsCall(); }
   bool HasPointerMap() const { return pointer_map_ != NULL; }
 
-  bool IsGapMoves() const { return opcode() == kGapInstruction; }
+  bool IsGapMoves() const {
+    return opcode() == kGapInstruction || opcode() == kBlockStartInstruction;
+  }
+  bool IsBlockStart() const { return opcode() == kBlockStartInstruction; }
   bool IsSourcePosition() const {
     return opcode() == kSourcePositionInstruction;
   }
@@ -637,6 +641,30 @@ class GapInstruction : public Instruction {
 };
 
 
+// This special kind of gap move instruction represents the beginning of a
+// block of code.
+class BlockStartInstruction FINAL : public GapInstruction {
+ public:
+  static BlockStartInstruction* New(Zone* zone) {
+    void* buffer = zone->New(sizeof(BlockStartInstruction));
+    return new (buffer) BlockStartInstruction();
+  }
+
+  static BlockStartInstruction* cast(Instruction* instr) {
+    DCHECK(instr->IsBlockStart());
+    return static_cast<BlockStartInstruction*>(instr);
+  }
+
+  static const BlockStartInstruction* cast(const Instruction* instr) {
+    DCHECK(instr->IsBlockStart());
+    return static_cast<const BlockStartInstruction*>(instr);
+  }
+
+ private:
+  BlockStartInstruction() : GapInstruction(kBlockStartInstruction) {}
+};
+
+
 class SourcePositionInstruction FINAL : public Instruction {
  public:
   static SourcePositionInstruction* New(Zone* zone, SourcePosition position) {
@@ -953,7 +981,7 @@ class InstructionSequence FINAL : public ZoneObject {
 
   void AddGapMove(int index, InstructionOperand* from, InstructionOperand* to);
 
-  GapInstruction* GetBlockStart(BasicBlock::RpoNumber rpo) const;
+  BlockStartInstruction* GetBlockStart(BasicBlock::RpoNumber rpo) const;
 
   typedef InstructionDeque::const_iterator const_iterator;
   const_iterator begin() const { return instructions_.begin(); }

src/compiler/register-allocator.cc

@@ -1078,7 +1078,7 @@ void RegisterAllocator::MeetRegisterConstraintsForLastInstructionInBlock(
       for (auto succ : block->successors()) {
         const InstructionBlock* successor = code()->InstructionBlockAt(succ);
         DCHECK(successor->PredecessorCount() == 1);
-        int gap_index = successor->first_instruction_index();
+        int gap_index = successor->first_instruction_index() + 1;
         DCHECK(code()->IsGapAt(gap_index));
 
         // Create an unconstrained operand for the same virtual register
@@ -1095,7 +1095,7 @@ void RegisterAllocator::MeetRegisterConstraintsForLastInstructionInBlock(
       for (auto succ : block->successors()) {
         const InstructionBlock* successor = code()->InstructionBlockAt(succ);
         DCHECK(successor->PredecessorCount() == 1);
-        int gap_index = successor->first_instruction_index();
+        int gap_index = successor->first_instruction_index() + 1;
         range->SpillAtDefinition(local_zone(), gap_index, output);
         range->SetSpillStartIndex(gap_index);
       }
@@ -2376,8 +2376,7 @@ void RegisterAllocator::SplitAndSpillIntersecting(LiveRange* current) {
 
 bool RegisterAllocator::IsBlockBoundary(LifetimePosition pos) {
   return pos.IsInstructionStart() &&
-         code()->GetInstructionBlock(pos.InstructionIndex())->code_start() ==
-             pos.InstructionIndex();
+         InstructionAt(pos.InstructionIndex())->IsBlockStart();
 }
 
 
@@ -2475,13 +2474,9 @@ void RegisterAllocator::SpillBetweenUntil(LiveRange* range,
     // The split result intersects with [start, end[.
     // Split it at position between ]start+1, end[, spill the middle part
     // and put the rest to unhandled.
-    auto third_part_end = end.PrevInstruction().InstructionEnd();
-    if (IsBlockBoundary(end.InstructionStart())) {
-      third_part_end = end.InstructionStart();
-    }
     auto third_part = SplitBetween(
         second_part, Max(second_part->Start().InstructionEnd(), until),
-        third_part_end);
+        end.PrevInstruction().InstructionEnd());
     if (!AllocationOk()) return;
 
     DCHECK(third_part != second_part);

test/cctest/compiler/test-instruction.cc

@@ -214,10 +214,14 @@ TEST(InstructionIsGapAt) {
   R.code->AddInstruction(g);
   R.code->EndBlock(b0->GetRpoNumber());
 
-  CHECK(R.code->instructions().size() == 4);
-  for (size_t i = 0; i < R.code->instructions().size(); ++i) {
-    CHECK_EQ(i % 2 == 0, R.code->instructions()[i]->IsGapMoves());
-  }
+  CHECK_EQ(true, R.code->InstructionAt(0)->IsBlockStart());
+
+  CHECK_EQ(true, R.code->IsGapAt(0));   // Label
+  CHECK_EQ(true, R.code->IsGapAt(1));   // Gap
+  CHECK_EQ(false, R.code->IsGapAt(2));  // i0
+  CHECK_EQ(true, R.code->IsGapAt(3));   // Gap
+  CHECK_EQ(true, R.code->IsGapAt(4));   // Gap
+  CHECK_EQ(false, R.code->IsGapAt(5));  // g
 }
 
 
@@ -244,10 +248,23 @@ TEST(InstructionIsGapAt2) {
   R.code->AddInstruction(g1);
   R.code->EndBlock(b1->GetRpoNumber());
 
-  CHECK(R.code->instructions().size() == 8);
-  for (size_t i = 0; i < R.code->instructions().size(); ++i) {
-    CHECK_EQ(i % 2 == 0, R.code->instructions()[i]->IsGapMoves());
-  }
+  CHECK_EQ(true, R.code->InstructionAt(0)->IsBlockStart());
+
+  CHECK_EQ(true, R.code->IsGapAt(0));   // Label
+  CHECK_EQ(true, R.code->IsGapAt(1));   // Gap
+  CHECK_EQ(false, R.code->IsGapAt(2));  // i0
+  CHECK_EQ(true, R.code->IsGapAt(3));   // Gap
+  CHECK_EQ(true, R.code->IsGapAt(4));   // Gap
+  CHECK_EQ(false, R.code->IsGapAt(5));  // g
+
+  CHECK_EQ(true, R.code->InstructionAt(6)->IsBlockStart());
+
+  CHECK_EQ(true, R.code->IsGapAt(6));    // Label
+  CHECK_EQ(true, R.code->IsGapAt(7));    // Gap
+  CHECK_EQ(false, R.code->IsGapAt(8));   // i1
+  CHECK_EQ(true, R.code->IsGapAt(9));    // Gap
+  CHECK_EQ(true, R.code->IsGapAt(10));   // Gap
+  CHECK_EQ(false, R.code->IsGapAt(11));  // g1
 }
 
 
@@ -265,12 +282,16 @@ TEST(InstructionAddGapMove) {
   R.code->AddInstruction(g);
   R.code->EndBlock(b0->GetRpoNumber());
 
-  CHECK(R.code->instructions().size() == 4);
-  for (size_t i = 0; i < R.code->instructions().size(); ++i) {
-    CHECK_EQ(i % 2 == 0, R.code->instructions()[i]->IsGapMoves());
-  }
+  CHECK_EQ(true, R.code->InstructionAt(0)->IsBlockStart());
+
+  CHECK_EQ(true, R.code->IsGapAt(0));   // Label
+  CHECK_EQ(true, R.code->IsGapAt(1));   // Gap
+  CHECK_EQ(false, R.code->IsGapAt(2));  // i0
+  CHECK_EQ(true, R.code->IsGapAt(3));   // Gap
+  CHECK_EQ(true, R.code->IsGapAt(4));   // Gap
+  CHECK_EQ(false, R.code->IsGapAt(5));  // g
 
-  int indexes[] = {0, 2, -1};
+  int indexes[] = {0, 1, 3, 4, -1};
   for (int i = 0; indexes[i] >= 0; i++) {
     int index = indexes[i];
 

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

@@ -60,14 +60,14 @@ class TestCode : public HandleAndZoneScope {
   void RedundantMoves() {
     Start();
     sequence_.AddInstruction(Instruction::New(main_zone(), kArchNop));
-    int index = static_cast<int>(sequence_.instructions().size()) - 2;
+    int index = static_cast<int>(sequence_.instructions().size()) - 1;
     sequence_.AddGapMove(index, RegisterOperand::Create(13, main_zone()),
                          RegisterOperand::Create(13, main_zone()));
   }
   void NonRedundantMoves() {
     Start();
     sequence_.AddInstruction(Instruction::New(main_zone(), kArchNop));
-    int index = static_cast<int>(sequence_.instructions().size()) - 2;
+    int index = static_cast<int>(sequence_.instructions().size()) - 1;
     sequence_.AddGapMove(index, ImmediateOperand::Create(11, main_zone()),
                          RegisterOperand::Create(11, main_zone()));
   }

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

@@ -167,7 +167,7 @@ TARGET_TEST_F(InstructionSelectorTest, ReturnFloat32Constant) {
   StreamBuilder m(this, kMachFloat32);
   m.Return(m.Float32Constant(kValue));
   Stream s = m.Build(kAllInstructions);
-  ASSERT_EQ(3U, s.size());
+  ASSERT_EQ(2U, s.size());
   EXPECT_EQ(kArchNop, s[0]->arch_opcode());
   ASSERT_EQ(InstructionOperand::CONSTANT, s[0]->OutputAt(0)->kind());
   EXPECT_FLOAT_EQ(kValue, s.ToFloat32(s[0]->OutputAt(0)));
@@ -180,7 +180,7 @@ TARGET_TEST_F(InstructionSelectorTest, ReturnParameter) {
   StreamBuilder m(this, kMachInt32, kMachInt32);
   m.Return(m.Parameter(0));
   Stream s = m.Build(kAllInstructions);
-  ASSERT_EQ(3U, s.size());
+  ASSERT_EQ(2U, s.size());
   EXPECT_EQ(kArchNop, s[0]->arch_opcode());
   ASSERT_EQ(1U, s[0]->OutputCount());
   EXPECT_EQ(kArchRet, s[1]->arch_opcode());
@@ -192,7 +192,7 @@ TARGET_TEST_F(InstructionSelectorTest, ReturnZero) {
   StreamBuilder m(this, kMachInt32);
   m.Return(m.Int32Constant(0));
   Stream s = m.Build(kAllInstructions);
-  ASSERT_EQ(3U, s.size());
+  ASSERT_EQ(2U, s.size());
   EXPECT_EQ(kArchNop, s[0]->arch_opcode());
   ASSERT_EQ(1U, s[0]->OutputCount());
   EXPECT_EQ(InstructionOperand::CONSTANT, s[0]->OutputAt(0)->kind());
@@ -210,7 +210,7 @@ TARGET_TEST_F(InstructionSelectorTest, TruncateFloat64ToInt32WithParameter) {
   StreamBuilder m(this, kMachInt32, kMachFloat64);
   m.Return(m.TruncateFloat64ToInt32(m.Parameter(0)));
   Stream s = m.Build(kAllInstructions);
-  ASSERT_EQ(4U, s.size());
+  ASSERT_EQ(3U, s.size());
   EXPECT_EQ(kArchNop, s[0]->arch_opcode());
   EXPECT_EQ(kArchTruncateDoubleToI, s[1]->arch_opcode());
   EXPECT_EQ(1U, s[1]->InputCount());
@@ -251,7 +251,7 @@ TARGET_TEST_F(InstructionSelectorTest, Finish) {
   Node* finish = m.NewNode(m.common()->Finish(1), param, m.graph()->start());
   m.Return(finish);
   Stream s = m.Build(kAllInstructions);
-  ASSERT_EQ(4U, s.size());
+  ASSERT_EQ(3U, s.size());
   EXPECT_EQ(kArchNop, s[0]->arch_opcode());
   ASSERT_EQ(1U, s[0]->OutputCount());
   ASSERT_TRUE(s[0]->Output()->IsUnallocated());

test/unittests/compiler/move-optimizer-unittest.cc

@@ -12,7 +12,11 @@ namespace compiler {
 class MoveOptimizerTest : public InstructionSequenceTest {
  public:
   GapInstruction* LastGap() {
-    return GapInstruction::cast(*(sequence()->instructions().rbegin() + 1));
+    auto instruction = sequence()->instructions().back();
+    if (!instruction->IsGapMoves()) {
+      instruction = *(sequence()->instructions().rbegin() + 1);
+    }
+    return GapInstruction::cast(instruction);
   }
 
   void AddMove(GapInstruction* gap, TestOperand from, TestOperand to,
@@ -86,10 +90,10 @@ class MoveOptimizerTest : public InstructionSequenceTest {
 
 TEST_F(MoveOptimizerTest, RemovesRedundant) {
   StartBlock();
-  EmitNop();
   AddMove(LastGap(), Reg(0), Reg(1));
   EmitNop();
   AddMove(LastGap(), Reg(1), Reg(0));
+  EmitNop();
   EndBlock(Last());
 
   Optimize();