Browse Source

Transforms: reapply SVN r219899

This restores the commit from SVN r219899 with an additional change to ensure
that the CodeGen is correct for the case that was identified as being incorrect
(originally PR7272).

In the case that during inlining we need to synthesize a value on the stack
(i.e. for passing a value byval), then any function involving that alloca must
be stripped of its tailness as the restriction that it does not access the
parent's stack no longer holds.  Unfortunately, a single alloca can cause a
rippling effect through out the inlining as the value may be aliased or may be
mutated through an escaped external call.  As such, we simply track if an alloca
has been introduced in the frame during inlining, and strip any tail calls.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@220811 91177308-0d34-0410-b5e6-96231b3b80d8
Saleem Abdulrasool 10 years ago
parent
commit
586cf3d88d

+ 2 - 8
lib/Transforms/Scalar/TailRecursionElimination.cpp

@@ -253,12 +253,7 @@ bool TailCallElim::markTails(Function &F, bool &AllCallsAreTailCalls) {
     return false;
     return false;
   AllCallsAreTailCalls = true;
   AllCallsAreTailCalls = true;
 
 
-  // The local stack holds all alloca instructions and all byval arguments.
   AllocaDerivedValueTracker Tracker;
   AllocaDerivedValueTracker Tracker;
-  for (Argument &Arg : F.args()) {
-    if (Arg.hasByValAttr())
-      Tracker.walk(&Arg);
-  }
   for (auto &BB : F) {
   for (auto &BB : F) {
     for (auto &I : BB)
     for (auto &I : BB)
       if (AllocaInst *AI = dyn_cast<AllocaInst>(&I))
       if (AllocaInst *AI = dyn_cast<AllocaInst>(&I))
@@ -314,9 +309,8 @@ bool TailCallElim::markTails(Function &F, bool &AllCallsAreTailCalls) {
         for (auto &Arg : CI->arg_operands()) {
         for (auto &Arg : CI->arg_operands()) {
           if (isa<Constant>(Arg.getUser()))
           if (isa<Constant>(Arg.getUser()))
             continue;
             continue;
-          if (Argument *A = dyn_cast<Argument>(Arg.getUser()))
-            if (!A->hasByValAttr())
-              continue;
+          if (isa<Argument>(Arg.getUser()))
+            continue;
           SafeToTail = false;
           SafeToTail = false;
           break;
           break;
         }
         }

+ 16 - 3
lib/Transforms/Utils/InlineFunction.cpp

@@ -743,7 +743,8 @@ static void HandleByValArgumentInit(Value *Dst, Value *Src, Module *M,
 static Value *HandleByValArgument(Value *Arg, Instruction *TheCall,
 static Value *HandleByValArgument(Value *Arg, Instruction *TheCall,
                                   const Function *CalledFunc,
                                   const Function *CalledFunc,
                                   InlineFunctionInfo &IFI,
                                   InlineFunctionInfo &IFI,
-                                  unsigned ByValAlignment) {
+                                  unsigned ByValAlignment,
+                                  bool &AddedNewAllocas) {
   PointerType *ArgTy = cast<PointerType>(Arg->getType());
   PointerType *ArgTy = cast<PointerType>(Arg->getType());
   Type *AggTy = ArgTy->getElementType();
   Type *AggTy = ArgTy->getElementType();
 
 
@@ -785,6 +786,7 @@ static Value *HandleByValArgument(Value *Arg, Instruction *TheCall,
   
   
   // Uses of the argument in the function should use our new alloca
   // Uses of the argument in the function should use our new alloca
   // instead.
   // instead.
+  AddedNewAllocas = true;
   return NewAlloca;
   return NewAlloca;
 }
 }
 
 
@@ -958,6 +960,7 @@ bool llvm::InlineFunction(CallSite CS, InlineFunctionInfo &IFI,
   SmallVector<ReturnInst*, 8> Returns;
   SmallVector<ReturnInst*, 8> Returns;
   ClonedCodeInfo InlinedFunctionInfo;
   ClonedCodeInfo InlinedFunctionInfo;
   Function::iterator FirstNewBlock;
   Function::iterator FirstNewBlock;
+  bool AddedNewAllocas = false;
 
 
   { // Scope to destroy VMap after cloning.
   { // Scope to destroy VMap after cloning.
     ValueToValueMapTy VMap;
     ValueToValueMapTy VMap;
@@ -981,7 +984,8 @@ bool llvm::InlineFunction(CallSite CS, InlineFunctionInfo &IFI,
       // modify the struct.
       // modify the struct.
       if (CS.isByValArgument(ArgNo)) {
       if (CS.isByValArgument(ArgNo)) {
         ActualArg = HandleByValArgument(ActualArg, TheCall, CalledFunc, IFI,
         ActualArg = HandleByValArgument(ActualArg, TheCall, CalledFunc, IFI,
-                                        CalledFunc->getParamAlignment(ArgNo+1));
+                                        CalledFunc->getParamAlignment(ArgNo+1),
+                                        AddedNewAllocas);
         if (ActualArg != *AI)
         if (ActualArg != *AI)
           ByValInit.push_back(std::make_pair(ActualArg, (Value*) *AI));
           ByValInit.push_back(std::make_pair(ActualArg, (Value*) *AI));
       }
       }
@@ -1096,9 +1100,18 @@ bool llvm::InlineFunction(CallSite CS, InlineFunctionInfo &IFI,
         //    f -> musttail g ->     tail f  ==>  f ->     tail f
         //    f -> musttail g ->     tail f  ==>  f ->     tail f
         //    f ->          g -> musttail f  ==>  f ->          f
         //    f ->          g -> musttail f  ==>  f ->          f
         //    f ->          g ->     tail f  ==>  f ->          f
         //    f ->          g ->     tail f  ==>  f ->          f
+        //
+        // If an alloca was introduced in the frame due to a byval parameter
+        // being passed to a subsequent call, tail calls must have the tail
+        // stripped as they may not access variables in the caller's stack.
+        // A single alloca ripples through out as the alloca may be aliased by
+        // bitcasts or may escape and be mutated outside of the function.
         CallInst::TailCallKind ChildTCK = CI->getTailCallKind();
         CallInst::TailCallKind ChildTCK = CI->getTailCallKind();
         ChildTCK = std::min(CallSiteTailKind, ChildTCK);
         ChildTCK = std::min(CallSiteTailKind, ChildTCK);
-        CI->setTailCallKind(ChildTCK);
+        if (AddedNewAllocas)
+          CI->setTailCallKind(CallInst::TCK_None);
+        else
+          CI->setTailCallKind(ChildTCK);
         InlinedMustTailCalls |= CI->isMustTailCall();
         InlinedMustTailCalls |= CI->isMustTailCall();
 
 
         // Calls inlined through a 'nounwind' call site should be marked
         // Calls inlined through a 'nounwind' call site should be marked

+ 1 - 1
test/Transforms/Inline/byval-tail-call.ll

@@ -34,7 +34,7 @@ define void @frob(i32* %x) {
 ; CHECK: %[[VAL:.*]] = load i32* %x
 ; CHECK: %[[VAL:.*]] = load i32* %x
 ; CHECK: store i32 %[[VAL]], i32* %[[POS]]
 ; CHECK: store i32 %[[VAL]], i32* %[[POS]]
 ; CHECK: {{^ *}}call void @ext(i32* %[[POS]]
 ; CHECK: {{^ *}}call void @ext(i32* %[[POS]]
-; CHECK: tail call void @ext(i32* null)
+; CHECK: {{^ *}}call void @ext(i32* null)
 ; CHECK: ret void
 ; CHECK: ret void
   tail call void @qux(i32* byval %x)
   tail call void @qux(i32* byval %x)
   ret void
   ret void

+ 0 - 36
test/Transforms/Inline/inline-tail.ll

@@ -49,42 +49,6 @@ define void @test_musttail_basic_a(i32* %p) {
   ret void
   ret void
 }
 }
 
 
-; Don't insert lifetime end markers here, the lifetime is trivially over due
-; the return.
-; CHECK: define void @test_byval_a(
-; CHECK: musttail call void @test_byval_c(
-; CHECK-NEXT: ret void
-
-declare void @test_byval_c(i32* byval %p)
-define internal void @test_byval_b(i32* byval %p) {
-  musttail call void @test_byval_c(i32* byval %p)
-  ret void
-}
-define void @test_byval_a(i32* byval %p) {
-  musttail call void @test_byval_b(i32* byval %p)
-  ret void
-}
-
-; Don't insert a stack restore, we're about to return.
-; CHECK: define void @test_dynalloca_a(
-; CHECK: call i8* @llvm.stacksave(
-; CHECK: alloca i8, i32 %n
-; CHECK: musttail call void @test_dynalloca_c(
-; CHECK-NEXT: ret void
-
-declare void @escape(i8* %buf)
-declare void @test_dynalloca_c(i32* byval %p, i32 %n)
-define internal void @test_dynalloca_b(i32* byval %p, i32 %n) alwaysinline {
-  %buf = alloca i8, i32 %n              ; dynamic alloca
-  call void @escape(i8* %buf)           ; escape it
-  musttail call void @test_dynalloca_c(i32* byval %p, i32 %n)
-  ret void
-}
-define void @test_dynalloca_a(i32* byval %p, i32 %n) {
-  musttail call void @test_dynalloca_b(i32* byval %p, i32 %n)
-  ret void
-}
-
 ; We can't merge the returns.
 ; We can't merge the returns.
 ; CHECK: define void @test_multiret_a(
 ; CHECK: define void @test_multiret_a(
 ; CHECK: musttail call void @test_multiret_c(
 ; CHECK: musttail call void @test_multiret_c(

+ 58 - 0
test/Transforms/Inline/inlined-allocas.ll

@@ -0,0 +1,58 @@
+; RUN: opt -dse -inline -S %s | FileCheck %s
+
+declare void @external(i32* byval)
+declare i32 @identity(i32* byval)
+
+; An alloca in the inlinee should not force the tail to be stripped
+
+define void @inlinee_with_alloca() {
+  %local = alloca i32
+  store i32 42, i32* %local, align 4
+  tail call void @external(i32* byval %local)
+  ret void
+}
+
+define void @inliner_without_alloca() {
+  tail call void @inlinee_with_alloca()
+  ret void
+}
+
+; CHECK-LABEL: inliner_without_alloca
+; CHECK-NEXT: %local.i = alloca i32
+; CHECK: store i32 42, i32* %local.i
+; CHECK: tail call void @external
+; CHECK: ret
+
+; An alloca in the inliner should not force the tail to be stripped
+
+define i32 @inliner_with_alloca() {
+  %local = alloca i32
+  store i32 42, i32* %local, align 4
+  %1 = tail call i32 @identity(i32* byval %local)
+  ret i32 %1
+}
+
+; CHECK-LABEL: inliner_with_alloca
+; CHECK: %local = alloca i32
+; CHECK: store i32 42, i32* %local
+; CHECK: %1 = tail call i32 @identity
+; CHECK: ret i32 %1
+
+; Force the synthesis of the value through the byval parameter.
+; The alloca should force the tail to be stripped
+
+define void @inlinee_with_passthru(i32* byval %value) {
+  tail call void @external(i32* byval %value)
+  ret void
+}
+
+define void @strip_tail(i32* %value) {
+  tail call void @inlinee_with_passthru(i32* %value)
+  ret void
+}
+
+; CHECK-LABEL: strip_tail
+; CHECK: %value1 = alloca i32
+; CHECK: {{^ *}}call void @external
+; CHECK: ret void
+

+ 1 - 1
test/Transforms/TailCallElim/basic.ll

@@ -147,7 +147,7 @@ cond_false:
 ; Don't tail call if a byval arg is captured.
 ; Don't tail call if a byval arg is captured.
 define void @test9(i32* byval %a) {
 define void @test9(i32* byval %a) {
 ; CHECK-LABEL: define void @test9(
 ; CHECK-LABEL: define void @test9(
-; CHECK: {{^ *}}call void @use(
+; CHECK: tail call void @use(
   call void @use(i32* %a)
   call void @use(i32* %a)
   ret void
   ret void
 }
 }

+ 34 - 0
test/Transforms/TailCallElim/byval.ll

@@ -0,0 +1,34 @@
+; RUN: opt -mtriple i386 -Os -S %s -o - | FileCheck %s
+; RUN: opt -mtriple x86_64 -Os -S %s -o - | FileCheck %s
+; RUN: opt -mtriple armv7 -Os -S %s -o - | FileCheck %s
+
+%struct.D16 = type { [16 x double] }
+
+declare void @_Z2OpP3D16PKS_S2_(%struct.D16*, %struct.D16*, %struct.D16*)
+
+define void @_Z7TestRefRK3D16S1_(%struct.D16* noalias sret %agg.result, %struct.D16* %RHS, %struct.D16* %LHS) {
+  %1 = alloca %struct.D16*, align 8
+  %2 = alloca %struct.D16*, align 8
+  store %struct.D16* %RHS, %struct.D16** %1, align 8
+  store %struct.D16* %LHS, %struct.D16** %2, align 8
+  %3 = load %struct.D16** %1, align 8
+  %4 = load %struct.D16** %2, align 8
+  call void @_Z2OpP3D16PKS_S2_(%struct.D16* %agg.result, %struct.D16* %3, %struct.D16* %4)
+  ret void
+}
+
+; CHECK: define void @_Z7TestRefRK3D16S1_({{.*}}) {
+; CHECK:   tail call void @_Z2OpP3D16PKS_S2_(%struct.D16* %agg.result, %struct.D16* %RHS, %struct.D16* %LHS)
+; CHECK:   ret void
+; CHECK: }
+
+define void @_Z7TestVal3D16S_(%struct.D16* noalias sret %agg.result, %struct.D16* byval align 8 %RHS, %struct.D16* byval align 8 %LHS) {
+  call void @_Z2OpP3D16PKS_S2_(%struct.D16* %agg.result, %struct.D16* %RHS, %struct.D16* %LHS)
+  ret void
+}
+
+; CHECK: define void @_Z7TestVal3D16S_({{.*}}) {
+; CHECK:   tail call void @_Z2OpP3D16PKS_S2_(%struct.D16* %agg.result, %struct.D16* %RHS, %struct.D16* %LHS)
+; CHECK:   ret void
+; CHECK: }
+