Skip to content

[NFC][AMDGPU] Remove an obsolete debug assertion trigger #127357

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

Closed

Conversation

shiltian
Copy link
Contributor

The removed code block was previously used to trigger an assertion, which has
since been fixed. However, its presence could cause inconsistent check lines
between debug (or assertion-enabled) builds and release builds. The fact that no
test updates were needed further confirms that this issue no longer exists.

Fixes SWDEV-515712.

The removed code block was previously used to trigger an assertion, which has
since been fixed. However, its presence could cause inconsistent check lines
between debug (or assertion-enabled) builds and release builds. The fact that no
test updates were needed further confirms that this issue no longer exists.

Fixes SWDEV-515712.
Copy link
Contributor Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@shiltian shiltian requested review from arsenm and cdevadas February 16, 2025 00:37
@llvmbot
Copy link
Member

llvmbot commented Feb 16, 2025

@llvm/pr-subscribers-backend-amdgpu

Author: Shilei Tian (shiltian)

Changes

The removed code block was previously used to trigger an assertion, which has
since been fixed. However, its presence could cause inconsistent check lines
between debug (or assertion-enabled) builds and release builds. The fact that no
test updates were needed further confirms that this issue no longer exists.

Fixes SWDEV-515712.


Full diff: https://github.com/llvm/llvm-project/pull/127357.diff

1 Files Affected:

  • (modified) llvm/lib/Target/AMDGPU/SILowerI1Copies.cpp (-4)
diff --git a/llvm/lib/Target/AMDGPU/SILowerI1Copies.cpp b/llvm/lib/Target/AMDGPU/SILowerI1Copies.cpp
index 5805ec658fd61..e177baa1382c7 100644
--- a/llvm/lib/Target/AMDGPU/SILowerI1Copies.cpp
+++ b/llvm/lib/Target/AMDGPU/SILowerI1Copies.cpp
@@ -512,10 +512,6 @@ bool PhiLoweringHelper::lowerPhis() {
              DT->getNode(RHS.Block)->getDFSNumIn();
     });
 
-#ifndef NDEBUG
-    PhiRegisters.insert(DstReg);
-#endif
-
     // Phis in a loop that are observed outside the loop receive a simple but
     // conservatively correct treatment.
     std::vector<MachineBasicBlock *> DomBlocks = {&MBB};

@@ -512,10 +512,6 @@ bool PhiLoweringHelper::lowerPhis() {
DT->getNode(RHS.Block)->getDFSNumIn();
});

#ifndef NDEBUG
PhiRegisters.insert(DstReg);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is still a count call, and the member exists. I'm not sure I follow what removing this fixes, but this should remove all the parts

@shiltian shiltian closed this Feb 16, 2025
@shiltian shiltian deleted the users/shiltian/remove-obsolete-assertion-trigger branch February 16, 2025 18:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants