Skip to content

Conversation

@lucas-rami
Copy link
Contributor

Any non-zero SubIdx passed to the method is composed with the rematerialized instruction's first operand's subregister to determine the new register's subregister. In our case we want the new register to have the same subregister as the old one, so we should pass 0.

@llvmbot
Copy link
Member

llvmbot commented Jul 23, 2025

@llvm/pr-subscribers-backend-amdgpu

Author: Lucas Ramirez (lucas-rami)

Changes

Any non-zero SubIdx passed to the method is composed with the rematerialized instruction's first operand's subregister to determine the new register's subregister. In our case we want the new register to have the same subregister as the old one, so we should pass 0.


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

1 Files Affected:

  • (modified) llvm/lib/Target/AMDGPU/GCNSchedStrategy.cpp (+2-6)
diff --git a/llvm/lib/Target/AMDGPU/GCNSchedStrategy.cpp b/llvm/lib/Target/AMDGPU/GCNSchedStrategy.cpp
index fce8f36d45969..20e489aa39023 100644
--- a/llvm/lib/Target/AMDGPU/GCNSchedStrategy.cpp
+++ b/llvm/lib/Target/AMDGPU/GCNSchedStrategy.cpp
@@ -1910,14 +1910,12 @@ void PreRARematStage::rematerialize() {
   for (auto &[DefMI, Remat] : Rematerializations) {
     MachineBasicBlock::iterator InsertPos(Remat.UseMI);
     Register Reg = DefMI->getOperand(0).getReg();
-    unsigned SubReg = DefMI->getOperand(0).getSubReg();
     unsigned DefRegion = MIRegion.at(DefMI);
 
     // Rematerialize DefMI to its use block.
-    TII->reMaterialize(*InsertPos->getParent(), InsertPos, Reg, SubReg, *DefMI,
+    TII->reMaterialize(*InsertPos->getParent(), InsertPos, Reg, 0, *DefMI,
                        *DAG.TRI);
     Remat.RematMI = &*std::prev(InsertPos);
-    Remat.RematMI->getOperand(0).setSubReg(SubReg);
     DAG.LIS->InsertMachineInstrInMaps(*Remat.RematMI);
 
     // Update region boundaries in regions we sinked from (remove defining MI)
@@ -2063,14 +2061,12 @@ void PreRARematStage::finalizeGCNSchedStage() {
     MachineBasicBlock::iterator InsertPos(DAG.Regions[DefRegion].second);
     MachineBasicBlock *MBB = RegionBB[DefRegion];
     Register Reg = RematMI.getOperand(0).getReg();
-    unsigned SubReg = RematMI.getOperand(0).getSubReg();
 
     // Re-rematerialize MI at the end of its original region. Note that it may
     // not be rematerialized exactly in the same position as originally within
     // the region, but it should not matter much.
-    TII->reMaterialize(*MBB, InsertPos, Reg, SubReg, RematMI, *DAG.TRI);
+    TII->reMaterialize(*MBB, InsertPos, Reg, 0, RematMI, *DAG.TRI);
     MachineInstr *NewMI = &*std::prev(InsertPos);
-    NewMI->getOperand(0).setSubReg(SubReg);
     DAG.LIS->InsertMachineInstrInMaps(*NewMI);
 
     auto UseRegion = MIRegion.find(Remat.UseMI);

@lucas-rami lucas-rami requested review from arsenm and jrbyrnes July 23, 2025 16:52
@lucas-rami lucas-rami merged commit e38f98f into llvm:main Jul 25, 2025
11 checks passed

// Rematerialize DefMI to its use block.
TII->reMaterialize(*InsertPos->getParent(), InsertPos, Reg, SubReg, *DefMI,
TII->reMaterialize(*InsertPos->getParent(), InsertPos, Reg, 0, *DefMI,
Copy link
Contributor

Choose a reason for hiding this comment

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

Should use AMDGPU::NoSubregister

mahesh-attarde pushed a commit to mahesh-attarde/llvm-project that referenced this pull request Jul 28, 2025
Any non-zero `SubIdx` passed to the method is composed with the
rematerialized instruction's first operand's subregister to determine
the new register's subregister. In our case we want the new register to
have the same subregister as the old one, so we should pass 0.
jrbyrnes pushed a commit to jrbyrnes/llvm-project that referenced this pull request Aug 13, 2025
Any non-zero `SubIdx` passed to the method is composed with the
rematerialized instruction's first operand's subregister to determine
the new register's subregister. In our case we want the new register to
have the same subregister as the old one, so we should pass 0.
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