[llvm-commits] [llvm] r131399 - in /llvm/trunk: include/llvm/Function.h lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp lib/Transforms/Scalar/TailRecursionElimination.cpp lib/VMCore/Function.cpp test/Transforms/TailCallElim/setjmp.ll

[Eli Friedman]

On Sun, May 15, 2011 at 8:05 PM, Rafael Espindola
<rafael.espindola at gmail.com> wrote:
> Author: rafael
> Date: Sun May 15 22:05:33 2011
> New Revision: 131399
>
> URL: http://llvm.org/viewvc/llvm-project?rev=131399&view=rev
> Log:
> Don't do tail calls in a function that call setjmp. The stack might be
> corrupted when setjmp returns again.
>
> Added:
>    llvm/trunk/test/Transforms/TailCallElim/setjmp.ll
> Modified:
>    llvm/trunk/include/llvm/Function.h
>    llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
>    llvm/trunk/lib/Transforms/Scalar/TailRecursionElimination.cpp
>    llvm/trunk/lib/VMCore/Function.cpp
>
> Modified: llvm/trunk/include/llvm/Function.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Function.h?rev=131399&r1=131398&r2=131399&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/Function.h (original)
> +++ llvm/trunk/include/llvm/Function.h Sun May 15 22:05:33 2011
> @@ -414,6 +414,10 @@
>   ///
>   bool hasAddressTaken(const User** = 0) const;
>
> +  /// callsFunctionThatReturnsTwice - Return true if the function has a call to
> +  /// setjmp or other function that gcc recognizes as "returning twice".
> +  bool callsFunctionThatReturnsTwice() const;
> +
>  private:
>   // Shadow Value::setValueSubclassData with a private forwarding method so that
>   // subclasses cannot accidentally use it.
>
> Modified: llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp?rev=131399&r1=131398&r2=131399&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp (original)
> +++ llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp Sun May 15 22:05:33 2011
> @@ -208,38 +208,6 @@
>   MachineFunctionPass::getAnalysisUsage(AU);
>  }
>
> -/// FunctionCallsSetJmp - Return true if the function has a call to setjmp or
> -/// other function that gcc recognizes as "returning twice". This is used to
> -/// limit code-gen optimizations on the machine function.
> -///
> -/// FIXME: Remove after <rdar://problem/8031714> is fixed.
> -static bool FunctionCallsSetJmp(const Function *F) {
> -  const Module *M = F->getParent();
> -  static const char *ReturnsTwiceFns[] = {
> -    "_setjmp",
> -    "setjmp",
> -    "sigsetjmp",
> -    "setjmp_syscall",
> -    "savectx",
> -    "qsetjmp",
> -    "vfork",
> -    "getcontext"
> -  };
> -
> -  for (unsigned I = 0; I < array_lengthof(ReturnsTwiceFns); ++I)
> -    if (const Function *Callee = M->getFunction(ReturnsTwiceFns[I])) {
> -      if (!Callee->use_empty())
> -        for (Value::const_use_iterator
> -               I = Callee->use_begin(), E = Callee->use_end();
> -             I != E; ++I)
> -          if (const CallInst *CI = dyn_cast<CallInst>(*I))
> -            if (CI->getParent()->getParent() == F)
> -              return true;
> -    }
> -
> -  return false;
> -}
> -
>  /// SplitCriticalSideEffectEdges - Look for critical edges with a PHI value that
>  /// may trap on it.  In this case we have to split the edge so that the path
>  /// through the predecessor block that doesn't go to the phi block doesn't
> @@ -390,7 +358,7 @@
>   }
>
>   // Determine if there is a call to setjmp in the machine function.
> -  MF->setCallsSetJmp(FunctionCallsSetJmp(&Fn));
> +  MF->setCallsSetJmp(Fn.callsFunctionThatReturnsTwice());
>
>   // Replace forward-declared registers with the registers containing
>   // the desired value.
>
> Modified: llvm/trunk/lib/Transforms/Scalar/TailRecursionElimination.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/TailRecursionElimination.cpp?rev=131399&r1=131398&r2=131399&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/Scalar/TailRecursionElimination.cpp (original)
> +++ llvm/trunk/lib/Transforms/Scalar/TailRecursionElimination.cpp Sun May 15 22:05:33 2011
> @@ -59,6 +59,7 @@
>  #include "llvm/Function.h"
>  #include "llvm/Instructions.h"
>  #include "llvm/IntrinsicInst.h"
> +#include "llvm/Module.h"
>  #include "llvm/Pass.h"
>  #include "llvm/Analysis/CaptureTracking.h"
>  #include "llvm/Analysis/InlineCost.h"
> @@ -209,10 +210,10 @@
>     }
>   }
>
> -  // Finally, if this function contains no non-escaping allocas, mark all calls
> -  // in the function as eligible for tail calls (there is no stack memory for
> -  // them to access).
> -  if (!FunctionContainsEscapingAllocas)
> +  // 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)
>         if (CallInst *CI = dyn_cast<CallInst>(I)) {
>
> Modified: llvm/trunk/lib/VMCore/Function.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/VMCore/Function.cpp?rev=131399&r1=131398&r2=131399&view=diff
> ==============================================================================
> --- llvm/trunk/lib/VMCore/Function.cpp (original)
> +++ llvm/trunk/lib/VMCore/Function.cpp Sun May 15 22:05:33 2011
> @@ -24,6 +24,7 @@
>  #include "llvm/Support/Threading.h"
>  #include "SymbolTableListTraitsImpl.h"
>  #include "llvm/ADT/DenseMap.h"
> +#include "llvm/ADT/STLExtras.h"
>  #include "llvm/ADT/StringExtras.h"
>  using namespace llvm;
>
> @@ -406,4 +407,36 @@
>   return false;
>  }
>
> +/// callsFunctionThatReturnsTwice - Return true if the function has a call to
> +/// setjmp or other function that gcc recognizes as "returning twice".
> +///
> +/// FIXME: Remove after <rdar://problem/8031714> is fixed.
> +/// FIXME: Is the obove FIXME valid?
> +bool Function::callsFunctionThatReturnsTwice() const {
> +  const Module *M = this->getParent();
> +  static const char *ReturnsTwiceFns[] = {
> +    "_setjmp",
> +    "setjmp",
> +    "sigsetjmp",
> +    "setjmp_syscall",
> +    "savectx",
> +    "qsetjmp",
> +    "vfork",
> +    "getcontext"
> +  };

I think CodeMetrics::analyzeBasicBlock could use this function as well?

-Eli