[PGO] Update ICP pass for recent byval type changes

[rnk]

Summary

Fixes verifier errors encountered in PR42413.

[t.p.northover]

llvm/lib/Transforms/Utils/CallPromotionUtils.cpp

419–420 ↗

(On Diff #206743)

It'd be better if we could copy/paste the source byval type.

We'd probably have to handle an unspecified byval type anyway; but it's the difference between code that will carry on working forever and we can prune later, and code that will need modifying again before opaque pointers are a thing.

I'm not asking you to do this, Reid; it's late here but I'll try to come up with a concrete diff tomorrow.

[t.p.northover]

How about this: https://reviews.llvm.org/differential/diff/207034/?revisionID=63842.

[rnk]

Prepare for a future with no poitnee types by checking if a byval type is available. If there is a pointee type and the IR is valid, this will be identical to FormalTy.

[rnk]

In D63842#1562110, @t.p.northover wrote:

How about this: https://reviews.llvm.org/differential/diff/207034/?revisionID=63842.

Looks good to me. I guess I applied those changes to this diff.

[rnk]

I see you haven't approved this patch, but I'll go ahead and land it later today. I'll assume you approve of this change since you wrote it. :)

llvm/trunk/lib/Transforms/Utils/CallPromotionUtils.cpp

for (unsigned ArgNo = 0; ArgNo < CalleeParamNum; ++ArgNo) {
     if (FormalTy != ActualTy) {
       auto *Cast = CastInst::CreateBitOrPointerCast(Arg, FormalTy, "",
                                                     CS.getInstruction());
       CS.setArgument(ArgNo, Cast);
 
       // Remove any incompatible attributes for the argument.
       AttrBuilder ArgAttrs(CallerPAL.getParamAttributes(ArgNo));
       ArgAttrs.remove(AttributeFuncs::typeIncompatible(FormalTy));
+
+      // If byval is used, this must be a pointer type, and the byval type must
+      // match the element type. Update it if present.
+      if (ArgAttrs.getByValType()) {
+        Type *NewTy = Callee->getParamByValType(ArgNo);
+        ArgAttrs.addByValAttr(
+            NewTy ? NewTy : cast<PointerType>(FormalTy)->getElementType());
+      }
+
       NewArgAttrs.push_back(AttributeSet::get(Ctx, ArgAttrs));
       AttributeChanged = true;
     } else
       NewArgAttrs.push_back(CallerPAL.getParamAttributes(ArgNo));
   }
 
   // If the return type of the call site doesn't match that of the callee, cast
   // the returned value to the appropriate type.

llvm/trunk/test/Transforms/PGOProfile/indirect_call_promotion_byval.ll

+; RUN: opt < %s -pgo-icall-prom -S -icp-total-percent-threshold=10 | FileCheck %s
+; RUN: opt < %s -passes=pgo-icall-prom -S -icp-total-percent-threshold=10 | FileCheck %s
+
+; PR42413: Previously the call promotion code did not correctly update the byval
+; attribute. Check that it does. This situation can come up in LTO scenarios
+; where struct types end up not matching.
+
+target triple = "i686-unknown-linux-gnu"
+
+%struct.Foo.1 = type { i32 }
+%struct.Foo.2 = type { i32 }
+
+@foo = common global i32 (%struct.Foo.2*)* null, align 8
+
+define i32 @func4(%struct.Foo.1* byval(%struct.Foo.1) %p) {
+entry:
+  %gep = getelementptr inbounds %struct.Foo.1, %struct.Foo.1* %p, i32 0, i32 0
+  %v = load i32, i32* %gep
+  ret i32 %v
+}
+
+define i32 @func5(%struct.Foo.1* byval %p) {
+entry:
+  %gep = getelementptr inbounds %struct.Foo.1, %struct.Foo.1* %p, i32 0, i32 0
+  %v = load i32, i32* %gep
+  ret i32 %v
+}
+
+define i32 @bar(%struct.Foo.2* %f2) {
+entry:
+  %tmp = load i32 (%struct.Foo.2*)*, i32 (%struct.Foo.2*)** @foo, align 8
+  %call = call i32 %tmp(%struct.Foo.2* byval(%struct.Foo.2) %f2), !prof !1
+  ret i32 %call
+}
+
+!1 = !{!"VP", i32 0, i64 3000, i64 7651369219802541373, i64 1000, i64 3667884930908592509, i64 1000}
+
+
+; CHECK: define i32 @bar(%struct.Foo.2* %f2)
+;     Cast %struct.Foo.2* to %struct.Foo.1* and use byval(%struct.Foo.2).
+; CHECK: %[[cast:[^ ]*]] = bitcast %struct.Foo.2* %f2 to %struct.Foo.1*
+; CHECK: call i32 @func4(%struct.Foo.1* byval(%struct.Foo.1) %[[cast]])
+;     Same but when callee doesn't have explicit byval type.
+; CHECK: %[[cast:[^ ]*]] = bitcast %struct.Foo.2* %f2 to %struct.Foo.1*
+; CHECK: call i32 @func5(%struct.Foo.1* byval(%struct.Foo.1) %[[cast]])
+;     Original call stays the same.
+; CHECK: call i32 %tmp(%struct.Foo.2* byval(%struct.Foo.2) %f2)