[CallPromotion] Fix byval/inalloca handling

[aeubanks]

Summary

If the previously indirect call contained a byval/inalloca attribute but
the callee did not have the same attribute, we'd end up not updating the
call's type attribute.

Followup to D63842.

Fixes PR51397.

[aeubanks]

the alternative is to not allow promotion to a call with differing ABI attributes

[efriedma]

I'd prefer to prohibit the promotion.

In general, when we see a mismatch in the argument list, we want to be conservative; ABI rules are complicated, so sometimes different frontends will emit different IR signatures for a given function which should be ABI-compatible. An argument list with byval is effectively a different signature from an argument list without byval.

[aeubanks]

In D107770#2942427, @efriedma wrote:

I'd prefer to prohibit the promotion.

In general, when we see a mismatch in the argument list, we want to be conservative; ABI rules are complicated, so sometimes different frontends will emit different IR signatures for a given function which should be ABI-compatible. An argument list with byval is effectively a different signature from an argument list without byval.

If we do indirect call promotion guarded by checking the function addresses, it should be fine in terms of correctness, we'd just presumably never do the direct call. I was trying to preserve the current state of allowing ABI-incompatible indirect call promotion. But we might as well just do what you said.
https://reviews.llvm.org/D107998

llvm/lib/Transforms/Utils/CallPromotionUtils.cpp

for (unsigned ArgNo = 0; ArgNo < CalleeParamNum; ++ArgNo) {
     Type *ActualTy = Arg->getType();
     if (FormalTy != ActualTy) {
       auto *Cast = CastInst::CreateBitOrPointerCast(Arg, FormalTy, "", &CB);
       CB.setArgOperand(ArgNo, Cast);
 
       // Remove any incompatible attributes for the argument.
       AttrBuilder ArgAttrs(CallerPAL.getParamAttributes(ArgNo));
       ArgAttrs.remove(AttributeFuncs::typeIncompatible(FormalTy));
+      ArgAttrs.removeAttribute(Attribute::ByVal);
+      ArgAttrs.removeAttribute(Attribute::InAlloca);
 
       // 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())
+      if (Callee->getParamByValType(ArgNo))
         ArgAttrs.addByValAttr(Callee->getParamByValType(ArgNo));
+      if (Callee->getParamInAllocaType(ArgNo))
+        ArgAttrs.addByValAttr(Callee->getParamInAllocaType(ArgNo));
 
       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

llvm/test/Transforms/PGOProfile/mismatched-byval.ll

+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -passes=pgo-icall-prom -profile-summary-hot-count=10 -S < %s | FileCheck %s
+
+define void @a(i8* %0) !prof !0 {
+; CHECK-LABEL: @a(
+; CHECK-NEXT:    ret void
+;
+  ret void
+}
+
+define void @b(void (i64*)** %v, i64* %p) !prof !1 {
+; CHECK-LABEL: @b(
+; CHECK-NEXT:    [[A:%.*]] = load void (i64*)*, void (i64*)** [[V:%.*]], align 8
+; CHECK-NEXT:    [[TMP1:%.*]] = icmp eq void (i64*)* [[A]], bitcast (void (i8*)* @a to void (i64*)*)
+; CHECK-NEXT:    br i1 [[TMP1]], label [[IF_TRUE_DIRECT_TARG:%.*]], label [[IF_FALSE_ORIG_INDIRECT:%.*]], !prof [[PROF2:![0-9]+]]
+; CHECK:       if.true.direct_targ:
+; CHECK-NEXT:    [[TMP2:%.*]] = bitcast i64* [[P:%.*]] to i8*
+; CHECK-NEXT:    call void @a(i8* [[TMP2]])
+; CHECK-NEXT:    br label [[IF_END_ICP:%.*]]
+; CHECK:       if.false.orig_indirect:
+; CHECK-NEXT:    call void [[A]](i64* byval(i64) [[P]])
+; CHECK-NEXT:    br label [[IF_END_ICP]]
+; CHECK:       if.end.icp:
+; CHECK-NEXT:    ret void
+;
+  %a = load void (i64*)*, void (i64*)** %v
+  call void %a(i64* byval(i64) %p), !prof !2
+  ret void
+}
+
+!0 = !{!"function_entry_count", i64 36}
+!1 = !{!"function_entry_count", i64 1}
+!2 = !{!"VP", i32 0, i64 18, i64 12157170054180749580, i64 18}

llvm/test/Transforms/PGOProfile/mismatched-inalloca.ll

+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -passes=pgo-icall-prom -profile-summary-hot-count=10 -S < %s | FileCheck %s
+
+define void @a(i8* %0) !prof !0 {
+; CHECK-LABEL: @a(
+; CHECK-NEXT:    ret void
+;
+  ret void
+}
+
+define void @b(void (i64*)** %v, i64* %p) !prof !1 {
+; CHECK-LABEL: @b(
+; CHECK-NEXT:    [[A:%.*]] = load void (i64*)*, void (i64*)** [[V:%.*]], align 8
+; CHECK-NEXT:    [[TMP1:%.*]] = icmp eq void (i64*)* [[A]], bitcast (void (i8*)* @a to void (i64*)*)
+; CHECK-NEXT:    br i1 [[TMP1]], label [[IF_TRUE_DIRECT_TARG:%.*]], label [[IF_FALSE_ORIG_INDIRECT:%.*]], !prof [[PROF2:![0-9]+]]
+; CHECK:       if.true.direct_targ:
+; CHECK-NEXT:    [[TMP2:%.*]] = bitcast i64* [[P:%.*]] to i8*
+; CHECK-NEXT:    call void @a(i8* [[TMP2]])
+; CHECK-NEXT:    br label [[IF_END_ICP:%.*]]
+; CHECK:       if.false.orig_indirect:
+; CHECK-NEXT:    call void [[A]](i64* inalloca(i64) [[P]])
+; CHECK-NEXT:    br label [[IF_END_ICP]]
+; CHECK:       if.end.icp:
+; CHECK-NEXT:    ret void
+;
+  %a = load void (i64*)*, void (i64*)** %v
+  call void %a(i64* inalloca(i64) %p), !prof !2
+  ret void
+}
+
+!0 = !{!"function_entry_count", i64 36}
+!1 = !{!"function_entry_count", i64 1}
+!2 = !{!"VP", i32 0, i64 18, i64 12157170054180749580, i64 18}