[Inliner] Don't propagate access attr to byval params

[goldsteinn]

• [Inliner] Add tests for bad propagationg of access attr for byval param; NFC
• [Inliner] Don't propagate access attr to byval params

We previously only handled the case where the byval attr was in the
callbase's param attr list. This PR also handles the case if the
ByVal was a param attr on the function's param attr list.

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

Author: None (goldsteinn)

Changes

• [Inliner] Add tests for bad propagationg of access attr for byval param; NFC
• [Inliner] Don't propagate access attr to byval params

---

Full diff: https://github.com/llvm/llvm-project/pull/112256.diff

2 Files Affected:

• (modified) llvm/lib/Transforms/Utils/InlineFunction.cpp (+11-6)
• (modified) llvm/test/Transforms/Inline/access-attributes-prop.ll (+20)

diff --git a/llvm/lib/Transforms/Utils/InlineFunction.cpp b/llvm/lib/Transforms/Utils/InlineFunction.cpp
index 110fd6de5c6968..c8a2fd95e3a970 100644
--- a/llvm/lib/Transforms/Utils/InlineFunction.cpp
+++ b/llvm/lib/Transforms/Utils/InlineFunction.cpp
@@ -1398,7 +1398,7 @@ static void AddParamAndFnBasicAttributes(const CallBase &CB,
         if (!Arg)
           continue;
 
-        if (AL.hasParamAttr(I, Attribute::ByVal))
+        if (NewInnerCB->getParamAttr(I, Attribute::ByVal).isValid())
           // It's unsound to propagate memory attributes to byval arguments.
           // Even if CalledFunction doesn't e.g. write to the argument,
           // the call to NewInnerCB may write to its by-value copy.
@@ -1407,24 +1407,29 @@ static void AddParamAndFnBasicAttributes(const CallBase &CB,
         unsigned ArgNo = Arg->getArgNo();
         // If so, propagate its access attributes.
         AL = AL.addParamAttributes(Context, I, ValidParamAttrs[ArgNo]);
+        auto NewCBHasParamAttr = [&](Attribute::AttrKind Kind) {
+          return AL.hasParamAttr(I, Kind) ||
+                 NewInnerCB->getParamAttr(I, Kind).isValid();
+        };
+
         // We can have conflicting attributes from the inner callsite and
         // to-be-inlined callsite. In that case, choose the most
         // restrictive.
 
         // readonly + writeonly means we can never deref so make readnone.
-        if (AL.hasParamAttr(I, Attribute::ReadOnly) &&
-            AL.hasParamAttr(I, Attribute::WriteOnly))
+        if (NewCBHasParamAttr(Attribute::ReadOnly) &&
+            NewCBHasParamAttr(Attribute::WriteOnly))
           AL = AL.addParamAttribute(Context, I, Attribute::ReadNone);
 
         // If have readnone, need to clear readonly/writeonly
-        if (AL.hasParamAttr(I, Attribute::ReadNone)) {
+        if (NewCBHasParamAttr(Attribute::ReadNone)) {
           AL = AL.removeParamAttribute(Context, I, Attribute::ReadOnly);
           AL = AL.removeParamAttribute(Context, I, Attribute::WriteOnly);
         }
 
         // Writable cannot exist in conjunction w/ readonly/readnone
-        if (AL.hasParamAttr(I, Attribute::ReadOnly) ||
-            AL.hasParamAttr(I, Attribute::ReadNone))
+        if (NewCBHasParamAttr(Attribute::ReadOnly) ||
+            NewCBHasParamAttr(Attribute::ReadNone))
           AL = AL.removeParamAttribute(Context, I, Attribute::Writable);
       }
       NewInnerCB->setAttributes(AL);
diff --git a/llvm/test/Transforms/Inline/access-attributes-prop.ll b/llvm/test/Transforms/Inline/access-attributes-prop.ll
index 2c55f5f3b1f6ca..5051c92345ec75 100644
--- a/llvm/test/Transforms/Inline/access-attributes-prop.ll
+++ b/llvm/test/Transforms/Inline/access-attributes-prop.ll
@@ -580,3 +580,23 @@ define ptr @callee_bad_param_prop(ptr readonly %x) {
   %r = tail call ptr @llvm.ptrmask(ptr %x, i64 -1)
   ret ptr %r
 }
+
+define dso_local void @foo_byval_readonly2(ptr readonly %p) {
+; CHECK-LABEL: define {{[^@]+}}@foo_byval_readonly2
+; CHECK-SAME: (ptr readonly [[P:%.*]]) {
+; CHECK-NEXT:    call void @bar4(ptr [[P]])
+; CHECK-NEXT:    ret void
+;
+  call void @bar4(ptr %p)
+  ret void
+}
+
+define void @prop_byval_readonly2(ptr %p) {
+; CHECK-LABEL: define {{[^@]+}}@prop_byval_readonly2
+; CHECK-SAME: (ptr [[P:%.*]]) {
+; CHECK-NEXT:    call void @bar4(ptr [[P]])
+; CHECK-NEXT:    ret void
+;
+  call void @foo_byval_readonly2(ptr %p)
+  ret void
+}

[nikic]

LGTM

[goldsteinn]

/cherry-pick 3c777f0

[llvmbot]

/pull-request #112365