[llvm-commits] [llvm] r166407 - in /llvm/trunk: lib/Transforms/Scalar/TailRecursionElimination.cpp test/Transforms/TailCallElim/nocapture.ll

Argyrios Kyrtzidis kyrtzidis at apple.com
Mon Oct 22 11:22:14 PDT 2012


Hi Nick,

I reverted this in r166424 because it crashes some (highly recursive) clang analyzer tests.

In order to reproduce configure as:

configure --build=x86_64-apple-darwin11 --host=x86_64-apple-darwin11 --enable-optimized --disable-assertions --disable-bindings --with-llvmcc=clang --without-llvmgcc --without-llvmgxx --enable-keep-symbols CC=/path/to/built/clang CXX=/path/to/built/clang++

the tests that crash:

Clang :: Analysis/dead-stores.cpp
Clang :: Analysis/inlining/stl.cpp
Clang :: Analysis/templates.cpp

-Argyrios

On Oct 21, 2012, at 8:03 PM, Nick Lewycky <nicholas at mxc.ca> wrote:

> Author: nicholas
> Date: Sun Oct 21 22:03:52 2012
> New Revision: 166407
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=166407&view=rev
> Log:
> Reapply r166405, teaching tailcallelim to be smarter about nocapture, with a
> very small but very important bugfix:
>  bool shouldExplore(Use *U) {
>    Value *V = U->get();
>    if (isa<CallInst>(V) || isa<InvokeInst>(V))
>    [...]
> should have read:
>  bool shouldExplore(Use *U) {
>    Value *V = U->getUser();
>    if (isa<CallInst>(V) || isa<InvokeInst>(V))
> Fixes PR14143!
> 
> 
> Modified:
>    llvm/trunk/lib/Transforms/Scalar/TailRecursionElimination.cpp
>    llvm/trunk/test/Transforms/TailCallElim/nocapture.ll
> 
> Modified: llvm/trunk/lib/Transforms/Scalar/TailRecursionElimination.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/TailRecursionElimination.cpp?rev=166407&r1=166406&r2=166407&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/Scalar/TailRecursionElimination.cpp (original)
> +++ llvm/trunk/lib/Transforms/Scalar/TailRecursionElimination.cpp Sun Oct 21 22:03:52 2012
> @@ -69,6 +69,7 @@
> #include "llvm/Support/CFG.h"
> #include "llvm/Support/Debug.h"
> #include "llvm/Support/raw_ostream.h"
> +#include "llvm/Support/ValueHandle.h"
> #include "llvm/ADT/Statistic.h"
> #include "llvm/ADT/STLExtras.h"
> using namespace llvm;
> @@ -117,35 +118,46 @@
>   return new TailCallElim();
> }
> 
> -/// 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.
> 
> -/// 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;
> +  // 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.push_back(V);
> +
> +    return true;
> +  }
> +
> +  bool captured(Use *U) {
> +    if (isa<ReturnInst>(U->getUser()))
> +      return false;
> +
> +    Captured = true;
> +    return true;
> +  }
> +
> +  SmallVector<WeakVH, 64> UsesAlloca;
> +
> +  bool Captured;
> +};
> +
> bool TailCallElim::runOnFunction(Function &F) {
>   // If this function is a varargs function, we won't be able to PHI the args
>   // right, so don't even try to convert it...
> @@ -157,38 +169,34 @@
>   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;
> -
> -  // 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;
> +  bool CanTRETailMarkedCall = true;
> 
> -    FunctionContainsEscapingAllocas |=
> -      CheckForEscapingAllocas(BB, CannotTCETailMarkedCall);
> +  // 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 (ACT.Captured)
> +          return false;
> +      }
> +    }
>   }
> 
> -  /// 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.
> +  // Second pass, change any tail recursive 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);
> +                                          ArgumentPHIs, !CanTRETailMarkedCall);
>       if (!Change && BB->getFirstNonPHIOrDbg() == Ret)
>         Change = FoldReturnAndProcessPred(BB, Ret, OldEntry,
>                                           TailCallsAreMarkedTail, ArgumentPHIs,
> -                                          CannotTCETailMarkedCall);
> +                                          !CanTRETailMarkedCall);
>       MadeChange |= Change;
>     }
>   }
> @@ -210,21 +218,24 @@
>     }
>   }
> 
> -  // 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).
> +  // Finally, if this function contains no non-escaping allocas and doesn't
> +  // call setjmp, mark all calls in the function as eligible for tail calls
> +  // (there is no stack memory for them to access).
> +  std::sort(ACT.UsesAlloca.begin(), ACT.UsesAlloca.end());
> +
>   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)
> -        if (CallInst *CI = dyn_cast<CallInst>(I)) {
> -          CI->setTailCall();
> -          MadeChange = true;
> -        }
> +        if (CallInst *CI = dyn_cast<CallInst>(I))
> +          if (!std::binary_search(ACT.UsesAlloca.begin(), ACT.UsesAlloca.end(),
> +                                  CI)) {
> +            CI->setTailCall();
> +            MadeChange = true;
> +          }
> 
>   return MadeChange;
> }
> 
> -
> /// CanMoveAboveCall - Return true if it is safe to move the specified
> /// instruction from after the call to before the call, assuming that all
> /// instructions between the call and this instruction are movable.
> 
> Modified: llvm/trunk/test/Transforms/TailCallElim/nocapture.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/TailCallElim/nocapture.ll?rev=166407&r1=166406&r2=166407&view=diff
> ==============================================================================
> --- llvm/trunk/test/Transforms/TailCallElim/nocapture.ll (original)
> +++ llvm/trunk/test/Transforms/TailCallElim/nocapture.ll Sun Oct 21 22:03:52 2012
> @@ -1,9 +1,9 @@
> ; RUN: opt %s -tailcallelim -S | FileCheck %s
> -; XFAIL: *
> 
> declare void @use(i8* nocapture, i8* nocapture)
> +declare void @boring()
> 
> -define i8* @foo(i8* nocapture %A, i1 %cond) {
> +define i8* @test1(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 ]
> @@ -14,12 +14,27 @@
> cond_true:
> ; CHECK: cond_true:
> ; CHECK: br label %tailrecurse
> -  call i8* @foo(i8* %B, i1 false)
> +  call i8* @test1(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)
> +; CHECK: call void @use(i8* %A.tr, i8* %B)
> +  call void @boring()
> +; CHECK: tail call void @boring()
>   ret i8* null
> ; CHECK: ret i8* null
> }
> +
> +; PR14143
> +define void @test2(i8* %a, i8* %b) {
> +; CHECK: @test2
> +; CHECK-NOT: tail call
> +; CHECK: ret void
> +  %c = alloca [100 x i8], align 16
> +  %tmp = bitcast [100 x i8]* %c to i8*
> +  call void @llvm.memcpy.p0i8.p0i8.i64(i8* %b, i8* %tmp, i64 100, i32 1, i1 false)
> +  ret void
> +}
> +
> +declare void @llvm.memcpy.p0i8.p0i8.i64(i8*, i8*, i64, i32, i1)
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits




More information about the llvm-commits mailing list