[llvm] r247313 - [MergeFuncs] Fix callsite attributes in thunk generation

[JF Bastien via llvm-commits]

Author: jfb
Date: Thu Sep 10 13:08:35 2015
New Revision: 247313

URL: http://llvm.org/viewvc/llvm-project?rev=247313&view=rev
Log:
[MergeFuncs] Fix callsite attributes in thunk generation

This change correctly sets the attributes on the callsites
generated in thunks. This makes sure things such as sret, sext, etc.
are correctly set, so that the call can be a proper tailcall.

Also, the transfer of attributes in the replaceDirectCallers function
appears to be unnecessary, but until this is confirmed it will remain.

Author: jrkoenig
Reviewers: dschuff, jfb
Subscribers: llvm-commits, nlewycky
Differential revision: http://reviews.llvm.org/D12581

Modified:
    llvm/trunk/lib/Transforms/IPO/MergeFunctions.cpp
    llvm/trunk/test/Transforms/MergeFunc/apply_function_attributes.ll
    llvm/trunk/test/Transforms/MergeFunc/inttoptr-address-space.ll
    llvm/trunk/test/Transforms/MergeFunc/inttoptr.ll

Modified: llvm/trunk/lib/Transforms/IPO/MergeFunctions.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/MergeFunctions.cpp?rev=247313&r1=247312&r2=247313&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/IPO/MergeFunctions.cpp (original)
+++ llvm/trunk/lib/Transforms/IPO/MergeFunctions.cpp Thu Sep 10 13:08:35 2015
@@ -1559,9 +1559,16 @@ void MergeFunctions::replaceDirectCaller
     CallSite CS(U->getUser());
     if (CS && CS.isCallee(U)) {
       // Transfer the called function's attributes to the call site. Due to the
-      // bitcast we will 'loose' ABI changing attributes because the 'called
+      // bitcast we will 'lose' ABI changing attributes because the 'called
       // function' is no longer a Function* but the bitcast. Code that looks up
       // the attributes from the called function will fail.
+
+      // FIXME: This is not actually true, at least not anymore. The callsite
+      // will always have the same ABI affecting attributes as the callee,
+      // because otherwise the original input has UB. Note that Old and New
+      // always have matching ABI, so no attributes need to be changed.
+      // Transferring other attributes may help other optimizations, but that
+      // should be done uniformly and not in this ad-hoc way.
       auto &Context = New->getContext();
       auto NewFuncAttrs = New->getAttributes();
       auto CallSiteAttrs = CS.getAttributes();
@@ -1656,6 +1663,7 @@ void MergeFunctions::writeThunk(Function
   CallInst *CI = Builder.CreateCall(F, Args);
   CI->setTailCall();
   CI->setCallingConv(F->getCallingConv());
+  CI->setAttributes(F->getAttributes());
   if (NewG->getReturnType()->isVoidTy()) {
     Builder.CreateRetVoid();
   } else {

Modified: llvm/trunk/test/Transforms/MergeFunc/apply_function_attributes.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/MergeFunc/apply_function_attributes.ll?rev=247313&r1=247312&r2=247313&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/MergeFunc/apply_function_attributes.ll (original)
+++ llvm/trunk/test/Transforms/MergeFunc/apply_function_attributes.ll Thu Sep 10 13:08:35 2015
@@ -38,3 +38,10 @@ define void @A(%Opaque_type* sret %a, %D
 ; CHECK:  tail call void bitcast (void (%Opaque_type*, %D2i*, i32*, i32*)* @A to void (%Opaque_type*, %S2i*, i32*, i32*)*)(%Opaque_type* sret %0, %S2i* %1, i32* %2, i32* %3)
 ; CHECK:  ret void
 
+
+; Make sure we transfer the parameter attributes to the call site.
+; CHECK-LABEL: define void @B(%Opaque_type* sret
+; CHECK:  %5 = bitcast
+; CHECK:  tail call void @A(%Opaque_type* sret %0, %D2i* %5, i32* %2, i32* %3)
+; CHECK:  ret void
+

Modified: llvm/trunk/test/Transforms/MergeFunc/inttoptr-address-space.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/MergeFunc/inttoptr-address-space.ll?rev=247313&r1=247312&r2=247313&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/MergeFunc/inttoptr-address-space.ll (original)
+++ llvm/trunk/test/Transforms/MergeFunc/inttoptr-address-space.ll Thu Sep 10 13:08:35 2015
@@ -21,7 +21,7 @@ define internal i8* @func35(%.qux.2585 a
 bb:
 ; CHECK-LABEL: @func35(
 ; CHECK: %[[V2:.+]] = bitcast %.qux.2585 addrspace(1)* %{{.*}} to %.qux.2496 addrspace(1)*
-; CHECK: %[[V3:.+]] = tail call i32 @func10(%.qux.2496 addrspace(1)* %[[V2]])
+; CHECK: %[[V3:.+]] = tail call i32 @func10(%.qux.2496 addrspace(1)* nocapture %[[V2]])
 ; CHECK: %{{.*}} = inttoptr i32 %[[V3]] to i8*
   %tmp = getelementptr inbounds %.qux.2585, %.qux.2585 addrspace(1)* %this, i32 0, i32 2
   %tmp1 = load i8*, i8* addrspace(1)* %tmp, align 4

Modified: llvm/trunk/test/Transforms/MergeFunc/inttoptr.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/MergeFunc/inttoptr.ll?rev=247313&r1=247312&r2=247313&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/MergeFunc/inttoptr.ll (original)
+++ llvm/trunk/test/Transforms/MergeFunc/inttoptr.ll Thu Sep 10 13:08:35 2015
@@ -48,7 +48,7 @@ define internal i8* @func35(%.qux.2585*
 bb:
 ; CHECK-LABEL: @func35(
 ; CHECK: %[[V2:.+]] = bitcast %.qux.2585* %{{.*}} to %.qux.2496*
-; CHECK: %[[V3:.+]] = tail call i32 @func10(%.qux.2496* %[[V2]])
+; CHECK: %[[V3:.+]] = tail call i32 @func10(%.qux.2496* nocapture %[[V2]])
 ; CHECK: %{{.*}} = inttoptr i32 %[[V3]] to i8*
   %tmp = getelementptr inbounds %.qux.2585, %.qux.2585* %this, i32 0, i32 2
   %tmp1 = load i8*, i8** %tmp, align 4