From 1f44e7267ef86e80312397ba954217f960709799 Mon Sep 17 00:00:00 2001 From: machenbach Date: Thu, 18 Dec 2014 07:49:48 -0800 Subject: [PATCH] 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 | 16 ++++---- src/compiler/instruction.h | 34 ++++++++++++++-- src/compiler/register-allocator.cc | 13 ++---- test/cctest/compiler/test-instruction.cc | 47 ++++++++++++++++------ test/cctest/compiler/test-jump-threading.cc | 4 +- .../compiler/instruction-selector-unittest.cc | 10 ++--- test/unittests/compiler/move-optimizer-unittest.cc | 8 +++- 7 files changed, 90 insertions(+), 42 deletions(-) diff --git a/src/compiler/instruction.cc b/src/compiler/instruction.cc index ba72eda..f83cdeb 100644 --- a/src/compiler/instruction.cc +++ b/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(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(instructions_.size()); InstructionBlock* block = InstructionBlockAt(rpo); - if (block->code_start() == end) { // Empty block. Insert a nop. - AddInstruction(Instruction::New(zone(), kArchNop)); - end = static_cast(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(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()); diff --git a/src/compiler/instruction.h b/src/compiler/instruction.h index cd5be01..41cda82 100644 --- a/src/compiler/instruction.h +++ b/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(instr); + } + + static const BlockStartInstruction* cast(const Instruction* instr) { + DCHECK(instr->IsBlockStart()); + return static_cast(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(); } diff --git a/src/compiler/register-allocator.cc b/src/compiler/register-allocator.cc index 416b970..9eb4a47 100644 --- a/src/compiler/register-allocator.cc +++ b/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); diff --git a/test/cctest/compiler/test-instruction.cc b/test/cctest/compiler/test-instruction.cc index f1c9bcd..294812f 100644 --- a/test/cctest/compiler/test-instruction.cc +++ b/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]; diff --git a/test/cctest/compiler/test-jump-threading.cc b/test/cctest/compiler/test-jump-threading.cc index 4365170..74bf43d 100644 --- a/test/cctest/compiler/test-jump-threading.cc +++ b/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(sequence_.instructions().size()) - 2; + int index = static_cast(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(sequence_.instructions().size()) - 2; + int index = static_cast(sequence_.instructions().size()) - 1; sequence_.AddGapMove(index, ImmediateOperand::Create(11, main_zone()), RegisterOperand::Create(11, main_zone())); } diff --git a/test/unittests/compiler/instruction-selector-unittest.cc b/test/unittests/compiler/instruction-selector-unittest.cc index f71be3d..c79a9e4 100644 --- a/test/unittests/compiler/instruction-selector-unittest.cc +++ b/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()); diff --git a/test/unittests/compiler/move-optimizer-unittest.cc b/test/unittests/compiler/move-optimizer-unittest.cc index 2452d19..5b956f0 100644 --- a/test/unittests/compiler/move-optimizer-unittest.cc +++ b/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(); -- 2.7.4