[llvm-commits] CVS: llvm/lib/Transforms/Utils/LoopSimplify.cpp

[Chris Lattner]

Changes in directory llvm/lib/Transforms/Utils:

LoopSimplify.cpp updated: 1.75 -> 1.76
---
Log message:

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! :)



---
Diffs of the changes:  (+50 -1)

 LoopSimplify.cpp |   51 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 50 insertions(+), 1 deletion(-)


Index: llvm/lib/Transforms/Utils/LoopSimplify.cpp
diff -u llvm/lib/Transforms/Utils/LoopSimplify.cpp:1.75 llvm/lib/Transforms/Utils/LoopSimplify.cpp:1.76
--- llvm/lib/Transforms/Utils/LoopSimplify.cpp:1.75	Sat Sep 23 02:40:52 2006
+++ llvm/lib/Transforms/Utils/LoopSimplify.cpp	Sat Sep 23 03:19:21 2006
@@ -84,7 +84,10 @@
     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 @@
     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 @@
   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 @@
   // 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();