[PATCH] D107998: [CallPromotion] Check for inalloca/byval mismatch

[Arthur Eubanks via Phabricator via llvm-commits]

aeubanks created this revision.
aeubanks added reviewers: rnk, wmi, efriedma.
Herald added subscribers: wenlei, hiraditya.
aeubanks requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Previously we would allow promotion even if the byval/inalloca
attributes on the call and the callee didn't match.

It's ok if the byval/inalloca types aren't the same. For example, LTO
importing may rename types.

Fixes PR51397.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D107998

Files:
  llvm/lib/Transforms/Utils/CallPromotionUtils.cpp
  llvm/test/Transforms/PGOProfile/mismatched-byval.ll
  llvm/test/Transforms/PGOProfile/mismatched-inalloca.ll


Index: llvm/test/Transforms/PGOProfile/mismatched-inalloca.ll
===================================================================
--- /dev/null
+++ llvm/test/Transforms/PGOProfile/mismatched-inalloca.ll
@@ -0,0 +1,21 @@
+; RUN: opt -passes=pgo-icall-prom -profile-summary-hot-count=10 -S < %s -pass-remarks-output=- | FileCheck %s
+
+; CHECK: inalloca mismatch
+
+define void @a(i8* %0) !prof !0 {
+  ret void
+}
+
+define void @b(void (i64*)** %v, i64* %p) !prof !1 {
+; CHECK-LABEL: @b
+; CHECK-NEXT: load
+; CHECK-NEXT: call void {{.*}}(i64* inalloca(i64)
+; 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}
Index: llvm/test/Transforms/PGOProfile/mismatched-byval.ll
===================================================================
--- /dev/null
+++ llvm/test/Transforms/PGOProfile/mismatched-byval.ll
@@ -0,0 +1,22 @@
+; RUN: opt -passes=pgo-icall-prom -profile-summary-hot-count=10 -S < %s -pass-remarks-output=- | FileCheck %s
+
+; CHECK: byval mismatch
+
+define void @a(i8* %0) !prof !0 {
+  ret void
+}
+
+define void @b(void (i64*)** %v, i64* %p) !prof !1 {
+; CHECK-LABEL: @b
+; CHECK-NEXT: load
+; CHECK-NEXT: call void {{.*}}(i64* byval(i64)
+; 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}
Index: llvm/lib/Transforms/Utils/CallPromotionUtils.cpp
===================================================================
--- llvm/lib/Transforms/Utils/CallPromotionUtils.cpp
+++ llvm/lib/Transforms/Utils/CallPromotionUtils.cpp
@@ -424,6 +424,20 @@
         *FailureReason = "Argument type mismatch";
       return false;
     }
+    // Make sure that the callee and call agree on byval/inalloca. The types do
+    // not have to match.
+    if (bool(Callee->getParamByValType(I)) !=
+        bool(CB.getAttributes().getParamByValType(I))) {
+      if (FailureReason)
+        *FailureReason = "byval mismatch";
+      return false;
+    }
+    if (bool(Callee->getParamInAllocaType(I)) !=
+        bool(CB.getAttributes().getParamInAllocaType(I))) {
+      if (FailureReason)
+        *FailureReason = "inalloca mismatch";
+      return false;
+    }
   }
   for (; I < NumArgs; I++) {
     // Vararg functions can have more arguments than parameters.
@@ -488,10 +502,11 @@
       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.
+      // We may have a different byval/inalloca type.
       if (ArgAttrs.getByValType())
         ArgAttrs.addByValAttr(Callee->getParamByValType(ArgNo));
+      if (ArgAttrs.getInAllocaType())
+        ArgAttrs.addInAllocaAttr(Callee->getParamInAllocaType(ArgNo));
 
       NewArgAttrs.push_back(AttributeSet::get(Ctx, ArgAttrs));
       AttributeChanged = true;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D107998.366106.patch
Type: text/x-patch
Size: 3280 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210812/baabc063/attachment.bin>