<div class="gmail_quote">On Sun, Oct 23, 2011 at 3:56 AM, Roman Divacky <span dir="ltr"><<a href="mailto:rdivacky@freebsd.org">rdivacky@freebsd.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
I enabled your MachineBlockPlacement pass by default and ran perlbench.<br>
<br>
Your pass gets perl from 94% (of gcc4.2.1 compiled one performance) to 96%.<br>
Great work!<br></blockquote><div><br></div><div>That's actually rather surprising. The probabilities we're basing any of this on are quite broken. But hey, I'm glad it's improving things. What hardware? How stable were the numbers?</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<div class="HOEnZb"><div class="h5"><br>
On Sun, Oct 23, 2011 at 09:18:45AM -0000, Chandler Carruth wrote:<br>
> Author: chandlerc<br>
> Date: Sun Oct 23 04:18:45 2011<br>
> New Revision: 142743<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=142743&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=142743&view=rev</a><br>
> Log:<br>
> Completely re-write the algorithm behind MachineBlockPlacement based on<br>
> discussions with Andy. Fundamentally, the previous algorithm is both<br>
> counter productive on several fronts and prioritizing things which<br>
> aren't necessarily the most important: static branch prediction.<br>
><br>
> The new algorithm uses the existing loop CFG structure information to<br>
> walk through the CFG itself to layout blocks. It coalesces adjacent<br>
> blocks within the loop where the CFG allows based on the most likely<br>
> path taken. Finally, it topologically orders the block chains that have<br>
> been formed. This allows it to choose a (mostly) topologically valid<br>
> ordering which still priorizes fallthrough within the structural<br>
> constraints.<br>
><br>
> As a final twist in the algorithm, it does violate the CFG when it<br>
> discovers a "hot" edge, that is an edge that is more than 4x hotter than<br>
> the competing edges in the CFG. These are forcibly merged into<br>
> a fallthrough chain.<br>
><br>
> Future transformations that need te be added are rotation of loop exit<br>
> conditions to be fallthrough, and better isolation of cold block chains.<br>
> I'm also planning on adding statistics to model how well the algorithm<br>
> does at laying out blocks based on the probabilities it receives.<br>
><br>
> The old tests mostly still pass, and I have some new tests to add, but<br>
> the nested loops are still behaving very strangely. This almost seems<br>
> like working-as-intended as it rotated the exit branch to be<br>
> fallthrough, but I'm not convinced this is actually the best layout. It<br>
> is well supported by the probabilities for loops we currently get, but<br>
> those are pretty broken for nested loops, so this may change later.<br>
><br>
> Modified:<br>
> llvm/trunk/lib/CodeGen/MachineBlockPlacement.cpp<br>
> llvm/trunk/test/CodeGen/X86/block-placement.ll<br>
><br>
> Modified: llvm/trunk/lib/CodeGen/MachineBlockPlacement.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/MachineBlockPlacement.cpp?rev=142743&r1=142742&r2=142743&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/MachineBlockPlacement.cpp?rev=142743&r1=142742&r2=142743&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/lib/CodeGen/MachineBlockPlacement.cpp (original)<br>
> +++ llvm/trunk/lib/CodeGen/MachineBlockPlacement.cpp Sun Oct 23 04:18:45 2011<br>
> @@ -7,15 +7,21 @@<br>
> //<br>
> //===----------------------------------------------------------------------===//<br>
> //<br>
> -// This file implements basic block placement transformations using branch<br>
> -// probability estimates. It is based around "Algo2" from Profile Guided Code<br>
> -// Positioning [<a href="http://portal.acm.org/citation.cfm?id=989433" target="_blank">http://portal.acm.org/citation.cfm?id=989433</a>].<br>
> +// This file implements basic block placement transformations using the CFG<br>
> +// structure and branch probability estimates.<br>
> //<br>
> -// We combine the BlockFrequencyInfo with BranchProbabilityInfo to simulate<br>
> -// measured edge-weights. The BlockFrequencyInfo effectively summarizes the<br>
> -// probability of starting from any particular block, and the<br>
> -// BranchProbabilityInfo the probability of exiting the block via a particular<br>
> -// edge. Combined they form a function-wide ordering of the edges.<br>
> +// The pass strives to preserve the structure of the CFG (that is, retain<br>
> +// a topological ordering of basic blocks) in the absense of a *strong* signal<br>
> +// to the contrary from probabilities. However, within the CFG structure, it<br>
> +// attempts to choose an ordering which favors placing more likely sequences of<br>
> +// blocks adjacent to each other.<br>
> +//<br>
> +// The algorithm works from the inner-most loop within a function outward, and<br>
> +// at each stage walks through the basic blocks, trying to coalesce them into<br>
> +// sequential chains where allowed by the CFG (or demanded by heavy<br>
> +// probabilities). Finally, it walks the blocks in topological order, and the<br>
> +// first time it reaches a chain of basic blocks, it schedules them in the<br>
> +// function in-order.<br>
> //<br>
> //===----------------------------------------------------------------------===//<br>
><br>
> @@ -29,8 +35,10 @@<br>
> #include "llvm/CodeGen/MachineModuleInfo.h"<br>
> #include "llvm/CodeGen/Passes.h"<br>
> #include "llvm/Support/Allocator.h"<br>
> +#include "llvm/Support/Debug.h"<br>
> #include "llvm/Support/ErrorHandling.h"<br>
> #include "llvm/ADT/DenseMap.h"<br>
> +#include "llvm/ADT/PostOrderIterator.h"<br>
> #include "llvm/ADT/SCCIterator.h"<br>
> #include "llvm/ADT/SmallPtrSet.h"<br>
> #include "llvm/ADT/SmallVector.h"<br>
> @@ -57,7 +65,7 @@<br>
> }<br>
><br>
> namespace {<br>
> -struct BlockChain;<br>
> +class BlockChain;<br>
> /// \brief Type for our function-wide basic block -> block chain mapping.<br>
> typedef DenseMap<MachineBasicBlock *, BlockChain *> BlockToChainMapType;<br>
> }<br>
> @@ -78,22 +86,12 @@<br>
> /// The block chains also have support for calculating and caching probability<br>
> /// information related to the chain itself versus other chains. This is used<br>
> /// for ranking during the final layout of block chains.<br>
> -struct BlockChain {<br>
> - class SuccIterator;<br>
> -<br>
> - /// \brief The first and last basic block that from this chain.<br>
> - ///<br>
> - /// The chain is stored within the existing function ilist of basic blocks.<br>
> - /// When merging chains or otherwise manipulating them, we splice the blocks<br>
> - /// within this ilist, giving us very cheap storage here and constant time<br>
> - /// merge operations.<br>
> +class BlockChain {<br>
> + /// \brief The sequence of blocks belonging to this chain.<br>
> ///<br>
> - /// It is extremely important to note that LastBB is the iterator pointing<br>
> - /// *at* the last basic block in the chain. That is, the chain consists of<br>
> - /// the *closed* range [FirstBB, LastBB]. We cannot use half-open ranges<br>
> - /// because the next basic block may get relocated to a different part of the<br>
> - /// function at any time during the run of this pass.<br>
> - MachineFunction::iterator FirstBB, LastBB;<br>
> + /// This is the sequence of blocks for a particular chain. These will be laid<br>
> + /// out in-order within the function.<br>
> + SmallVector<MachineBasicBlock *, 4> Blocks;<br>
><br>
> /// \brief A handle to the function-wide basic block to block chain mapping.<br>
> ///<br>
> @@ -103,158 +101,66 @@<br>
> /// structure.<br>
> BlockToChainMapType &BlockToChain;<br>
><br>
> - /// \brief The weight used to rank two block chains in the same SCC.<br>
> - ///<br>
> - /// This is used during SCC layout of block chains to cache and rank the<br>
> - /// chains. It is supposed to represent the expected frequency with which<br>
> - /// control reaches a block within this chain, has the option of branching to<br>
> - /// a block in some other chain participating in the SCC, but instead<br>
> - /// continues within this chain. The higher this is, the more costly we<br>
> - /// expect mis-predicted branches between this chain and other chains within<br>
> - /// the SCC to be. Thus, since we expect branches between chains to be<br>
> - /// predicted when backwards and not predicted when forwards, the higher this<br>
> - /// is the more important that this chain is laid out first among those<br>
> - /// chains in the same SCC as it.<br>
> - BlockFrequency InChainEdgeFrequency;<br>
> -<br>
> +public:<br>
> /// \brief Construct a new BlockChain.<br>
> ///<br>
> /// This builds a new block chain representing a single basic block in the<br>
> /// function. It also registers itself as the chain that block participates<br>
> /// in with the BlockToChain mapping.<br>
> BlockChain(BlockToChainMapType &BlockToChain, MachineBasicBlock *BB)<br>
> - : FirstBB(BB), LastBB(BB), BlockToChain(BlockToChain) {<br>
> + : Blocks(1, BB), BlockToChain(BlockToChain) {<br>
> assert(BB && "Cannot create a chain with a null basic block");<br>
> BlockToChain[BB] = this;<br>
> }<br>
><br>
> - /// \brief Merge another block chain into this one.<br>
> + /// \brief Iterator over blocks within the chain.<br>
> + typedef SmallVectorImpl<MachineBasicBlock *>::const_iterator iterator;<br>
> +<br>
> + /// \brief Beginning of blocks within the chain.<br>
> + iterator begin() const { return Blocks.begin(); }<br>
> +<br>
> + /// \brief End of blocks within the chain.<br>
> + iterator end() const { return Blocks.end(); }<br>
> +<br>
> + /// \brief Merge a block chain into this one.<br>
> ///<br>
> /// This routine merges a block chain into this one. It takes care of forming<br>
> /// a contiguous sequence of basic blocks, updating the edge list, and<br>
> /// updating the block -> chain mapping. It does not free or tear down the<br>
> /// old chain, but the old chain's block list is no longer valid.<br>
> - void merge(BlockChain *Chain) {<br>
> - assert(Chain && "Cannot merge a null chain");<br>
> - MachineFunction::iterator EndBB = llvm::next(LastBB);<br>
> - MachineFunction::iterator ChainEndBB = llvm::next(Chain->LastBB);<br>
> -<br>
> - // Update the incoming blocks to point to this chain.<br>
> - for (MachineFunction::iterator BI = Chain->FirstBB, BE = ChainEndBB;<br>
> - BI != BE; ++BI) {<br>
> - assert(BlockToChain[BI] == Chain && "Incoming blocks not in chain");<br>
> - BlockToChain[BI] = this;<br>
> + void merge(MachineBasicBlock *BB, BlockChain *Chain) {<br>
> + assert(BB);<br>
> + assert(!Blocks.empty());<br>
> + assert(Blocks.back()->isSuccessor(BB));<br>
> +<br>
> + // Fast path in case we don't have a chain already.<br>
> + if (!Chain) {<br>
> + assert(!BlockToChain[BB]);<br>
> + Blocks.push_back(BB);<br>
> + BlockToChain[BB] = this;<br>
> + return;<br>
> }<br>
><br>
> - // We splice the blocks together within the function (unless they already<br>
> - // are adjacent) so we can represent the new chain with a pair of pointers<br>
> - // to basic blocks within the function. This is also useful as each chain<br>
> - // of blocks will end up being laid out contiguously within the function.<br>
> - if (EndBB != Chain->FirstBB)<br>
> - FirstBB->getParent()->splice(EndBB, Chain->FirstBB, ChainEndBB);<br>
> - LastBB = Chain->LastBB;<br>
> - }<br>
> -};<br>
> -}<br>
> -<br>
> -namespace {<br>
> -/// \brief Successor iterator for BlockChains.<br>
> -///<br>
> -/// This is an iterator that walks over the successor block chains by looking<br>
> -/// through its blocks successors and mapping those back to block chains. This<br>
> -/// iterator is not a fully-functioning iterator, it is designed specifically<br>
> -/// to support the interface required by SCCIterator when forming and walking<br>
> -/// SCCs of BlockChains.<br>
> -///<br>
> -/// Note that this iterator cannot be used while the chains are still being<br>
> -/// formed and/or merged. Unlike the chains themselves, it does store end<br>
> -/// iterators which could be moved if the chains are re-ordered. Once we begin<br>
> -/// forming and iterating over an SCC of chains, the order of blocks within the<br>
> -/// function must not change until we finish using the SCC iterators.<br>
> -class BlockChain::SuccIterator<br>
> - : public std::iterator<std::forward_iterator_tag,<br>
> - BlockChain *, ptrdiff_t> {<br>
> - BlockChain *Chain;<br>
> - MachineFunction::iterator BI, BE;<br>
> - MachineBasicBlock::succ_iterator SI;<br>
> + assert(BB == *Chain->begin());<br>
> + assert(Chain->begin() != Chain->end());<br>
><br>
> -public:<br>
> - explicit SuccIterator(BlockChain *Chain)<br>
> - : Chain(Chain), BI(Chain->FirstBB), BE(llvm::next(Chain->LastBB)),<br>
> - SI(BI->succ_begin()) {<br>
> - while (BI != BE && BI->succ_begin() == BI->succ_end())<br>
> - ++BI;<br>
> - if (BI != BE)<br>
> - SI = BI->succ_begin();<br>
> - }<br>
> -<br>
> - /// \brief Helper function to create an end iterator for a particular chain.<br>
> - ///<br>
> - /// The "end" state is extremely arbitrary. We chose to have BI == BE, and SI<br>
> - /// == Chain->FirstBB->succ_begin(). The value of SI doesn't really make any<br>
> - /// sense, but rather than try to rationalize SI and our increment, when we<br>
> - /// detect an "end" state, we just immediately call this function to build<br>
> - /// the canonical end iterator.<br>
> - static SuccIterator CreateEnd(BlockChain *Chain) {<br>
> - SuccIterator It(Chain);<br>
> - It.BI = It.BE;<br>
> - return It;<br>
> - }<br>
> -<br>
> - bool operator==(const SuccIterator &RHS) const {<br>
> - return (Chain == RHS.Chain && BI == <a href="http://RHS.BI" target="_blank">RHS.BI</a> && SI == <a href="http://RHS.SI" target="_blank">RHS.SI</a>);<br>
> - }<br>
> - bool operator!=(const SuccIterator &RHS) const {<br>
> - return !operator==(RHS);<br>
> - }<br>
> -<br>
> - SuccIterator& operator++() {<br>
> - assert(*this != CreateEnd(Chain) && "Cannot increment the end iterator");<br>
> - // There may be null successor pointers, skip over them.<br>
> - // FIXME: I don't understand *why* there are null successor pointers.<br>
> - do {<br>
> - ++SI;<br>
> - if (SI != BI->succ_end() && *SI)<br>
> - return *this;<br>
> -<br>
> - // There may be a basic block without successors. Skip over them.<br>
> - do {<br>
> - ++BI;<br>
> - if (BI == BE)<br>
> - return *this = CreateEnd(Chain);<br>
> - } while (BI->succ_begin() == BI->succ_end());<br>
> - SI = BI->succ_begin();<br>
> - } while (!*SI);<br>
> - return *this;<br>
> - }<br>
> - SuccIterator operator++(int) {<br>
> - SuccIterator tmp = *this;<br>
> - ++*this;<br>
> - return tmp;<br>
> - }<br>
> -<br>
> - BlockChain *operator*() const {<br>
> - assert(Chain->BlockToChain.lookup(*SI) && "Missing chain");<br>
> - return Chain->BlockToChain.lookup(*SI);<br>
> - }<br>
> -};<br>
> -}<br>
> -<br>
> -namespace {<br>
> -/// \brief Sorter used with containers of BlockChain pointers.<br>
> -///<br>
> -/// Sorts based on the \see BlockChain::InChainEdgeFrequency -- see its<br>
> -/// comments for details on what this ordering represents.<br>
> -struct ChainPtrPrioritySorter {<br>
> - bool operator()(const BlockChain *LHS, const BlockChain *RHS) const {<br>
> - assert(LHS && RHS && "Null chain entry");<br>
> - return LHS->InChainEdgeFrequency < RHS->InChainEdgeFrequency;<br>
> + // Update the incoming blocks to point to this chain, and add them to the<br>
> + // chain structure.<br>
> + for (BlockChain::iterator BI = Chain->begin(), BE = Chain->end();<br>
> + BI != BE; ++BI) {<br>
> + Blocks.push_back(*BI);<br>
> + assert(BlockToChain[*BI] == Chain && "Incoming blocks not in chain");<br>
> + BlockToChain[*BI] = this;<br>
> + }<br>
> }<br>
> };<br>
> }<br>
><br>
> namespace {<br>
> class MachineBlockPlacement : public MachineFunctionPass {<br>
> + /// \brief A typedef for a block filter set.<br>
> + typedef SmallPtrSet<MachineBasicBlock *, 16> BlockFilterSet;<br>
> +<br>
> /// \brief A handle to the branch probability pass.<br>
> const MachineBranchProbabilityInfo *MBPI;<br>
><br>
> @@ -270,17 +176,6 @@<br>
> /// \brief A handle to the target's lowering info.<br>
> const TargetLowering *TLI;<br>
><br>
> - /// \brief A prioritized list of edges in the BB-graph.<br>
> - ///<br>
> - /// For each function, we insert all control flow edges between BBs, along<br>
> - /// with their "global" frequency. The Frequency of an edge being taken is<br>
> - /// defined as the frequency of entering the source BB (from MBFI) times the<br>
> - /// probability of taking a particular branch out of that block (from MBPI).<br>
> - ///<br>
> - /// Once built, this list is sorted in ascending frequency, making the last<br>
> - /// edge the hottest one in the function.<br>
> - SmallVector<WeightedEdge, 64> Edges;<br>
> -<br>
> /// \brief Allocator and owner of BlockChain structures.<br>
> ///<br>
> /// We build BlockChains lazily by merging together high probability BB<br>
> @@ -297,24 +192,12 @@<br>
> /// between basic blocks.<br>
> DenseMap<MachineBasicBlock *, BlockChain *> BlockToChain;<br>
><br>
> - /// \brief A prioritized sequence of chains.<br>
> - ///<br>
> - /// We build up the ideal sequence of basic block chains in reverse order<br>
> - /// here, and then walk backwards to arrange the final function ordering.<br>
> - SmallVector<BlockChain *, 16> PChains;<br>
> -<br>
> -#ifndef NDEBUG<br>
> - /// \brief A set of active chains used to sanity-check the pass algorithm.<br>
> - ///<br>
> - /// All operations on this member should be wrapped in an assert or NDEBUG.<br>
> - SmallPtrSet<BlockChain *, 16> ActiveChains;<br>
> -#endif<br>
> -<br>
> BlockChain *CreateChain(MachineBasicBlock *BB);<br>
> - void PrioritizeEdges(MachineFunction &F);<br>
> - void BuildBlockChains();<br>
> - void PrioritizeChains(MachineFunction &F);<br>
> - void PlaceBlockChains(MachineFunction &F);<br>
> + void mergeSuccessor(MachineBasicBlock *BB, BlockChain *Chain,<br>
> + BlockFilterSet *Filter = 0);<br>
> + void buildLoopChains(MachineFunction &F, MachineLoop &L);<br>
> + void buildCFGChains(MachineFunction &F);<br>
> + void placeChainsTopologically(MachineFunction &F);<br>
> void AlignLoops(MachineFunction &F);<br>
><br>
> public:<br>
> @@ -349,21 +232,30 @@<br>
> return new MachineBlockPlacement();<br>
> }<br>
><br>
> -namespace llvm {<br>
> -/// \brief GraphTraits specialization for our BlockChain graph.<br>
> -template <> struct GraphTraits<BlockChain *> {<br>
> - typedef BlockChain NodeType;<br>
> - typedef BlockChain::SuccIterator ChildIteratorType;<br>
> -<br>
> - static NodeType *getEntryNode(NodeType *N) { return N; }<br>
> - static BlockChain::SuccIterator child_begin(NodeType *N) {<br>
> - return BlockChain::SuccIterator(N);<br>
> - }<br>
> - static BlockChain::SuccIterator child_end(NodeType *N) {<br>
> - return BlockChain::SuccIterator::CreateEnd(N);<br>
> - }<br>
> -};<br>
> +#ifndef NDEBUG<br>
> +/// \brief Helper to print the name of a MBB.<br>
> +///<br>
> +/// Only used by debug logging.<br>
> +static std::string getBlockName(MachineBasicBlock *BB) {<br>
> + std::string Result;<br>
> + raw_string_ostream OS(Result);<br>
> + OS << "BB#" << BB->getNumber()<br>
> + << " (derived from LLVM BB '" << BB->getName() << "')";<br>
> + OS.flush();<br>
> + return Result;<br>
> +}<br>
> +<br>
> +/// \brief Helper to print the number of a MBB.<br>
> +///<br>
> +/// Only used by debug logging.<br>
> +static std::string getBlockNum(MachineBasicBlock *BB) {<br>
> + std::string Result;<br>
> + raw_string_ostream OS(Result);<br>
> + OS << "BB#" << BB->getNumber();<br>
> + OS.flush();<br>
> + return Result;<br>
> }<br>
> +#endif<br>
><br>
> /// \brief Helper to create a new chain for a single BB.<br>
> ///<br>
> @@ -373,224 +265,168 @@<br>
> BlockChain *MachineBlockPlacement::CreateChain(MachineBasicBlock *BB) {<br>
> BlockChain *Chain =<br>
> new (ChainAllocator.Allocate()) BlockChain(BlockToChain, BB);<br>
> - assert(ActiveChains.insert(Chain));<br>
> + //assert(ActiveChains.insert(Chain));<br>
> return Chain;<br>
> }<br>
><br>
> -/// \brief Build a prioritized list of edges.<br>
> +/// \brief Merge a chain with any viable successor.<br>
> ///<br>
> -/// The priority is determined by the product of the block frequency (how<br>
> -/// likely it is to arrive at a particular block) times the probability of<br>
> -/// taking this particular edge out of the block. This provides a function-wide<br>
> -/// ordering of the edges.<br>
> -void MachineBlockPlacement::PrioritizeEdges(MachineFunction &F) {<br>
> - assert(Edges.empty() && "Already have an edge list");<br>
> - SmallVector<MachineOperand, 4> Cond; // For AnalyzeBranch.<br>
> - BlockChain *RequiredChain = 0;<br>
> - for (MachineFunction::iterator FI = F.begin(), FE = F.end(); FI != FE; ++FI) {<br>
> - MachineBasicBlock *From = &*FI;<br>
> - // We only consider MBBs with analyzable branches. Even if the analysis<br>
> - // fails, if there is no fallthrough, we can still work with the MBB.<br>
> - MachineBasicBlock *TBB = 0, *FBB = 0;<br>
> - Cond.clear();<br>
> - if (TII->AnalyzeBranch(*From, TBB, FBB, Cond) && From->canFallThrough()) {<br>
> - // We push all unanalyzed blocks onto a chain eagerly to prevent them<br>
> - // from being split later. Create the chain if needed, otherwise just<br>
> - // keep track that these blocks reside on it.<br>
> - if (!RequiredChain)<br>
> - RequiredChain = CreateChain(From);<br>
> - else<br>
> - BlockToChain[From] = RequiredChain;<br>
> - } else {<br>
> - // As soon as we find an analyzable branch, add that block to and<br>
> - // finalize any required chain that has been started. The required chain<br>
> - // is only modeling potentially inexplicable fallthrough, so the first<br>
> - // block to have analyzable fallthrough is a known-safe stopping point.<br>
> - if (RequiredChain) {<br>
> - BlockToChain[From] = RequiredChain;<br>
> - RequiredChain->LastBB = FI;<br>
> - RequiredChain = 0;<br>
> - }<br>
> - }<br>
> -<br>
> - BlockFrequency BaseFrequency = MBFI->getBlockFreq(From);<br>
> - for (MachineBasicBlock::succ_iterator SI = From->succ_begin(),<br>
> - SE = From->succ_end();<br>
> - SI != SE; ++SI) {<br>
> - MachineBasicBlock *To = *SI;<br>
> - WeightedEdge WE = { BaseFrequency * MBPI->getEdgeProbability(From, To),<br>
> - From, To };<br>
> - Edges.push_back(WE);<br>
> - }<br>
> - }<br>
> - assert(!RequiredChain && "Never found a terminator for a required chain");<br>
> - std::stable_sort(Edges.begin(), Edges.end());<br>
> -}<br>
> +/// This routine walks the predecessors of the current block, looking for<br>
> +/// viable merge candidates. It has strict rules it uses to determine when<br>
> +/// a predecessor can be merged with the current block, which center around<br>
> +/// preserving the CFG structure. It performs the merge if any viable candidate<br>
> +/// is found.<br>
> +void MachineBlockPlacement::mergeSuccessor(MachineBasicBlock *BB,<br>
> + BlockChain *Chain,<br>
> + BlockFilterSet *Filter) {<br>
> + assert(BB);<br>
> + assert(Chain);<br>
> +<br>
> + // If this block is not at the end of its chain, it cannot merge with any<br>
> + // other chain.<br>
> + if (Chain && *llvm::prior(Chain->end()) != BB)<br>
> + return;<br>
><br>
> -/// \brief Build chains of basic blocks along hot paths.<br>
> -///<br>
> -/// Build chains by trying to merge each pair of blocks from the mostly costly<br>
> -/// edge first. This is essentially "Algo2" from the Profile Guided Code<br>
> -/// Placement paper. While each node is considered a chain of one block, this<br>
> -/// routine lazily build the chain objects themselves so that when possible it<br>
> -/// can just merge a block into an existing chain.<br>
> -void MachineBlockPlacement::BuildBlockChains() {<br>
> - for (SmallVectorImpl<WeightedEdge>::reverse_iterator EI = Edges.rbegin(),<br>
> - EE = Edges.rend();<br>
> - EI != EE; ++EI) {<br>
> - MachineBasicBlock *SourceB = EI->From, *DestB = EI->To;<br>
> - if (SourceB == DestB) continue;<br>
> -<br>
> - BlockChain *SourceChain = BlockToChain.lookup(SourceB);<br>
> - if (!SourceChain) SourceChain = CreateChain(SourceB);<br>
> - BlockChain *DestChain = BlockToChain.lookup(DestB);<br>
> - if (!DestChain) DestChain = CreateChain(DestB);<br>
> - if (SourceChain == DestChain)<br>
> + // Walk through the successors looking for the highest probability edge.<br>
> + // FIXME: This is an annoying way to do the comparison, but it's correct.<br>
> + // Support should be added to BranchProbability to properly compare two.<br>
> + MachineBasicBlock *Successor = 0;<br>
> + BlockFrequency BestFreq;<br>
> + DEBUG(dbgs() << "Attempting merge from: " << getBlockName(BB) << "\n");<br>
> + for (MachineBasicBlock::succ_iterator SI = BB->succ_begin(),<br>
> + SE = BB->succ_end();<br>
> + SI != SE; ++SI) {<br>
> + if (BB == *SI || (Filter && !Filter->count(*SI)))<br>
> continue;<br>
><br>
> - bool IsSourceTail =<br>
> - SourceChain->LastBB == MachineFunction::iterator(SourceB);<br>
> - bool IsDestHead =<br>
> - DestChain->FirstBB == MachineFunction::iterator(DestB);<br>
> + BlockFrequency SuccFreq(BlockFrequency::getEntryFrequency());<br>
> + SuccFreq *= MBPI->getEdgeProbability(BB, *SI);<br>
> + DEBUG(dbgs() << " " << getBlockName(*SI) << " -> " << SuccFreq << "\n");<br>
> + if (!Successor || SuccFreq > BestFreq || (!(SuccFreq < BestFreq) &&<br>
> + BB->isLayoutSuccessor(*SI))) {<br>
> + Successor = *SI;<br>
> + BestFreq = SuccFreq;<br>
> + }<br>
> + }<br>
> + if (!Successor)<br>
> + return;<br>
><br>
> - if (!IsSourceTail || !IsDestHead)<br>
> - continue;<br>
> + // Grab a chain if it exists already for this successor and make sure the<br>
> + // successor is at the start of the chain as we can't merge mid-chain. Also,<br>
> + // if the successor chain is the same as our chain, we're already merged.<br>
> + BlockChain *SuccChain = BlockToChain[Successor];<br>
> + if (SuccChain && (SuccChain == Chain || Successor != *SuccChain->begin()))<br>
> + return;<br>
><br>
> - SourceChain->merge(DestChain);<br>
> - assert(ActiveChains.erase(DestChain));<br>
> + // We only merge chains across a CFG merge when the desired merge path is<br>
> + // significantly hotter than the incoming edge. We define a hot edge more<br>
> + // strictly than the BranchProbabilityInfo does, as the two predecessor<br>
> + // blocks may have dramatically different incoming probabilities we need to<br>
> + // account for. Therefor we use the "global" edge weight which is the<br>
> + // branch's probability times the block frequency of the predecessor.<br>
> + BlockFrequency MergeWeight = MBFI->getBlockFreq(BB);<br>
> + MergeWeight *= MBPI->getEdgeProbability(BB, Successor);<br>
> + // We only want to consider breaking the CFG when the merge weight is much<br>
> + // higher (80% vs. 20%), so multiply it by 1/4. This will require the merged<br>
> + // edge to be 4x more likely before we disrupt the CFG. This number matches<br>
> + // the definition of "hot" in BranchProbabilityAnalysis (80% vs. 20%).<br>
> + MergeWeight *= BranchProbability(1, 4);<br>
> + for (MachineBasicBlock::pred_iterator PI = Successor->pred_begin(),<br>
> + PE = Successor->pred_end();<br>
> + PI != PE; ++PI) {<br>
> + if (BB == *PI || Successor == *PI) continue;<br>
> + BlockFrequency PredWeight = MBFI->getBlockFreq(*PI);<br>
> + PredWeight *= MBPI->getEdgeProbability(*PI, Successor);<br>
> +<br>
> + // Return on the first predecessor we find which outstrips our merge weight.<br>
> + if (MergeWeight < PredWeight)<br>
> + return;<br>
> + DEBUG(dbgs() << "Breaking CFG edge!\n"<br>
> + << " Edge from " << getBlockNum(BB) << " to "<br>
> + << getBlockNum(Successor) << ": " << MergeWeight << "\n"<br>
> + << " vs. " << getBlockNum(BB) << " to "<br>
> + << getBlockNum(*PI) << ": " << PredWeight << "\n");<br>
> + }<br>
> +<br>
> + DEBUG(dbgs() << "Merging from " << getBlockNum(BB) << " to "<br>
> + << getBlockNum(Successor) << "\n");<br>
> + Chain->merge(Successor, SuccChain);<br>
> +}<br>
> +<br>
> +/// \brief Forms basic block chains from the natural loop structures.<br>
> +///<br>
> +/// These chains are designed to preserve the existing *structure* of the code<br>
> +/// as much as possible. We can then stitch the chains together in a way which<br>
> +/// both preserves the topological structure and minimizes taken conditional<br>
> +/// branches.<br>
> +void MachineBlockPlacement::buildLoopChains(MachineFunction &F, MachineLoop &L) {<br>
> + // First recurse through any nested loops, building chains for those inner<br>
> + // loops.<br>
> + for (MachineLoop::iterator LI = L.begin(), LE = L.end(); LI != LE; ++LI)<br>
> + buildLoopChains(F, **LI);<br>
> +<br>
> + SmallPtrSet<MachineBasicBlock *, 16> LoopBlockSet(L.block_begin(),<br>
> + L.block_end());<br>
> +<br>
> + // Begin building up a set of chains of blocks within this loop which should<br>
> + // remain contiguous. Some of the blocks already belong to a chain which<br>
> + // represents an inner loop.<br>
> + for (MachineLoop::block_iterator BI = L.block_begin(), BE = L.block_end();<br>
> + BI != BE; ++BI) {<br>
> + MachineBasicBlock *BB = *BI;<br>
> + BlockChain *Chain = BlockToChain[BB];<br>
> + if (!Chain) Chain = CreateChain(BB);<br>
> + mergeSuccessor(BB, Chain, &LoopBlockSet);<br>
> + }<br>
> +}<br>
> +<br>
> +void MachineBlockPlacement::buildCFGChains(MachineFunction &F) {<br>
> + // First build any loop-based chains.<br>
> + for (MachineLoopInfo::iterator LI = MLI->begin(), LE = MLI->end(); LI != LE;<br>
> + ++LI)<br>
> + buildLoopChains(F, **LI);<br>
> +<br>
> + // Now walk the blocks of the function forming chains where they don't<br>
> + // violate any CFG structure.<br>
> + for (MachineFunction::iterator BI = F.begin(), BE = F.end();<br>
> + BI != BE; ++BI) {<br>
> + MachineBasicBlock *BB = BI;<br>
> + BlockChain *Chain = BlockToChain[BB];<br>
> + if (!Chain) Chain = CreateChain(BB);<br>
> + mergeSuccessor(BB, Chain);<br>
> }<br>
> }<br>
><br>
> -/// \brief Prioritize the chains to minimize back-edges between chains.<br>
> -///<br>
> -/// This is the trickiest part of the placement algorithm. Each chain is<br>
> -/// a hot-path through a sequence of basic blocks, but there are conditional<br>
> -/// branches away from this hot path, and to some other chain. Hardware branch<br>
> -/// predictors favor back edges over forward edges, and so it is desirable to<br>
> -/// arrange the targets of branches away from a hot path and to some other<br>
> -/// chain to come later in the function, making them forward branches, and<br>
> -/// helping the branch predictor to predict fallthrough.<br>
> -///<br>
> -/// In some cases, this is easy. simply topologically walking from the entry<br>
> -/// chain through its successors in order would work if there were no cycles<br>
> -/// between the chains of blocks, but often there are. In such a case, we first<br>
> -/// need to identify the participants in the cycle, and then rank them so that<br>
> -/// the linearizing of the chains has the lowest *probability* of causing<br>
> -/// a mispredicted branch. To compute the correct rank for a chain, we take the<br>
> -/// complement of the branch probability for each branch leading away from the<br>
> -/// chain and multiply it by the frequency of the source block for that branch.<br>
> -/// This gives us the probability of that particular branch *not* being taken<br>
> -/// in this function. The sum of these probabilities for each chain is used as<br>
> -/// a rank, so that we order the chain with the highest such sum first.<br>
> -/// FIXME: This seems like a good approximation, but there is probably a known<br>
> -/// technique for ordering of an SCC given edge weights. It would be good to<br>
> -/// use that, or even use its code if possible.<br>
> -///<br>
> -/// Also notable is that we prioritize the chains from the bottom up, and so<br>
> -/// all of the "first" and "before" relationships end up inverted in the code.<br>
> -void MachineBlockPlacement::PrioritizeChains(MachineFunction &F) {<br>
> +void MachineBlockPlacement::placeChainsTopologically(MachineFunction &F) {<br>
> MachineBasicBlock *EntryB = &F.front();<br>
> BlockChain *EntryChain = BlockToChain[EntryB];<br>
> assert(EntryChain && "Missing chain for entry block");<br>
> - assert(EntryChain->FirstBB == F.begin() &&<br>
> + assert(*EntryChain->begin() == EntryB &&<br>
> "Entry block is not the head of the entry block chain");<br>
><br>
> - // Form an SCC and walk it from the bottom up.<br>
> - SmallPtrSet<BlockChain *, 4> IsInSCC;<br>
> - for (scc_iterator<BlockChain *> I = scc_begin(EntryChain);<br>
> - !I.isAtEnd(); ++I) {<br>
> - const std::vector<BlockChain *> &SCC = *I;<br>
> - PChains.insert(PChains.end(), SCC.begin(), SCC.end());<br>
> -<br>
> - // If there is only one chain in the SCC, it's trivially sorted so just<br>
> - // bail out early. Sorting the SCC is expensive.<br>
> - if (SCC.size() == 1)<br>
> + // Walk the blocks in RPO, and insert each block for a chain in order the<br>
> + // first time we see that chain.<br>
> + MachineFunction::iterator InsertPos = F.begin();<br>
> + SmallPtrSet<BlockChain *, 16> VisitedChains;<br>
> + ReversePostOrderTraversal<MachineBasicBlock *> RPOT(EntryB);<br>
> + typedef ReversePostOrderTraversal<MachineBasicBlock *>::rpo_iterator<br>
> + rpo_iterator;<br>
> + for (rpo_iterator I = RPOT.begin(), E = RPOT.end(); I != E; ++I) {<br>
> + BlockChain *Chain = BlockToChain[*I];<br>
> + assert(Chain);<br>
> + if(!VisitedChains.insert(Chain))<br>
> continue;<br>
> -<br>
> - // We work strictly on the PChains range from here on out to maximize<br>
> - // locality.<br>
> - SmallVectorImpl<BlockChain *>::iterator SCCEnd = PChains.end(),<br>
> - SCCBegin = SCCEnd - SCC.size();<br>
> - IsInSCC.clear();<br>
> - IsInSCC.insert(SCCBegin, SCCEnd);<br>
> -<br>
> - // Compute the edge frequency of staying in a chain, despite the existency<br>
> - // of an edge to some other chain within this SCC.<br>
> - for (SmallVectorImpl<BlockChain *>::iterator SCCI = SCCBegin;<br>
> - SCCI != SCCEnd; ++SCCI) {<br>
> - BlockChain *Chain = *SCCI;<br>
> -<br>
> - // Special case the entry chain. Regardless of the weights of other<br>
> - // chains, the entry chain *must* come first, so move it to the end, and<br>
> - // avoid processing that chain at all.<br>
> - if (Chain == EntryChain) {<br>
> - --SCCEnd;<br>
> - if (SCCI == SCCEnd) break;<br>
> - Chain = *SCCI = *SCCEnd;<br>
> - *SCCEnd = EntryChain;<br>
> - }<br>
> -<br>
> - // Walk over every block in this chain looking for out-bound edges to<br>
> - // other chains in this SCC.<br>
> - for (MachineFunction::iterator BI = Chain->FirstBB,<br>
> - BE = llvm::next(Chain->LastBB);<br>
> - BI != BE; ++BI) {<br>
> - MachineBasicBlock *From = &*BI;<br>
> - for (MachineBasicBlock::succ_iterator SI = BI->succ_begin(),<br>
> - SE = BI->succ_end();<br>
> - SI != SE; ++SI) {<br>
> - MachineBasicBlock *To = *SI;<br>
> - if (!To || !IsInSCC.count(BlockToChain[To]))<br>
> - continue;<br>
> - BranchProbability ComplEdgeProb =<br>
> - MBPI->getEdgeProbability(From, To).getCompl();<br>
> - Chain->InChainEdgeFrequency +=<br>
> - MBFI->getBlockFreq(From) * ComplEdgeProb;<br>
> - }<br>
> - }<br>
> + for (BlockChain::iterator BI = Chain->begin(), BE = Chain->end(); BI != BE;<br>
> + ++BI) {<br>
> + DEBUG(dbgs() << (BI == Chain->begin() ? "Placing chain "<br>
> + : " ... ")<br>
> + << getBlockName(*BI) << "\n");<br>
> + if (InsertPos != MachineFunction::iterator(*BI))<br>
> + F.splice(InsertPos, *BI);<br>
> + else<br>
> + ++InsertPos;<br>
> }<br>
> -<br>
> - // Sort the chains within the SCC according to their edge frequencies,<br>
> - // which should make the least costly chain of blocks to mis-place be<br>
> - // ordered first in the prioritized sequence.<br>
> - std::stable_sort(SCCBegin, SCCEnd, ChainPtrPrioritySorter());<br>
> }<br>
> -}<br>
> -<br>
> -/// \brief Splice the function blocks together based on the chain priorities.<br>
> -///<br>
> -/// Each chain is already represented as a contiguous range of blocks in the<br>
> -/// function. Simply walk backwards down the prioritized chains and splice in<br>
> -/// any chains out of order. Note that the first chain we visit is necessarily<br>
> -/// the entry chain. It has no predecessors and so must be the top of the SCC.<br>
> -/// Also, we cannot splice any chain prior to the entry chain as we can't<br>
> -/// splice any blocks prior to the entry block.<br>
> -void MachineBlockPlacement::PlaceBlockChains(MachineFunction &F) {<br>
> - assert(!PChains.empty() && "No chains were prioritized");<br>
> - assert(PChains.back() == BlockToChain[&F.front()] &&<br>
> - "The entry chain must always be the final chain");<br>
> -<br>
> - MachineFunction::iterator InsertPos = F.begin();<br>
> - for (SmallVectorImpl<BlockChain *>::reverse_iterator CI = PChains.rbegin(),<br>
> - CE = PChains.rend();<br>
> - CI != CE; ++CI) {<br>
> - BlockChain *Chain = *CI;<br>
> - // Check that we process this chain only once for debugging.<br>
> - assert(ActiveChains.erase(Chain) && "Processed a chain twice");<br>
> -<br>
> - // If this chain is already in the right position, just skip past it.<br>
> - // Otherwise, splice it into position.<br>
> - if (InsertPos == Chain->FirstBB)<br>
> - InsertPos = llvm::next(Chain->LastBB);<br>
> - else<br>
> - F.splice(InsertPos, Chain->FirstBB, llvm::next(Chain->LastBB));<br>
> - }<br>
> -<br>
> - // Note that we can't assert this is empty as there may be unreachable blocks<br>
> - // in the function.<br>
> -#ifndef NDEBUG<br>
> - ActiveChains.clear();<br>
> -#endif<br>
><br>
> // Now that every block is in its final position, update all of the<br>
> // terminators.<br>
> @@ -638,21 +474,13 @@<br>
> MLI = &getAnalysis<MachineLoopInfo>();<br>
> TII = F.getTarget().getInstrInfo();<br>
> TLI = F.getTarget().getTargetLowering();<br>
> - assert(Edges.empty());<br>
> assert(BlockToChain.empty());<br>
> - assert(PChains.empty());<br>
> - assert(ActiveChains.empty());<br>
><br>
> - PrioritizeEdges(F);<br>
> - BuildBlockChains();<br>
> - PrioritizeChains(F);<br>
> - PlaceBlockChains(F);<br>
> + buildCFGChains(F);<br>
> + placeChainsTopologically(F);<br>
> AlignLoops(F);<br>
><br>
> - Edges.clear();<br>
> BlockToChain.clear();<br>
> - PChains.clear();<br>
> - ChainAllocator.DestroyAll();<br>
><br>
> // We always return true as we have no way to track whether the final order<br>
> // differs from the original order.<br>
><br>
> Modified: llvm/trunk/test/CodeGen/X86/block-placement.ll<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/block-placement.ll?rev=142743&r1=142742&r2=142743&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/block-placement.ll?rev=142743&r1=142742&r2=142743&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/test/CodeGen/X86/block-placement.ll (original)<br>
> +++ llvm/trunk/test/CodeGen/X86/block-placement.ll Sun Oct 23 04:18:45 2011<br>
> @@ -105,11 +105,10 @@<br>
> ; CHECK: test_nested_loop_align:<br>
> ; CHECK: %entry<br>
> ; CHECK: .align [[ALIGN]],<br>
> -; CHECK-NEXT: %loop.body.1<br>
> +; CHECK-NEXT: %loop.body.2<br>
> ; CHECK: .align [[ALIGN]],<br>
> ; CHECK-NEXT: %inner.loop.body<br>
> ; CHECK-NOT: .align<br>
> -; CHECK: %loop.body.2<br>
> ; CHECK: %exit<br>
><br>
> entry:<br>
><br>
><br>
> _______________________________________________<br>
> llvm-commits mailing list<br>
> <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</div></div></blockquote></div><br>