From 2093345fc9668b71bc7e62f7023449b7ed66a98b Mon Sep 17 00:00:00 2001 From: Hannes Verschore Date: Fri, 5 Apr 2013 17:23:52 +0200 Subject: [PATCH] Bug 844779: IonMonkey: Improve the order of blocks, r=jandem --- js/src/ion/AsmJS.cpp | 5 - js/src/ion/Ion.cpp | 17 +++- js/src/ion/IonAnalysis.cpp | 204 +++++++++++++++++++++++++++++++++++++ js/src/ion/IonAnalysis.h | 6 ++ js/src/ion/IonBuilder.cpp | 22 +--- js/src/ion/MIRGraph.h | 23 ++++- 6 files changed, 247 insertions(+), 30 deletions(-) diff --git a/js/src/ion/AsmJS.cpp b/js/src/ion/AsmJS.cpp index 115945a24985..0a38b24786a8 100644 --- a/js/src/ion/AsmJS.cpp +++ b/js/src/ion/AsmJS.cpp @@ -1915,7 +1915,6 @@ class FunctionCompiler joinBlock->addPredecessor(curBlock_); } curBlock_ = joinBlock; - mirGraph().moveBlockToEnd(curBlock_); } MBasicBlock *switchToElse(MBasicBlock *elseBlock) @@ -1924,7 +1923,6 @@ class FunctionCompiler return NULL; MBasicBlock *thenEnd = curBlock_; curBlock_ = elseBlock; - mirGraph().moveBlockToEnd(curBlock_); return thenEnd; } @@ -2031,8 +2029,6 @@ class FunctionCompiler loopEntry->setBackedge(curBlock_); } curBlock_ = afterLoop; - if (curBlock_) - mirGraph().moveBlockToEnd(curBlock_); return bindUnlabeledBreaks(pn); } @@ -2145,7 +2141,6 @@ class FunctionCompiler (*cases)[i] = bb; } } - mirGraph().moveBlockToEnd(*defaultBlock); return true; } diff --git a/js/src/ion/Ion.cpp b/js/src/ion/Ion.cpp index 5fea7ff021c9..0152cd5c87ed 100644 --- a/js/src/ion/Ion.cpp +++ b/js/src/ion/Ion.cpp @@ -893,7 +893,7 @@ bool OptimizeMIR(MIRGenerator *mir) { IonSpewPass("BuildSSA"); - // Note: don't call AssertGraphCoherency before SplitCriticalEdges, + // Note: don't call AssertGraphCoherency before ReorderBlocks, // the graph is not in RPO at this point. MIRGraph &graph = mir->graph(); @@ -904,11 +904,24 @@ OptimizeMIR(MIRGenerator *mir) if (!SplitCriticalEdges(graph)) return false; IonSpewPass("Split Critical Edges"); - AssertGraphCoherency(graph); if (mir->shouldCancel("Split Critical Edges")) return false; + if (!RecalculateLoopDepth(graph)) + return false; + // No spew: graph not changed. + + if (mir->shouldCancel("Recalculate Loop Depth")) + return false; + + if (!ReorderBlocks(graph)) + return false; + // No spew: wait for right block numbers + + if (mir->shouldCancel("ReorderBlocks")) + return false; + if (!RenumberBlocks(graph)) return false; IonSpewPass("Renumber Blocks"); diff --git a/js/src/ion/IonAnalysis.cpp b/js/src/ion/IonAnalysis.cpp index fc13e62faec3..95f8d5da59d2 100644 --- a/js/src/ion/IonAnalysis.cpp +++ b/js/src/ion/IonAnalysis.cpp @@ -623,6 +623,210 @@ ion::ApplyTypeInformation(MIRGenerator *mir, MIRGraph &graph) return true; } +static bool +SetLoopDepth(MBasicBlock *header) +{ + size_t depth = header->loopDepth() + 1; + + Vector worklist; + worklist.append(header->backedge()); + + // Iterate over all blocks that are in a path from backedge to the loopheader. + while (!worklist.empty()) { + MBasicBlock *block = worklist.popCopy(); + block->setLoopDepth(depth); + if (block == header) + continue; + + for (size_t i = 0; i < block->numPredecessors(); i++) { + MBasicBlock *pred = block->getPredecessor(i); + if (pred->loopDepth() == depth) + continue; + + if (!worklist.append(pred)) + return false; + } + } + + return true; +} + +bool +ion::RecalculateLoopDepth(MIRGraph &graph) +{ + // Recalculate the loop depth information. + // + // The definition of a loop (important to correctly reorder blocks and for LICM) is: + // all blocks reached when creating a path from the loop backedge to the loopheader. + // This information differs from the loopdepth information gathered in IonBuilder. + // In IonBuilder blocks that are placed inside the loop are considered to be part of the loop. + // E.g.: + // for (...) { + // + // if (...) { + // return; + // } + // } + // IonBuilder will consider the content of the conditional to be part of the loop. + // As of this pass/call this block won't be part of the loop anymore. + + + // Reset all loopdepth information + for (MBasicBlockIterator block(graph.begin()); block != graph.end(); block++) + (*block)->setLoopDepth(0); + + // Iterate through graph for loopheaders + // The graph isn't in RPO order yet. Therefore we need to iterate differently. + // In order to iterate correctly we only need to make sure we see the loopheader + // of outer loops, before the loopheader of an innerloop. + // This is guarenteed if we walk just by following the successors, + // since we can't enter a loopbody before seeing the loopheader. + // Here we do DFS. + Vector worklist; + if (!worklist.append(graph.entryBlock())) + return false; + + JS_ASSERT(!graph.entryBlock()->isLoopHeader()); + while (!worklist.empty()) { + MBasicBlock *block = worklist.back(); + + // Add not yet visited successors to the worklist. + for (size_t i = 0; i < block->numSuccessors(); i++) { + MBasicBlock *succ = block->getSuccessor(i); + if (!succ->isMarked()) { + if (!worklist.append(succ)) + return false; + succ->mark(); + + // Set the loopdepth correctly. + if (succ->isLoopHeader()) { + if (!SetLoopDepth(succ)) + return false; + } + } + } + + // If any blocks were added, process them first. + if (block != worklist.back()) + continue; + + // All successors of this block are visited. + worklist.popBack(); + } + + graph.unmarkBlocks(); + + return true; +} + +static void +SortByLoopDepth(InlineList &pending, MBasicBlockIterator from) +{ + // Sort the blocks between "from" and the end of the pending list. + // In almost all cases this list will contain at max 2 items (except for tableswitch). + // Therefore no need for a fancy algorithm. + + MBasicBlockIterator end = pending.end(); + while (from != end) { + MBasicBlockIterator current(from); + MBasicBlock *max = *current; + while (current != end) { + if (current->loopDepth() > max->loopDepth()) + max = *current; + current++; + } + if (max != *from) { + pending.remove(max); + pending.insertBefore(*from, max); + } + from++; + } +} + +bool +ion::ReorderBlocks(MIRGraph &graph) +{ + // The blocks get reordered to be in RPO (they are already). + // The order will be improved by adding an extra constrain: + // when deciding which successor to visit, always start with the one with lowest loopdepth. + // Consequences: + // => blocks with the same loopdepth are closer (=> tighter loops) + // => loop backedges and loop headers aren't interleaved when visiting + // the graph in RPO (important for Alias Analysis) + + // The RPO algorithm works as following. + // It has a stack (pending) and we put the successors on there. + // The last item on the stack is the one we are looking too. + // If it still has unmarked successsors those get added and there is a new last item. + // If there are no unmarked successors anymore, all successors are processed + // and this block can get popped from the stack and added to the graph. + + mozilla::DebugOnly numBlocks = graph.numBlocks(); + + InlineList pending; + MBasicBlock *entry = graph.entryBlock(); + MBasicBlock *osr = graph.osrBlock(); + graph.clearBlockList(); + + // Start with entry block + pending.pushBack(entry); + entry->mark(); + + // Iterate all blocks to put the blocks in PostOrder to the graph, + // reversing happens by using moveBlockToBegin. + while (!pending.empty()) { + MBasicBlock *block = pending.peekBack(); + bool sortSuccessors = false; + + // Add not yet visited successors to the pending list. + for (size_t i = 0; i < block->numSuccessors(); i++) { + MBasicBlock *succ = block->getSuccessor(i); + if (!succ->isMarked()) { + pending.pushBack(succ); + succ->mark(); + + // Only request sorting, when there are different loopDepths. + if (succ->loopDepth() != block->loopDepth()) + sortSuccessors = true; + } + } + + // Test if there was a new block added + if (block != pending.peekBack()) { + // Sort the blocks if needed + if (sortSuccessors) { + MBasicBlockIterator start = pending.begin(block); + start++; + SortByLoopDepth(pending, start); + } + continue; + } + + // Here we are sure all successors of this block have been added to the graph. + // Now this block can get added to the graph. + pending.popBack(); + graph.addBlock(block); + graph.moveBlockToBegin(block); + } + + // Put the osr block in the second place + if (osr) { + graph.addBlock(osr); + graph.moveBlockToBegin(osr); + graph.moveBlockToBegin(entry); + } + + graph.unmarkBlocks(); + + JS_ASSERT(graph.numBlocks() == numBlocks); + +#ifdef DEBUG + graph.setInRPO(); +#endif + + return true; +} + bool ion::RenumberBlocks(MIRGraph &graph) { diff --git a/js/src/ion/IonAnalysis.h b/js/src/ion/IonAnalysis.h index 42b02bb3f961..8c6d9cb1dec1 100644 --- a/js/src/ion/IonAnalysis.h +++ b/js/src/ion/IonAnalysis.h @@ -39,6 +39,12 @@ EliminateDeadCode(MIRGenerator *mir, MIRGraph &graph); bool ApplyTypeInformation(MIRGenerator *mir, MIRGraph &graph); +bool +RecalculateLoopDepth(MIRGraph &graph); + +bool +ReorderBlocks(MIRGraph &graph); + bool RenumberBlocks(MIRGraph &graph); diff --git a/js/src/ion/IonBuilder.cpp b/js/src/ion/IonBuilder.cpp index f7bbbe6fdbe1..3a367d8d92b5 100644 --- a/js/src/ion/IonBuilder.cpp +++ b/js/src/ion/IonBuilder.cpp @@ -1265,7 +1265,6 @@ IonBuilder::processIfEnd(CFGState &state) } current = state.branch.ifFalse; - graph().moveBlockToEnd(current); pc = current->pc(); return ControlStatus_Joined; } @@ -1280,7 +1279,6 @@ IonBuilder::processIfElseTrueEnd(CFGState &state) state.stopAt = state.branch.falseEnd; pc = state.branch.ifFalse->pc(); current = state.branch.ifFalse; - graph().moveBlockToEnd(current); return ControlStatus_Jumped; } @@ -1339,10 +1337,7 @@ IonBuilder::processBrokenLoop(CFGState &state) // structure never actually loops, the condition itself can still fail and // thus we must resume at the successor, if one exists. current = state.loop.successor; - if (current) { - JS_ASSERT(current->loopDepth() == loopDepth_); - graph().moveBlockToEnd(current); - } + JS_ASSERT_IF(current, current->loopDepth() == loopDepth_); // Join the breaks together and continue parsing. if (state.loop.breaks) { @@ -1384,10 +1379,8 @@ IonBuilder::finishLoop(CFGState &state, MBasicBlock *successor) // including the successor. if (!state.loop.entry->setBackedge(current)) return ControlStatus_Error; - if (successor) { - graph().moveBlockToEnd(successor); + if (successor) successor->inheritPhis(state.loop.entry); - } if (state.loop.breaks) { // Propagate phis placed in the header to individual break exit points. @@ -1642,9 +1635,6 @@ IonBuilder::processNextTableSwitchCase(CFGState &state) successor->addPredecessor(current); } - // Insert successor after the current block, to maintain RPO. - graph().moveBlockToEnd(successor); - // If this is the last successor the block should stop at the end of the tableswitch // Else it should stop at the start of the next successor if (state.tableswitch.currentBlock+1 < state.tableswitch.ins->numBlocks()) @@ -1668,7 +1658,6 @@ IonBuilder::processAndOrEnd(CFGState &state) return ControlStatus_Error; current = state.branch.ifFalse; - graph().moveBlockToEnd(current); pc = current->pc(); return ControlStatus_Joined; } @@ -2215,9 +2204,6 @@ IonBuilder::tableSwitch(JSOp op, jssrcnote *sn) pc2 += JUMP_OFFSET_LEN; } - // Move defaultcase to the end, to maintain RPO. - graph().moveBlockToEnd(defaultcase); - JS_ASSERT(tableswitch->numCases() == (uint32_t)(high - low + 1)); JS_ASSERT(tableswitch->numSuccessors() > 0); @@ -2543,9 +2529,6 @@ IonBuilder::processCondSwitchBody(CFGState &state) MBasicBlock *nextBody = bodies[currentIdx++]; JS_ASSERT_IF(current, pc == nextBody->pc()); - // Fix the reverse post-order iteration. - graph().moveBlockToEnd(nextBody); - // The last body continue into the new one. if (current) { current->end(MGoto::New(nextBody)); @@ -3869,7 +3852,6 @@ IonBuilder::inlineCalls(CallInfo &callInfo, AutoObjectVector &targets, // Check the depth change: +1 for retval JS_ASSERT(returnBlock->stackDepth() == dispatchBlock->stackDepth() - callInfo.numFormals() + 1); - graph().moveBlockToEnd(returnBlock); current = returnBlock; return true; } diff --git a/js/src/ion/MIRGraph.h b/js/src/ion/MIRGraph.h index 99818abc9bc9..dd4cf732cfa4 100644 --- a/js/src/ion/MIRGraph.h +++ b/js/src/ion/MIRGraph.h @@ -482,6 +482,11 @@ class MIRGraph size_t numBlocks_; +#ifdef DEBUG + // Is the graph in Reverse Post Order + bool inRPO_; +#endif + public: MIRGraph(TempAllocator *alloc) : alloc_(alloc), @@ -491,6 +496,9 @@ class MIRGraph osrBlock_(NULL), osrStart_(NULL), numBlocks_(0) +#ifdef DEBUG + , inRPO_(false) +#endif { } template @@ -539,25 +547,28 @@ class MIRGraph return blocks_.end(); } PostorderIterator poBegin() { + JS_ASSERT(inRPO_); return blocks_.rbegin(); } PostorderIterator poEnd() { + JS_ASSERT(inRPO_); return blocks_.rend(); } ReversePostorderIterator rpoBegin() { + JS_ASSERT(inRPO_); return blocks_.begin(); } ReversePostorderIterator rpoEnd() { + JS_ASSERT(inRPO_); return blocks_.end(); } void removeBlock(MBasicBlock *block) { blocks_.remove(block); numBlocks_--; } - void moveBlockToEnd(MBasicBlock *block) { - JS_ASSERT(block->id()); + void moveBlockToBegin(MBasicBlock *block) { blocks_.remove(block); - blocks_.pushBack(block); + blocks_.pushFront(block); } size_t numBlocks() const { return numBlocks_; @@ -612,6 +623,12 @@ class MIRGraph return scripts_.begin(); } +#ifdef DEBUG + void setInRPO() { + inRPO_ = true; + } +#endif + // The ParSlice is an instance of ForkJoinSlice*, it carries // "per-helper-thread" information. So as not to modify the // calling convention for parallel code, we obtain the current