[InterleavedAccessPass]: Ensure that dead nodes get erased only once#122643
Merged
hassnaaHamdi merged 1 commit intollvm:mainfrom Jan 14, 2025
Merged
[InterleavedAccessPass]: Ensure that dead nodes get erased only once#122643hassnaaHamdi merged 1 commit intollvm:mainfrom
hassnaaHamdi merged 1 commit intollvm:mainfrom
Conversation
Member
|
@llvm/pr-subscribers-llvm-transforms Author: Hassnaa Hamdi (hassnaaHamdi) Changes2 Files Affected:
diff --git a/llvm/lib/CodeGen/InterleavedAccessPass.cpp b/llvm/lib/CodeGen/InterleavedAccessPass.cpp
index 8b6e3180986c30..a197e1c7e4b202 100644
--- a/llvm/lib/CodeGen/InterleavedAccessPass.cpp
+++ b/llvm/lib/CodeGen/InterleavedAccessPass.cpp
@@ -545,7 +545,8 @@ bool InterleavedAccessImpl::runOnFunction(Function &F) {
}
for (auto *I : DeadInsts)
- I->eraseFromParent();
+ if (I->getParent())
+ I->eraseFromParent();
return Changed;
}
diff --git a/llvm/test/Transforms/InterleavedAccess/AArch64/sve-interleave4.ll b/llvm/test/Transforms/InterleavedAccess/AArch64/sve-interleave4.ll
index e8d113ae3763d7..085089978d8f51 100644
--- a/llvm/test/Transforms/InterleavedAccess/AArch64/sve-interleave4.ll
+++ b/llvm/test/Transforms/InterleavedAccess/AArch64/sve-interleave4.ll
@@ -55,3 +55,17 @@ define void @mix_interleave4_interleave2(ptr %dst1, ptr %dst2, <vscale x 4 x i32
store <vscale x 8 x i32> %interleaved, ptr %dst2, align 4
ret void
}
+
+; This case tests when the interleave is using same parameter twice,
+; the dead parameter will not get deleted twice.
+define void @duplicate_by_interleave(<vscale x 4 x i32> %A, <vscale x 4 x i32> %B, ptr writeonly %AB_duplicate) {
+; CHECK-LABEL: define void @duplicate_by_interleave
+; CHECK-SAME: (<vscale x 4 x i32> [[A:%.*]], <vscale x 4 x i32> [[B:%.*]], ptr writeonly [[AB_DUPLICATE:%.*]]) #[[ATTR0]] {
+; CHECK-NEXT: call void @llvm.aarch64.sve.st4.nxv4i32(<vscale x 4 x i32> [[A]], <vscale x 4 x i32> [[A]], <vscale x 4 x i32> [[B]], <vscale x 4 x i32> [[B]], <vscale x 4 x i1> splat (i1 true), ptr [[AB_DUPLICATE]])
+; CHECK-NEXT: ret void
+;
+ %interleave = tail call <vscale x 8 x i32> @llvm.vector.interleave2.nxv8i32(<vscale x 4 x i32> %A, <vscale x 4 x i32> %B)
+ %duplicate_by_interleave = tail call <vscale x 16 x i32> @llvm.vector.interleave2.nxv16i32(<vscale x 8 x i32> %interleave, <vscale x 8 x i32> %interleave)
+ store <vscale x 16 x i32> %duplicate_by_interleave, ptr %AB_duplicate, align 4
+ ret void
+}
|
hassnaaHamdi
requested review from
MDevereau
and removed request for
MDevereau
david-arm
reviewed
Jan 13, 2025
|
|
||
| for (auto *I : DeadInsts) | ||
| I->eraseFromParent(); | ||
| if (I->getParent()) |
Contributor
There was a problem hiding this comment.
It looks like you're trying to fix a scenario where the same Instruction is included twice in DeadInsts, right? I think the problem here is that I may have been deleted and so you're dereferencing unreliable contents in memory or we could seg fault. I think you probably need to fix this a different way either by:
- Keeping track of which instructions you've deleted, i.e. by maintaining a map of visited instructions. Or,
- Removing duplicate dead instructions from the list before iterating over them.
Collaborator
|
Could it use SmallSetVector to store the Dead instructions? It should rule out any duplicated whilst keeping them in order, so that they are removed in the correct order. |
Member
Author
I thought about this, but I found that there will be changes in other functions also. |
Use SmallSetVector instead of SmallVector to avoid duplication, so that dead nodes get erased/deleted only once.
hassnaaHamdi
force-pushed
the
interleave_bug
branch
from
3488632 to
191f472
Compare
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/16/builds/11951 Here is the relevant piece of the build log for the reference |
hassnaaHamdi
added a commit
that referenced
this pull request
Jan 17, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Use SmallSetVector instead of SmallVector to avoid duplication,
so that dead nodes get erased/deleted only once.