[PATCH] D12581: [MergeFuncs] Fix callsite attributes in thunk generation

[Jason Koenig via llvm-commits]

jrkoenig updated this revision to Diff 33968.
jrkoenig added a comment.

Clarified comment about questionable attribute transfers.


http://reviews.llvm.org/D12581

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

Index: test/Transforms/MergeFunc/inttoptr.ll
===================================================================
--- test/Transforms/MergeFunc/inttoptr.ll
+++ test/Transforms/MergeFunc/inttoptr.ll
@@ -48,7 +48,7 @@
 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
Index: test/Transforms/MergeFunc/inttoptr-address-space.ll
===================================================================
--- test/Transforms/MergeFunc/inttoptr-address-space.ll
+++ test/Transforms/MergeFunc/inttoptr-address-space.ll
@@ -21,7 +21,7 @@
 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
Index: test/Transforms/MergeFunc/apply_function_attributes.ll
===================================================================
--- test/Transforms/MergeFunc/apply_function_attributes.ll
+++ test/Transforms/MergeFunc/apply_function_attributes.ll
@@ -38,3 +38,10 @@
 ; 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
+
Index: lib/Transforms/IPO/MergeFunctions.cpp
===================================================================
--- lib/Transforms/IPO/MergeFunctions.cpp
+++ lib/Transforms/IPO/MergeFunctions.cpp
@@ -1552,6 +1552,13 @@
     ++UI;
     CallSite CS(U->getUser());
     if (CS && CS.isCallee(U)) {
+      // 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.
+
       // Transfer the called function's attributes to the call site. Due to the
       // bitcast we will 'loose' ABI changing attributes because the 'called
       // function' is no longer a Function* but the bitcast. Code that looks up
@@ -1650,6 +1657,7 @@
   CallInst *CI = Builder.CreateCall(F, Args);
   CI->setTailCall();
   CI->setCallingConv(F->getCallingConv());
+  CI->setAttributes(F->getAttributes());
   if (NewG->getReturnType()->isVoidTy()) {
     Builder.CreateRetVoid();
   } else {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D12581.33968.patch
Type: text/x-patch
Size: 3314 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150903/21bfcccb/attachment.bin>