Skip to content

Conversation

@perlfu
Copy link
Contributor

@perlfu perlfu commented Feb 15, 2024

Most operations added or removed during CF lowering interact with SCC. As with EXEC the live interval of SCC must be recomputed. Tidy up RegUnit recomputation requests to end of pass if any work was done.

Most operations added or removed during CF lowering interact with
SCC.  As with EXEC the live interval of SCC must be recomputed.
Tidy up RegUnit recomputation requests to end of pass if any work
was done.
@llvmbot
Copy link
Member

llvmbot commented Feb 15, 2024

@llvm/pr-subscribers-backend-amdgpu

Author: Carl Ritson (perlfu)

Changes

Most operations added or removed during CF lowering interact with SCC. As with EXEC the live interval of SCC must be recomputed. Tidy up RegUnit recomputation requests to end of pass if any work was done.


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

1 Files Affected:

  • (modified) llvm/lib/Target/AMDGPU/SILowerControlFlow.cpp (+5-6)
diff --git a/llvm/lib/Target/AMDGPU/SILowerControlFlow.cpp b/llvm/lib/Target/AMDGPU/SILowerControlFlow.cpp
index f178324dbbe246..2e6f6463d61dae 100644
--- a/llvm/lib/Target/AMDGPU/SILowerControlFlow.cpp
+++ b/llvm/lib/Target/AMDGPU/SILowerControlFlow.cpp
@@ -292,7 +292,6 @@ void SILowerControlFlow::emitIf(MachineInstr &MI) {
   LIS->InsertMachineInstrInMaps(*SetExec);
   LIS->InsertMachineInstrInMaps(*NewBr);
 
-  LIS->removeAllRegUnitsForPhysReg(AMDGPU::EXEC);
   MI.eraseFromParent();
 
   // FIXME: Is there a better way of adjusting the liveness? It shouldn't be
@@ -362,9 +361,6 @@ void SILowerControlFlow::emitElse(MachineInstr &MI) {
   RecomputeRegs.insert(SrcReg);
   RecomputeRegs.insert(DstReg);
   LIS->createAndComputeVirtRegInterval(SaveReg);
-
-  // Let this be recomputed.
-  LIS->removeAllRegUnitsForPhysReg(AMDGPU::EXEC);
 }
 
 void SILowerControlFlow::emitIfBreak(MachineInstr &MI) {
@@ -932,8 +928,6 @@ bool SILowerControlFlow::runOnMachineFunction(MachineFunction &MF) {
       case AMDGPU::SI_INIT_EXEC:
       case AMDGPU::SI_INIT_EXEC_FROM_INPUT:
         lowerInitExec(MBB, MI);
-        if (LIS)
-          LIS->removeAllRegUnitsForPhysReg(AMDGPU::EXEC);
         Changed = true;
         break;
 
@@ -951,6 +945,11 @@ bool SILowerControlFlow::runOnMachineFunction(MachineFunction &MF) {
   optimizeEndCf();
 
   if (LIS) {
+    if (Changed) {
+      // These will need to be recomputed for insertions and removals.
+      LIS->removeAllRegUnitsForPhysReg(AMDGPU::EXEC);
+      LIS->removeAllRegUnitsForPhysReg(AMDGPU::SCC);
+    }
     for (Register Reg : RecomputeRegs) {
       LIS->removeInterval(Reg);
       LIS->createAndComputeVirtRegInterval(Reg);

Copy link
Contributor

@jayfoad jayfoad left a comment

Choose a reason for hiding this comment

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

Do you have a test case that fails verification without this?

Copy link
Contributor

@arsenm arsenm left a comment

Choose a reason for hiding this comment

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

Needs test

if (LIS) {
if (Changed) {
// These will need to be recomputed for insertions and removals.
LIS->removeAllRegUnitsForPhysReg(AMDGPU::EXEC);
Copy link
Contributor

Choose a reason for hiding this comment

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

exec is reserved and has no liveness tracking, not sure why it was handled before

@perlfu perlfu closed this Oct 28, 2025
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