[PATCH] InlineFunction doesn't update InlineFunctionInfo with allocas created for byval arguments

[Julien Lerouge]

On Tue, Apr 08, 2014 at 02:26:52PM -0700, Julien Lerouge wrote:
> On Tue, Apr 08, 2014 at 10:30:44AM -0700, Manman Ren wrote:
> > On Fri, Apr 4, 2014 at 5:15 PM, Julien Lerouge <julien.lerouge at m4x.org>wrote:
> > 
> > > When llvm::InlineFunction inlines a function that has byval arguments,
> > > it creates allocas in the caller. Those allocas aren't inserted in the
> > > InlineFunctionInfo data structure. So after inlining, if the client code
> > > wants to know where are the allocas that were created, it will miss
> > > those.
> > >
> > > Should InlineFunctionInfo contain these allocas, or is the omission
> > > deliberate ?
> > >
> > 
> > Hi Julien,
> > 
> > I don't think the omission is deliberate, but I may be wrong.
> > Do you have a testing case?
> > 
> > Thanks,
> > Manman
> > 
> 
> Yeah, sorry, I'll add one and resend the patch.
> 
> Thanks,
> Julien
> 

Attached below. The test just inlines a function with byval and makes
sure the lifetime marker gets created. Without the fix, no lifetime
marker is created.

-------------- next part --------------
commit d67e36bf3357085b87b2eb2aba41bfafbed6ebeb
Author: Julien Lerouge <jlerouge at apple.com>
Date:   Wed Apr 2 17:07:18 2014 -0700

    Keep byval allocas.

diff --git a/lib/Transforms/Utils/InlineFunction.cpp b/lib/Transforms/Utils/InlineFunction.cpp
index 86def3e..130bcbf 100644
--- a/lib/Transforms/Utils/InlineFunction.cpp
+++ b/lib/Transforms/Utils/InlineFunction.cpp
@@ -368,6 +368,8 @@ static Value *HandleByValArgument(Value *Arg, Instruction *TheCall,
   
   Value *NewAlloca = new AllocaInst(AggTy, 0, Align, Arg->getName(), 
                                     &*Caller->begin()->begin());
+  IFI.StaticAllocas.push_back(cast<AllocaInst>(NewAlloca));
+
   // Emit a memcpy.
   Type *Tys[3] = {VoidPtrTy, VoidPtrTy, Type::getInt64Ty(Context)};
   Function *MemCpyFn = Intrinsic::getDeclaration(Caller->getParent(),
diff --git a/test/Transforms/Inline/byval_lifetime.ll b/test/Transforms/Inline/byval_lifetime.ll
new file mode 100644
index 0000000..7c582c3
--- /dev/null
+++ b/test/Transforms/Inline/byval_lifetime.ll
@@ -0,0 +1,25 @@
+; RUN: opt -S -inline < %s | FileCheck %s
+; END.
+
+; By inlining foo, an alloca is created in main to hold the argument, so a
+; lifetime marker should be generated as well by default.
+
+%struct.foo = type { i32, [16 x i32] }
+
+ at gFoo = global %struct.foo zeroinitializer, align 8
+
+define i32 @foo(%struct.foo* byval align 8 %f, i32 %a) {
+entry:
+  %a1 = getelementptr inbounds %struct.foo* %f, i32 0, i32 1
+  %arrayidx = getelementptr inbounds [16 x i32]* %a1, i32 0, i32 %a
+  %tmp2 = load i32* %arrayidx, align 1
+  ret i32 %tmp2
+}
+
+define i32 @main(i32 %argc, i8** %argv) {
+; CHECK-LABEL: @main
+; CHECK: llvm.lifetime.start
+entry:
+  %call = call i32 @foo(%struct.foo* byval align 8 @gFoo, i32 %argc)
+  ret i32 %call
+}