-
Notifications
You must be signed in to change notification settings - Fork 13.6k
[NewGVN] Check intrinsic ID before accessing operands #141571
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
[NewGVN] Check intrinsic ID before accessing operands #141571
Conversation
This patch is fixing assertions in downstream tests
@llvm/pr-subscribers-backend-amdgpu @llvm/pr-subscribers-llvm-transforms Author: Mariusz Sikora (mariusz-sikora-at-amd) ChangesThis patch is fixing assertions in downstream tests Full diff: https://github.com/llvm/llvm-project/pull/141571.diff 1 Files Affected:
diff --git a/llvm/lib/Transforms/Scalar/NewGVN.cpp b/llvm/lib/Transforms/Scalar/NewGVN.cpp
index 85bd8ef1f09a8..584cb0e89ef66 100644
--- a/llvm/lib/Transforms/Scalar/NewGVN.cpp
+++ b/llvm/lib/Transforms/Scalar/NewGVN.cpp
@@ -1530,11 +1530,12 @@ NewGVN::performSymbolicLoadCoercion(Type *LoadType, Value *LoadPtr,
}
if (auto *II = dyn_cast<IntrinsicInst>(DepInst)) {
- auto *LifetimePtr = II->getOperand(1);
- if (II->getIntrinsicID() == Intrinsic::lifetime_start &&
- (LoadPtr == lookupOperandLeader(LifetimePtr) ||
- AA->isMustAlias(LoadPtr, LifetimePtr)))
- return createConstantExpression(UndefValue::get(LoadType));
+ if (II->getIntrinsicID() == Intrinsic::lifetime_start) {
+ auto *LifetimePtr = II->getOperand(1);
+ if (LoadPtr == lookupOperandLeader(LifetimePtr) ||
+ AA->isMustAlias(LoadPtr, LifetimePtr))
+ return createConstantExpression(UndefValue::get(LoadType));
+ }
}
// All of the below are only true if the loaded pointer is produced
|
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.
Yes, LGTM
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.
Thank you!
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.
This patch is fixing assertions in downstream tests
Would it be possible to add the test?
Yes, I will try to prepare a small test in a moment. |
LGTM |
This patch is fixing assertions in downstream tests
This patch is fixing assertions in downstream tests