PATCH: Inliner: Don't generate memcpy calls for byval readonly parameters

[Tom Stellard]

Hi,

The function inliner avoids generating memcpy calls when the parameter
is marked byval and the function is marked readonly.  The attached
patch extends this behavior to the case where the function parameter is
readonly, but the function isn't.

Please review.

Thanks,
Tom
-------------- next part --------------
>From 4e0883ddf24a4a30bbea5053c13b81b19cf60937 Mon Sep 17 00:00:00 2001
From: Vincent Lejeune <vljn at ovi.com>
Date: Tue, 22 Oct 2013 00:18:58 +0200
Subject: [PATCH] Inliner: Handle readonly attribute per argument when adding
 memcpy

Patch by: Vincent Lejeune
---
 lib/Transforms/Utils/InlineFunction.cpp |  9 ++++++---
 test/Transforms/Inline/byval.ll         | 33 +++++++++++++++++++++++++++++----
 2 files changed, 35 insertions(+), 7 deletions(-)

diff --git a/lib/Transforms/Utils/InlineFunction.cpp b/lib/Transforms/Utils/InlineFunction.cpp
index dabb67b..6a01b32 100644
--- a/lib/Transforms/Utils/InlineFunction.cpp
+++ b/lib/Transforms/Utils/InlineFunction.cpp
@@ -337,7 +337,9 @@ static void UpdateCallGraphAfterInlining(CallSite CS,
 
 /// HandleByValArgument - When inlining a call site that has a byval argument,
 /// we have to make the implicit memcpy explicit by adding it.
-static Value *HandleByValArgument(Value *Arg, Instruction *TheCall,
+static Value *HandleByValArgument(Value *Arg,
+                                  const Argument *ArgAsArgument,
+                                  Instruction *TheCall,
                                   const Function *CalledFunc,
                                   InlineFunctionInfo &IFI,
                                   unsigned ByValAlignment) {
@@ -346,7 +348,7 @@ static Value *HandleByValArgument(Value *Arg, Instruction *TheCall,
   // If the called function is readonly, then it could not mutate the caller's
   // copy of the byval'd memory.  In this case, it is safe to elide the copy and
   // temporary.
-  if (CalledFunc->onlyReadsMemory()) {
+  if (CalledFunc->onlyReadsMemory() || ArgAsArgument->onlyReadsMemory()) {
     // If the byval argument has a specified alignment that is greater than the
     // passed in pointer, then we either have to round up the input pointer or
     // give up on this transformation.
@@ -588,13 +590,14 @@ bool llvm::InlineFunction(CallSite CS, InlineFunctionInfo &IFI,
     for (Function::const_arg_iterator I = CalledFunc->arg_begin(),
          E = CalledFunc->arg_end(); I != E; ++I, ++AI, ++ArgNo) {
       Value *ActualArg = *AI;
+      const Argument *Arg = I;
 
       // When byval arguments actually inlined, we need to make the copy implied
       // by them explicit.  However, we don't do this if the callee is readonly
       // or readnone, because the copy would be unneeded: the callee doesn't
       // modify the struct.
       if (CS.isByValArgument(ArgNo)) {
-        ActualArg = HandleByValArgument(ActualArg, TheCall, CalledFunc, IFI,
+        ActualArg = HandleByValArgument(ActualArg, Arg, TheCall, CalledFunc, IFI,
                                         CalledFunc->getParamAlignment(ArgNo+1));
  
         // Calls that we inline may use the new alloca, so we need to clear
diff --git a/test/Transforms/Inline/byval.ll b/test/Transforms/Inline/byval.ll
index e601faf..aceae11 100644
--- a/test/Transforms/Inline/byval.ll
+++ b/test/Transforms/Inline/byval.ll
@@ -25,7 +25,7 @@ entry:
 	store i64 2, i64* %tmp4, align 4
 	call void @f( %struct.ss* byval  %S ) nounwind 
 	ret i32 0
-; CHECK: @test1()
+; CHECK-LABEL: @test1()
 ; CHECK: %S1 = alloca %struct.ss
 ; CHECK: %S = alloca %struct.ss
 ; CHECK: call void @llvm.memcpy
@@ -52,7 +52,7 @@ entry:
 	store i64 2, i64* %tmp4, align 4
 	%X = call i32 @f2( %struct.ss* byval  %S ) nounwind 
 	ret i32 %X
-; CHECK: @test2()
+; CHECK-LABEL: @test2()
 ; CHECK: %S = alloca %struct.ss
 ; CHECK-NOT: call void @llvm.memcpy
 ; CHECK: ret i32
@@ -74,7 +74,7 @@ entry:
 	%S = alloca %struct.ss, align 1  ;; May not be aligned.
 	call void @f3( %struct.ss* byval align 64 %S) nounwind 
 	ret void
-; CHECK: @test3()
+; CHECK-LABEL: @test3()
 ; CHECK: %S1 = alloca %struct.ss, align 64
 ; CHECK: %S = alloca %struct.ss
 ; CHECK: call void @llvm.memcpy
@@ -97,10 +97,35 @@ entry:
 	%S = alloca %struct.ss, align 2		; <%struct.ss*> [#uses=4]
 	%X = call i32 @f4( %struct.ss* byval align 64 %S ) nounwind 
 	ret i32 %X
-; CHECK: @test4()
+; CHECK-LABEL: @test4()
 ; CHECK: %S = alloca %struct.ss, align 64
 ; CHECK-NOT: call void @llvm.memcpy
 ; CHECK: call void @g3
 ; CHECK: ret i32 4
 }
 
+; Inlining a byval struct should NOT cause an explicit copy
+; into an alloca if the parameter is readonly
+
+define internal i32 @f5(%struct.ss* byval readonly %b) nounwind {
+entry:
+	%tmp = getelementptr %struct.ss* %b, i32 0, i32 0		; <i32*> [#uses=2]
+	%tmp1 = load i32* %tmp, align 4		; <i32> [#uses=1]
+	%tmp2 = add i32 %tmp1, 1		; <i32> [#uses=1]
+	ret i32 %tmp2
+}
+
+define i32 @test5() nounwind  {
+entry:
+	%S = alloca %struct.ss		; <%struct.ss*> [#uses=4]
+	%tmp1 = getelementptr %struct.ss* %S, i32 0, i32 0		; <i32*> [#uses=1]
+	store i32 1, i32* %tmp1, align 8
+	%tmp4 = getelementptr %struct.ss* %S, i32 0, i32 1		; <i64*> [#uses=1]
+	store i64 2, i64* %tmp4, align 4
+	%X = call i32 @f5( %struct.ss* byval  %S ) nounwind 
+	ret i32 %X
+; CHECK-LABEL: @test5()
+; CHECK: %S = alloca %struct.ss
+; CHECK-NOT: call void @llvm.memcpy
+; CHECK: ret i32
+}
-- 
1.7.11.4