[llvm] r186057 - Teach TailRecursionElimination to handle certain cases of nocapture escaping allocas.

Michael Gottesman mgottesman at apple.com
Wed Jul 10 21:40:01 PDT 2013


Author: mgottesman
Date: Wed Jul 10 23:40:01 2013
New Revision: 186057

URL: http://llvm.org/viewvc/llvm-project?rev=186057&view=rev
Log:
Teach TailRecursionElimination to handle certain cases of nocapture escaping allocas.

Without the changes introduced into this patch, if TRE saw any allocas at all,
TRE would not perform TRE *or* mark callsites with the tail marker.

Because TRE runs after mem2reg, this inadequacy is not a death sentence. But
given a callsite A without escaping alloca argument, A may not be able to have
the tail marker placed on it due to a separate callsite B having a write-back
parameter passed in via an argument with the nocapture attribute.

Assume that B is the only other callsite besides A and B only has nocapture
escaping alloca arguments (*NOTE* B may have other arguments that are not passed
allocas). In this case not marking A with the tail marker is unnecessarily
conservative since:

  1. By assumption A has no escaping alloca arguments itself so it can not
     access the caller's stack via its arguments.

  2. Since all of B's escaping alloca arguments are passed as parameters with
     the nocapture attribute, we know that B does not stash said escaping
     allocas in a manner that outlives B itself and thus could be accessed
     indirectly by A.

With the changes introduced by this patch:

  1. If we see any escaping allocas passed as a capturing argument, we do
     nothing and bail early.

  2. If we do not see any escaping allocas passed as captured arguments but we
     do see escaping allocas passed as nocapture arguments:

       i. We do not perform TRE to avoid PR962 since the code generator produces
          significantly worse code for the dynamic allocas that would be created
          by the TRE algorithm.

       ii. If we do not return twice, mark call sites without escaping allocas
           with the tail marker. *NOTE* This excludes functions with escaping
           nocapture allocas.

  3. If we do not see any escaping allocas at all (whether captured or not):

       i. If we do not have usage of setjmp, mark all callsites with the tail
          marker.

       ii. If there are no dynamic/variable sized allocas in the function,
           attempt to perform TRE on all callsites in the function.

Based off of a patch by Nick Lewycky.

rdar://14324281.

Removed:
    llvm/trunk/test/Transforms/TailCallElim/nocapture.ll
Modified:
    llvm/trunk/lib/Transforms/Scalar/TailRecursionElimination.cpp
    llvm/trunk/test/Transforms/TailCallElim/basic.ll

Modified: llvm/trunk/lib/Transforms/Scalar/TailRecursionElimination.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/TailRecursionElimination.cpp?rev=186057&r1=186056&r2=186057&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/TailRecursionElimination.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/TailRecursionElimination.cpp Wed Jul 10 23:40:01 2013
@@ -53,6 +53,7 @@
 #define DEBUG_TYPE "tailcallelim"
 #include "llvm/Transforms/Scalar.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/Statistic.h"
 #include "llvm/Analysis/CaptureTracking.h"
 #include "llvm/Analysis/InlineCost.h"
@@ -69,6 +70,7 @@
 #include "llvm/Support/CFG.h"
 #include "llvm/Support/CallSite.h"
 #include "llvm/Support/Debug.h"
+#include "llvm/Support/ValueHandle.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Transforms/Utils/BasicBlockUtils.h"
 #include "llvm/Transforms/Utils/Local.h"
@@ -129,34 +131,42 @@ void TailCallElim::getAnalysisUsage(Anal
   AU.addRequired<TargetTransformInfo>();
 }
 
-/// AllocaMightEscapeToCalls - Return true if this alloca may be accessed by
-/// callees of this function.  We only do very simple analysis right now, this
-/// could be expanded in the future to use mod/ref information for particular
-/// call sites if desired.
-static bool AllocaMightEscapeToCalls(AllocaInst *AI) {
-  // FIXME: do simple 'address taken' analysis.
-  return true;
-}
+/// CanTRE - Scan the specified basic block for alloca instructions.
+/// If it contains any that are variable-sized or not in the entry block,
+/// returns false.
+static bool CanTRE(AllocaInst *AI) {
+  // Because of PR962, we don't TRE allocas outside the entry block.
+
+  // If this alloca is in the body of the function, or if it is a variable
+  // sized allocation, we cannot tail call eliminate calls marked 'tail'
+  // with this mechanism.
+  BasicBlock *BB = AI->getParent();
+  return BB == &BB->getParent()->getEntryBlock() &&
+         isa<ConstantInt>(AI->getArraySize());
+}
+
+struct AllocaCaptureTracker : public CaptureTracker {
+  AllocaCaptureTracker() : Captured(false) {}
+
+  void tooManyUses() { Captured = true; }
+
+  bool shouldExplore(Use *U) {
+    Value *V = U->getUser();
+    if (isa<CallInst>(V) || isa<InvokeInst>(V))
+      UsesAlloca.insert(V);
+    return true;
+  }
 
-/// CheckForEscapingAllocas - Scan the specified basic block for alloca
-/// instructions.  If it contains any that might be accessed by calls, return
-/// true.
-static bool CheckForEscapingAllocas(BasicBlock *BB,
-                                    bool &CannotTCETailMarkedCall) {
-  bool RetVal = false;
-  for (BasicBlock::iterator I = BB->begin(), E = BB->end(); I != E; ++I)
-    if (AllocaInst *AI = dyn_cast<AllocaInst>(I)) {
-      RetVal |= AllocaMightEscapeToCalls(AI);
-
-      // If this alloca is in the body of the function, or if it is a variable
-      // sized allocation, we cannot tail call eliminate calls marked 'tail'
-      // with this mechanism.
-      if (BB != &BB->getParent()->getEntryBlock() ||
-          !isa<ConstantInt>(AI->getArraySize()))
-        CannotTCETailMarkedCall = true;
-    }
-  return RetVal;
-}
+  bool captured(Use *U) {
+    if (isa<ReturnInst>(U->getUser()))
+      return false;
+    Captured = true;
+    return true;
+  }
+
+  bool Captured;
+  SmallPtrSet<const Value *, 64> UsesAlloca;
+};
 
 bool TailCallElim::runOnFunction(Function &F) {
   // If this function is a varargs function, we won't be able to PHI the args
@@ -168,41 +178,44 @@ bool TailCallElim::runOnFunction(Functio
   bool TailCallsAreMarkedTail = false;
   SmallVector<PHINode*, 8> ArgumentPHIs;
   bool MadeChange = false;
-  bool FunctionContainsEscapingAllocas = false;
 
-  // CannotTCETailMarkedCall - If true, we cannot perform TCE on tail calls
+  // CanTRETailMarkedCall - If false, we cannot perform TRE on tail calls
   // marked with the 'tail' attribute, because doing so would cause the stack
-  // size to increase (real TCE would deallocate variable sized allocas, TCE
+  // size to increase (real TRE would deallocate variable sized allocas, TRE
   // doesn't).
-  bool CannotTCETailMarkedCall = false;
+  bool CanTRETailMarkedCall = true;
+
+  // Find calls that can be marked tail.
+  AllocaCaptureTracker ACT;
+  for (Function::iterator BB = F.begin(), EE = F.end(); BB != EE; ++BB) {
+    for (BasicBlock::iterator I = BB->begin(), E = BB->end(); I != E; ++I) {
+      if (AllocaInst *AI = dyn_cast<AllocaInst>(I)) {
+        CanTRETailMarkedCall &= CanTRE(AI);
+        PointerMayBeCaptured(AI, &ACT);
+        // If any allocas are captured, exit.
+        if (ACT.Captured)
+          return false;
+      }
+    }
+  }
 
-  // Loop over the function, looking for any returning blocks, and keeping track
-  // of whether this function has any non-trivially used allocas.
-  for (Function::iterator BB = F.begin(), E = F.end(); BB != E; ++BB) {
-    if (FunctionContainsEscapingAllocas && CannotTCETailMarkedCall)
-      break;
-
-    FunctionContainsEscapingAllocas |=
-      CheckForEscapingAllocas(BB, CannotTCETailMarkedCall);
-  }
-
-  /// FIXME: The code generator produces really bad code when an 'escaping
-  /// alloca' is changed from being a static alloca to being a dynamic alloca.
-  /// Until this is resolved, disable this transformation if that would ever
-  /// happen.  This bug is PR962.
-  if (FunctionContainsEscapingAllocas)
-    return false;
-
-  // Second pass, change any tail calls to loops.
-  for (Function::iterator BB = F.begin(), E = F.end(); BB != E; ++BB) {
-    if (ReturnInst *Ret = dyn_cast<ReturnInst>(BB->getTerminator())) {
-      bool Change = ProcessReturningBlock(Ret, OldEntry, TailCallsAreMarkedTail,
-                                          ArgumentPHIs,CannotTCETailMarkedCall);
-      if (!Change && BB->getFirstNonPHIOrDbg() == Ret)
-        Change = FoldReturnAndProcessPred(BB, Ret, OldEntry,
-                                          TailCallsAreMarkedTail, ArgumentPHIs,
-                                          CannotTCETailMarkedCall);
-      MadeChange |= Change;
+  // Second pass, change any tail recursive calls to loops.
+  //
+  // FIXME: The code generator produces really bad code when an 'escaping
+  // alloca' is changed from being a static alloca to being a dynamic alloca.
+  // Until this is resolved, disable this transformation if that would ever
+  // happen.  This bug is PR962.
+  if (ACT.UsesAlloca.empty()) {
+    for (Function::iterator BB = F.begin(), E = F.end(); BB != E; ++BB) {
+      if (ReturnInst *Ret = dyn_cast<ReturnInst>(BB->getTerminator())) {
+        bool Change = ProcessReturningBlock(Ret, OldEntry, TailCallsAreMarkedTail,
+                                            ArgumentPHIs, !CanTRETailMarkedCall);
+        if (!Change && BB->getFirstNonPHIOrDbg() == Ret)
+          Change = FoldReturnAndProcessPred(BB, Ret, OldEntry,
+                                            TailCallsAreMarkedTail, ArgumentPHIs,
+                                            !CanTRETailMarkedCall);
+        MadeChange |= Change;
+      }
     }
   }
 
@@ -223,16 +236,24 @@ bool TailCallElim::runOnFunction(Functio
     }
   }
 
-  // Finally, if this function contains no non-escaping allocas, or calls
-  // setjmp, mark all calls in the function as eligible for tail calls
-  //(there is no stack memory for them to access).
-  if (!FunctionContainsEscapingAllocas && !F.callsFunctionThatReturnsTwice())
-    for (Function::iterator BB = F.begin(), E = F.end(); BB != E; ++BB)
-      for (BasicBlock::iterator I = BB->begin(), E = BB->end(); I != E; ++I)
+  // At this point, we know that the function does not have any captured
+  // allocas. If additionally the function does not call setjmp, mark all calls
+  // in the function that do not access stack memory with the tail keyword. This
+  // implies ensuring that there does not exist any path from a call that takes
+  // in an alloca but does not capture it and the call which we wish to mark
+  // with "tail".
+  if (!F.callsFunctionThatReturnsTwice()) {
+    for (Function::iterator BB = F.begin(), E = F.end(); BB != E; ++BB) {
+      for (BasicBlock::iterator I = BB->begin(), E = BB->end(); I != E; ++I) {
         if (CallInst *CI = dyn_cast<CallInst>(I)) {
-          CI->setTailCall();
-          MadeChange = true;
+          if (!ACT.UsesAlloca.count(CI)) {
+            CI->setTailCall();
+            MadeChange = true;
+          }
         }
+      }
+    }
+  }
 
   return MadeChange;
 }

Modified: llvm/trunk/test/Transforms/TailCallElim/basic.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/TailCallElim/basic.ll?rev=186057&r1=186056&r2=186057&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/TailCallElim/basic.ll (original)
+++ llvm/trunk/test/Transforms/TailCallElim/basic.ll Wed Jul 10 23:40:01 2013
@@ -2,6 +2,8 @@
 
 declare void @noarg()
 declare void @use(i32*)
+declare void @use_nocapture(i32* nocapture)
+declare void @use2_nocapture(i32* nocapture, i32* nocapture)
 
 ; Trivial case. Mark @noarg with tail call.
 define void @test0() {
@@ -57,3 +59,87 @@ return:		; preds = %entry
 	ret i32 0
 }
 
+; Make sure that a nocapture pointer does not stop adding a tail call marker to
+; an unrelated call and additionally that we do not mark the nocapture call with
+; a tail call.
+;
+; rdar://14324281
+define void @test4() {
+; CHECK: void @test4
+; CHECK-NOT: tail call void @use_nocapture
+; CHECK: tail call void @noarg()
+; CHECK: ret void
+  %a = alloca i32
+  call void @use_nocapture(i32* %a)
+  call void @noarg()
+  ret void
+}
+
+; Make sure that we do not perform TRE even with a nocapture use. This is due to
+; bad codegen caused by PR962.
+;
+; rdar://14324281.
+define i32* @test5(i32* nocapture %A, i1 %cond) {
+; CHECK: i32* @test5
+; CHECK-NOT: tailrecurse:
+; CHECK: ret i32* null
+  %B = alloca i32
+  br i1 %cond, label %cond_true, label %cond_false
+cond_true:
+  call i32* @test5(i32* %B, i1 false)
+  ret i32* null
+cond_false:
+  call void @use2_nocapture(i32* %A, i32* %B)
+  call void @noarg()
+  ret i32* null
+}
+
+; PR14143: Make sure that we do not mark functions with nocapture allocas with tail.
+;
+; rdar://14324281.
+define void @test6(i32* %a, i32* %b) {
+; CHECK: @test6
+; CHECK-NOT: tail call
+; CHECK: ret void
+  %c = alloca [100 x i8], align 16
+  %tmp = bitcast [100 x i8]* %c to i32*
+  call void @use2_nocapture(i32* %b, i32* %tmp)
+  ret void
+}
+
+; PR14143: Make sure that we do not mark functions with nocapture allocas with tail.
+;
+; rdar://14324281
+define void @test7(i32* %a, i32* %b) nounwind uwtable {
+entry:
+; CHECK: @test7
+; CHECK-NOT: tail call
+; CHECK: ret void
+  %c = alloca [100 x i8], align 16
+  %0 = bitcast [100 x i8]* %c to i32*
+  call void @use2_nocapture(i32* %0, i32* %a)
+  call void @use2_nocapture(i32* %b, i32* %0)
+  ret void
+}
+
+; If we have a mix of escaping captured/non-captured allocas, ensure that we do
+; not do anything including marking callsites with the tail call marker.
+;
+; rdar://14324281.
+define i32* @test8(i32* nocapture %A, i1 %cond) {
+; CHECK: i32* @test8
+; CHECK-NOT: tailrecurse:
+; CHECK-NOT: tail call
+; CHECK: ret i32* null
+  %B = alloca i32
+  %B2 = alloca i32
+  br i1 %cond, label %cond_true, label %cond_false
+cond_true:
+  call void @use(i32* %B2)
+  call i32* @test8(i32* %B, i1 false)
+  ret i32* null
+cond_false:
+  call void @use2_nocapture(i32* %A, i32* %B)
+  call void @noarg()
+  ret i32* null
+}

Removed: llvm/trunk/test/Transforms/TailCallElim/nocapture.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/TailCallElim/nocapture.ll?rev=186056&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/TailCallElim/nocapture.ll (original)
+++ llvm/trunk/test/Transforms/TailCallElim/nocapture.ll (removed)
@@ -1,25 +0,0 @@
-; RUN: opt -tailcallelim -S < %s | FileCheck %s
-; XFAIL: *
-
-declare void @use(i8* nocapture, i8* nocapture)
-
-define i8* @foo(i8* nocapture %A, i1 %cond) {
-; CHECK: tailrecurse:
-; CHECK: %A.tr = phi i8* [ %A, %0 ], [ %B, %cond_true ]
-; CHECK: %cond.tr = phi i1 [ %cond, %0 ], [ false, %cond_true ]
-  %B = alloca i8
-; CHECK: %B = alloca i8
-  br i1 %cond, label %cond_true, label %cond_false
-; CHECK: br i1 %cond.tr, label %cond_true, label %cond_false
-cond_true:
-; CHECK: cond_true:
-; CHECK: br label %tailrecurse
-  call i8* @foo(i8* %B, i1 false)
-  ret i8* null
-cond_false:
-; CHECK: cond_false
-  call void @use(i8* %A, i8* %B)
-; CHECK: tail call void @use(i8* %A.tr, i8* %B)
-  ret i8* null
-; CHECK: ret i8* null
-}





More information about the llvm-commits mailing list