[llvm-branch-commits] [llvm-branch] r121528 - /llvm/branches/Apple/whitney/lib/Transforms/IPO/Inliner.cpp

[Daniel Dunbar]

Author: ddunbar
Date: Fri Dec 10 15:31:48 2010
New Revision: 121528

URL: http://llvm.org/viewvc/llvm-project?rev=121528&view=rev
Log:
Merge r120995:
--
Author: Chris Lattner <clattner at apple.com>
Date:   Mon Dec 6 07:52:42 2010 +0000

    Fix PR8735, a really terrible problem in the inliner's "alloca merging"
    optimization.

    Consider:
    static void foo() {
      A = alloca
      ...
    }

    static void bar() {
      B = alloca
      ...
      call foo();
    }

    void main() {
      bar()
    }

    The inliner proceeds bottom up, but lets pretend it decides not to inline foo
    into bar.  When it gets to main, it inlines bar into main(), and says "hey, I
    just inlined an alloca "B" into main, lets remember that.  Then it keeps going
    and finds that it now contains a call to foo.  It decides to inline foo into
    main, and says "hey, foo has an alloca A, and I have an alloca B from another
    inlined call site, lets reuse it".  The problem with this of course, is that
    the lifetime of A and B are nested, not disjoint.

    Unfortunately I can't create a reasonable testcase for this: the one in the
    PR is both huge and extremely sensitive, because you minor tweaks end up
    causing foo to get inlined into bar too early.  We already have tests for the
    basic alloca merging optimization and this does not break them.

Modified:
    llvm/branches/Apple/whitney/lib/Transforms/IPO/Inliner.cpp

Modified: llvm/branches/Apple/whitney/lib/Transforms/IPO/Inliner.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/branches/Apple/whitney/lib/Transforms/IPO/Inliner.cpp?rev=121528&r1=121527&r2=121528&view=diff
==============================================================================
--- llvm/branches/Apple/whitney/lib/Transforms/IPO/Inliner.cpp (original)
+++ llvm/branches/Apple/whitney/lib/Transforms/IPO/Inliner.cpp Fri Dec 10 15:31:48 2010
@@ -74,7 +74,8 @@
 /// inline this call site we attempt to reuse already available allocas or add
 /// any new allocas to the set if not possible.
 static bool InlineCallIfPossible(CallSite CS, InlineFunctionInfo &IFI,
-                                 InlinedArrayAllocasTy &InlinedArrayAllocas) {
+                                 InlinedArrayAllocasTy &InlinedArrayAllocas,
+                                 int InlineHistory) {
   Function *Callee = CS.getCalledFunction();
   Function *Caller = CS.getCaller();
 
@@ -91,7 +92,6 @@
            !Caller->hasFnAttr(Attribute::StackProtectReq))
     Caller->addFnAttr(Attribute::StackProtect);
 
-  
   // Look at all of the allocas that we inlined through this call site.  If we
   // have already inlined other allocas through other calls into this function,
   // then we know that they have disjoint lifetimes and that we can merge them.
@@ -115,6 +115,21 @@
   //
   SmallPtrSet<AllocaInst*, 16> UsedAllocas;
   
+  // When processing our SCC, check to see if CS was inlined from some other
+  // call site.  For example, if we're processing "A" in this code:
+  //   A() { B() }
+  //   B() { x = alloca ... C() }
+  //   C() { y = alloca ... }
+  // Assume that C was not inlined into B initially, and so we're processing A
+  // and decide to inline B into A.  Doing this makes an alloca available for
+  // reuse and makes a callsite (C) available for inlining.  When we process
+  // the C call site we don't want to do any alloca merging between X and Y
+  // because their scopes are not disjoint.  We could make this smarter by
+  // keeping track of the inline history for each alloca in the
+  // InlinedArrayAllocas but this isn't likely to be a significant win.
+  if (InlineHistory != -1)  // Only do merging for top-level call sites in SCC.
+    return true;
+  
   // Loop over all the allocas we have so far and see if they can be merged with
   // a previously inlined alloca.  If not, remember that we had it.
   for (unsigned AllocaNo = 0, e = IFI.StaticAllocas.size();
@@ -416,7 +431,8 @@
           continue;
 
         // Attempt to inline the function.
-        if (!InlineCallIfPossible(CS, InlineInfo, InlinedArrayAllocas))
+        if (!InlineCallIfPossible(CS, InlineInfo, InlinedArrayAllocas,
+                                  InlineHistoryID))
           continue;
         ++NumInlined;