-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[Inliner] Prevent adding pointer attributes to non-pointer arguments #115569
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@llvm/pr-subscribers-llvm-transforms Author: Harald van Dijk (hvdijk) ChangesFixes a crash seen after #114311 Full diff: https://github.com/llvm/llvm-project/pull/115569.diff 2 Files Affected:
diff --git a/llvm/lib/Transforms/Utils/InlineFunction.cpp b/llvm/lib/Transforms/Utils/InlineFunction.cpp
index a27cb4dd219c30..cbad66ed9dfe5e 100644
--- a/llvm/lib/Transforms/Utils/InlineFunction.cpp
+++ b/llvm/lib/Transforms/Utils/InlineFunction.cpp
@@ -1477,6 +1477,15 @@ static void AddParamAndFnBasicAttributes(const CallBase &CB,
// If so, propagate its access attributes.
AL = AL.addParamAttributes(Context, I, ValidObjParamAttrs[ArgNo]);
+
+ // If the argument is not a pointer type, remove attributes which only
+ // apply to pointer types.
+ if (!NewInnerCB->getArgOperand(I)->getType()->isPointerTy()) {
+ AL = AL.removeParamAttribute(Context, I, Attribute::ReadNone);
+ AL = AL.removeParamAttribute(Context, I, Attribute::ReadOnly);
+ continue;
+ }
+
// We can have conflicting attributes from the inner callsite and
// to-be-inlined callsite. In that case, choose the most
// restrictive.
diff --git a/llvm/test/Transforms/Inline/arg-attr-propagation.ll b/llvm/test/Transforms/Inline/arg-attr-propagation.ll
index 7b096539e7e1b1..fad6c7ced2edd2 100644
--- a/llvm/test/Transforms/Inline/arg-attr-propagation.ll
+++ b/llvm/test/Transforms/Inline/arg-attr-propagation.ll
@@ -76,3 +76,29 @@ define i32 @caller3(ptr dereferenceable(33) %t1) {
ret i32 %t2
}
+; Make sure that we don't propagate a pointer-only attribute to a vector of pointers.
+
+declare void @helper4(<4 x ptr> %ptr)
+
+define void @callee4(ptr readonly %ptr, <4 x i64> %idx) {
+; CHECK-LABEL: define {{[^@]+}}@callee4
+; CHECK-SAME: (ptr readonly [[PTR:%.*]], <4 x i64> [[IDX:%.*]]) {
+; CHECK-NEXT: [[PTRS:%.*]] = getelementptr inbounds i8, ptr [[PTR]], <4 x i64> [[IDX]]
+; CHECK-NEXT: call void @helper4(<4 x ptr> [[PTRS]])
+; CHECK-NEXT: ret void
+;
+ %ptrs = getelementptr inbounds i8, ptr %ptr, <4 x i64> %idx
+ call void @helper4(<4 x ptr> %ptrs)
+ ret void
+}
+
+define void @caller4(ptr readonly %ptr, <4 x i64> %idx) {
+; CHECK-LABEL: define {{[^@]+}}@caller4
+; CHECK-SAME: (ptr readonly [[PTR:%.*]], <4 x i64> [[IDX:%.*]]) {
+; CHECK-NEXT: [[PTRS_I:%.*]] = getelementptr inbounds i8, ptr [[PTR]], <4 x i64> [[IDX]]
+; CHECK-NEXT: call void @helper4(<4 x ptr> [[PTRS_I]])
+; CHECK-NEXT: ret void
+;
+ call void @callee4(ptr readonly %ptr, <4 x i64> %idx)
+ ret void
+}
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should entirely skip the getUnderlyingObject logic for non-pointer arguments instead.
Fixes a crash seen after llvm#114311
48f6814
to
199420a
Compare
Good thinking, that passes testing and is simpler. Done. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/88/builds/4437 Here is the relevant piece of the build log for the reference
|
…lvm#115569) Fixes a crash seen after llvm#114311
Fixes a crash seen after #114311