From 73e4a18ca240338d32740523e908af50c72b59a0 Mon Sep 17 00:00:00 2001 From: Noah Goldstein Date: Mon, 14 Oct 2024 10:51:31 -0500 Subject: [PATCH 1/3] [Inliner] Add tests for bad propagationg of access attr for `byval` param; NFC --- .../Inline/access-attributes-prop.ll | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/llvm/test/Transforms/Inline/access-attributes-prop.ll b/llvm/test/Transforms/Inline/access-attributes-prop.ll index 2c55f5f3b1f6c..1374b4bd3a9e4 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 readonly [[P]]) +; CHECK-NEXT: ret void +; + call void @foo_byval_readonly2(ptr %p) + ret void +} From 74f7931fb6c4c81598afce74ef48291e561b6268 Mon Sep 17 00:00:00 2001 From: Noah Goldstein Date: Mon, 14 Oct 2024 10:51:35 -0500 Subject: [PATCH 2/3] [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. --- llvm/lib/Transforms/Utils/InlineFunction.cpp | 2 +- llvm/test/Transforms/Inline/access-attributes-prop.ll | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/llvm/lib/Transforms/Utils/InlineFunction.cpp b/llvm/lib/Transforms/Utils/InlineFunction.cpp index 110fd6de5c696..c6cefc7ab5eb0 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. diff --git a/llvm/test/Transforms/Inline/access-attributes-prop.ll b/llvm/test/Transforms/Inline/access-attributes-prop.ll index 1374b4bd3a9e4..5051c92345ec7 100644 --- a/llvm/test/Transforms/Inline/access-attributes-prop.ll +++ b/llvm/test/Transforms/Inline/access-attributes-prop.ll @@ -594,7 +594,7 @@ define dso_local void @foo_byval_readonly2(ptr readonly %p) { define void @prop_byval_readonly2(ptr %p) { ; CHECK-LABEL: define {{[^@]+}}@prop_byval_readonly2 ; CHECK-SAME: (ptr [[P:%.*]]) { -; CHECK-NEXT: call void @bar4(ptr readonly [[P]]) +; CHECK-NEXT: call void @bar4(ptr [[P]]) ; CHECK-NEXT: ret void ; call void @foo_byval_readonly2(ptr %p) From 462853dda7f48d2312c7d6d6535105104315c812 Mon Sep 17 00:00:00 2001 From: Noah Goldstein Date: Mon, 14 Oct 2024 16:48:51 -0500 Subject: [PATCH 3/3] Use paramHasAttr --- llvm/lib/Transforms/Utils/InlineFunction.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/llvm/lib/Transforms/Utils/InlineFunction.cpp b/llvm/lib/Transforms/Utils/InlineFunction.cpp index c6cefc7ab5eb0..55ad2b6d62000 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 (NewInnerCB->getParamAttr(I, Attribute::ByVal).isValid()) + if (NewInnerCB->paramHasAttr(I, Attribute::ByVal)) // 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.