From 6bd6da4097e63172329f5680f69f70a46a8995a7 Mon Sep 17 00:00:00 2001 From: Chris Lattner <sabre@nondot.org> Date: Sat, 23 Sep 2006 08:19:21 +0000 Subject: [PATCH] Be far more careful when splitting a loop header, either to form a preheader or when splitting loops with a common header into multiple loops. In particular the old code would always insert the preheader before the old loop header. This is disasterous in cases where the loop hasn't been rotated. For example, it can produce code like: .. outside the loop... jmp LBB1_2 #bb13.outer LBB1_1: #bb1 movsd 8(%esp,%esi,8), %xmm1 mulsd (%edi), %xmm1 addsd %xmm0, %xmm1 addl $24, %edi incl %esi jmp LBB1_3 #bb13 LBB1_2: #bb13.outer leal (%edx,%eax,8), %edi pxor %xmm1, %xmm1 xorl %esi, %esi LBB1_3: #bb13 movapd %xmm1, %xmm0 cmpl $4, %esi jl LBB1_1 #bb1 Note that the loop body is actually LBB1_1 + LBB1_3, which means that the loop now contains an uncond branch WITHIN it to jump around the inserted loop header (LBB1_2). Doh. This patch changes the preheader insertion code to insert it in the right spot, producing this code: ... outside the loop, fall into the header ... LBB1_1: #bb13.outer leal (%edx,%eax,8), %esi pxor %xmm0, %xmm0 xorl %edi, %edi jmp LBB1_3 #bb13 LBB1_2: #bb1 movsd 8(%esp,%edi,8), %xmm0 mulsd (%esi), %xmm0 addsd %xmm1, %xmm0 addl $24, %esi incl %edi LBB1_3: #bb13 movapd %xmm0, %xmm1 cmpl $4, %edi jl LBB1_2 #bb1 Totally crazy, no branch in the loop! :) llvm-svn: 30587 --- llvm/lib/Transforms/Utils/LoopSimplify.cpp | 51 +++++++++++++++++++++- 1 file changed, 50 insertions(+), 1 deletion(-) diff --git a/llvm/lib/Transforms/Utils/LoopSimplify.cpp b/llvm/lib/Transforms/Utils/LoopSimplify.cpp index f2ab0bdfa450..620a581c8a35 100644 --- a/llvm/lib/Transforms/Utils/LoopSimplify.cpp +++ b/llvm/lib/Transforms/Utils/LoopSimplify.cpp @@ -84,7 +84,10 @@ namespace { void InsertPreheaderForLoop(Loop *L); Loop *SeparateNestedLoop(Loop *L); void InsertUniqueBackedgeBlock(Loop *L); - + void PlaceSplitBlockCarefully(BasicBlock *NewBB, + std::vector<BasicBlock*> &SplitPreds, + Loop *L); + void UpdateDomInfoForRevectoredPreds(BasicBlock *NewBB, std::vector<BasicBlock*> &PredBlocks); }; @@ -367,6 +370,10 @@ void LoopSimplify::InsertPreheaderForLoop(Loop *L) { Parent->addBasicBlockToLoop(NewBB, *LI); UpdateDomInfoForRevectoredPreds(NewBB, OutsideBlocks); + + // Make sure that NewBB is put someplace intelligent, which doesn't mess up + // code layout too horribly. + PlaceSplitBlockCarefully(NewBB, OutsideBlocks, L); } /// RewriteLoopExitBlock - Ensure that the loop preheader dominates all exit @@ -435,6 +442,44 @@ static PHINode *FindPHIToPartitionLoops(Loop *L, DominatorSet &DS, return 0; } +// PlaceSplitBlockCarefully - If the block isn't already, move the new block to +// right after some 'outside block' block. This prevents the preheader from +// being placed inside the loop body, e.g. when the loop hasn't been rotated. +void LoopSimplify::PlaceSplitBlockCarefully(BasicBlock *NewBB, + std::vector<BasicBlock*>&SplitPreds, + Loop *L) { + // Check to see if NewBB is already well placed. + Function::iterator BBI = NewBB; --BBI; + for (unsigned i = 0, e = SplitPreds.size(); i != e; ++i) { + if (&*BBI == SplitPreds[i]) + return; + } + + // If it isn't already after an outside block, move it after one. This is + // always good as it makes the uncond branch from the outside block into a + // fall-through. + + // Figure out *which* outside block to put this after. Prefer an outside + // block that neighbors a BB actually in the loop. + BasicBlock *FoundBB = 0; + for (unsigned i = 0, e = SplitPreds.size(); i != e; ++i) { + Function::iterator BBI = SplitPreds[i]; + if (++BBI != NewBB->getParent()->end() && + L->contains(BBI)) { + FoundBB = SplitPreds[i]; + break; + } + } + + // If our heuristic for a *good* bb to place this after doesn't find + // anything, just pick something. It's likely better than leaving it within + // the loop. + if (!FoundBB) + FoundBB = SplitPreds[0]; + NewBB->moveAfter(FoundBB); +} + + /// SeparateNestedLoop - If this loop has multiple backedges, try to pull one of /// them out into a nested loop. This is important for code that looks like /// this: @@ -471,6 +516,10 @@ Loop *LoopSimplify::SeparateNestedLoop(Loop *L) { // Update dominator information (set, immdom, domtree, and domfrontier) UpdateDomInfoForRevectoredPreds(NewBB, OuterLoopPreds); + // Make sure that NewBB is put someplace intelligent, which doesn't mess up + // code layout too horribly. + PlaceSplitBlockCarefully(NewBB, OuterLoopPreds, L); + // Create the new outer loop. Loop *NewOuter = new Loop();