-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
[InterleavedAccessPass] Avoid optimizing load instructions if it has dead binop users #71339
Conversation
@llvm/pr-subscribers-llvm-transforms Author: None (Skwoogey) ChangesIf a load instruction qualifies to be optimized by InterleavedAccess Pass, but also has a dead binop instruction this will lead to a crash. Binop instruction will not be deleted, because normally it would be deleted through its' users, but it has none. Later on deleting a load instruction will fail because it still has uses. Full diff: https://github.com/llvm/llvm-project/pull/71339.diff 2 Files Affected:
diff --git a/llvm/lib/CodeGen/InterleavedAccessPass.cpp b/llvm/lib/CodeGen/InterleavedAccessPass.cpp
index 6b3848531569c0b..672caea3086cd63 100644
--- a/llvm/lib/CodeGen/InterleavedAccessPass.cpp
+++ b/llvm/lib/CodeGen/InterleavedAccessPass.cpp
@@ -251,7 +251,7 @@ bool InterleavedAccess::lowerInterleavedLoad(
continue;
}
if (auto *BI = dyn_cast<BinaryOperator>(User)) {
- if (all_of(BI->users(), [](auto *U) {
+ if (!BI->users().empty() && all_of(BI->users(), [](auto *U) {
auto *SVI = dyn_cast<ShuffleVectorInst>(U);
return SVI && isa<UndefValue>(SVI->getOperand(1));
})) {
diff --git a/llvm/test/Transforms/InterleavedAccess/AArch64/binopshuffles.ll b/llvm/test/Transforms/InterleavedAccess/AArch64/binopshuffles.ll
index 67910305f56d66c..2e8a7cf42ac506b 100644
--- a/llvm/test/Transforms/InterleavedAccess/AArch64/binopshuffles.ll
+++ b/llvm/test/Transforms/InterleavedAccess/AArch64/binopshuffles.ll
@@ -220,3 +220,24 @@ entry:
store <8 x i8> %shuffled, ptr %p2
ret void
}
+
+define void @skip_optimizing_dead_binop(ptr %p0, ptr %p1) {
+; CHECK-LABEL: @skip_optimizing_dead_binop(
+; CHECK-NEXT: entry:
+; CHECK-NEXT: [[V0:%.*]] = load <8 x double>, ptr [[P0:%.*]]
+; CHECK-NEXT: [[SHUFFLED_1:%.*]] = shufflevector <8 x double> [[V0]], <8 x double> undef, <2 x i32> <i32 0, i32 4>
+; CHECK-NEXT: [[SHUFFLED_2:%.*]] = shufflevector <8 x double> [[V0]], <8 x double> undef, <2 x i32> <i32 1, i32 5>
+; CHECK-NEXT: [[SHUFFLED_3:%.*]] = shufflevector <8 x double> [[V0]], <8 x double> undef, <2 x i32> <i32 2, i32 6>
+; CHECK-NEXT: [[SHUFFLED_4:%.*]] = shufflevector <8 x double> [[V0]], <8 x double> undef, <2 x i32> <i32 3, i32 7>
+; CHECK-NEXT: [[DEAD_BINOP:%.*]] = fadd <8 x double> [[V0]], [[V0]]
+; CHECK-NEXT: ret void
+;
+entry:
+ %v0 = load <8 x double>, ptr %p0
+ %shuffled_1 = shufflevector <8 x double> %v0, <8 x double> undef, <2 x i32> <i32 0, i32 4>
+ %shuffled_2 = shufflevector <8 x double> %v0, <8 x double> undef, <2 x i32> <i32 1, i32 5>
+ %shuffled_3 = shufflevector <8 x double> %v0, <8 x double> undef, <2 x i32> <i32 2, i32 6>
+ %shuffled_4 = shufflevector <8 x double> %v0, <8 x double> undef, <2 x i32> <i32 3, i32 7>
+ %dead_binop = fadd <8 x double> %v0, %v0
+ ret void
+}
|
5ecb692
to
e0570bc
Compare
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 looks good to me, if these userless instructions are left around. Thanks for the fix.
…dead binop users If a load instruction qualifies to be optimized by InterleavedAccess Pass, but also has a dead binop instruction, this will lead to a crash. Binop instruction will not be deleted, because normally it would be deleted through its' users, but it has none. Later on deleting a load instruction will fail because it still has uses.
e0570bc
to
45ff0a8
Compare
Encountered that issue with a proprietary compiler and decided to try and upstream it. Thanks for the review. |
Thanks |
If a load instruction qualifies to be optimized by InterleavedAccess Pass, but also has a dead binop instruction, this will lead to a crash.
Binop instruction will not be deleted, because normally it would be deleted through its' users, but it has none. Later on deleting a load instruction will fail because it still has uses.