Skip to content

Conversation

@topperc
Copy link
Collaborator

@topperc topperc commented May 23, 2025

Similar to #141261.

These aren't easy to test without write MIR tests in areas we don't currently have tests. I'm not sure we even use X0_Pair anywhere today.

I'm going to try to migrate RISCVMakeCompressible to use copyToReg so we can share that code instead of basically duplicating it.

I'm still concerned that target independent code may fold an extract_subreg operation and get an incorrect register if we do start using X0_Pair. We may have to special case dummy_reg_pair_with_x0 in the encoder and printer to be safe.

Similar to llvm#141261.

These aren't easy to test without write MIR tests in areas we
don't currently have tests. I'm not sure we even use X0_Pair anywhere
today.

I'm going to try to migrate RISCVMakeCompressible to use copyToReg
so we can share that code instead of basically duplicating it.

I'm still concerned that target independent code may decompose
an extract_subreg operation and get an incorrect register if we
do start using X0_Pair. We may have to special case dummy_reg_pair_with_x0
in the encoder and printer to be safe.
@topperc topperc requested a review from lenary May 23, 2025 18:26
@llvmbot
Copy link
Member

llvmbot commented May 23, 2025

@llvm/pr-subscribers-backend-risc-v

Author: Craig Topper (topperc)

Changes

Similar to #141261.

These aren't easy to test without write MIR tests in areas we don't currently have tests. I'm not sure we even use X0_Pair anywhere today.

I'm going to try to migrate RISCVMakeCompressible to use copyToReg so we can share that code instead of basically duplicating it.

I'm still concerned that target independent code may fold an extract_subreg operation and get an incorrect register if we do start using X0_Pair. We may have to special case dummy_reg_pair_with_x0 in the encoder and printer to be safe.


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

2 Files Affected:

  • (modified) llvm/lib/Target/RISCV/RISCVExpandPseudoInsts.cpp (+3)
  • (modified) llvm/lib/Target/RISCV/RISCVInstrInfo.cpp (+9-4)
diff --git a/llvm/lib/Target/RISCV/RISCVExpandPseudoInsts.cpp b/llvm/lib/Target/RISCV/RISCVExpandPseudoInsts.cpp
index 4d26f77d4ed2c..d3dce4edb1e75 100644
--- a/llvm/lib/Target/RISCV/RISCVExpandPseudoInsts.cpp
+++ b/llvm/lib/Target/RISCV/RISCVExpandPseudoInsts.cpp
@@ -323,6 +323,8 @@ bool RISCVExpandPseudo::expandRV32ZdinxStore(MachineBasicBlock &MBB,
       TRI->getSubReg(MBBI->getOperand(0).getReg(), RISCV::sub_gpr_even);
   Register Hi =
       TRI->getSubReg(MBBI->getOperand(0).getReg(), RISCV::sub_gpr_odd);
+  if (Hi == RISCV::DUMMY_REG_PAIR_WITH_X0)
+    Hi = RISCV::X0;
 
   auto MIBLo = BuildMI(MBB, MBBI, DL, TII->get(RISCV::SW))
                    .addReg(Lo, getKillRegState(MBBI->getOperand(0).isKill()))
@@ -370,6 +372,7 @@ bool RISCVExpandPseudo::expandRV32ZdinxLoad(MachineBasicBlock &MBB,
       TRI->getSubReg(MBBI->getOperand(0).getReg(), RISCV::sub_gpr_even);
   Register Hi =
       TRI->getSubReg(MBBI->getOperand(0).getReg(), RISCV::sub_gpr_odd);
+  assert(Hi != RISCV::DUMMY_REG_PAIR_WITH_X0 && "Cannot write to X0_Pair");
 
   MachineInstrBuilder MIBLo, MIBHi;
 
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
index 590b45172d43c..6367c8c6d04a0 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
@@ -543,16 +543,21 @@ void RISCVInstrInfo::copyPhysReg(MachineBasicBlock &MBB,
   }
 
   if (RISCV::GPRPairRegClass.contains(DstReg, SrcReg)) {
+    MCRegister EvenReg = TRI->getSubReg(SrcReg, RISCV::sub_gpr_even);
+    MCRegister OddReg = TRI->getSubReg(SrcReg, RISCV::sub_gpr_odd);
+    // We need to correct the odd register of X0_Pair.
+    if (OddReg == RISCV::DUMMY_REG_PAIR_WITH_X0)
+      OddReg == RISCV::X0;
+    assert(DstReg != RISCV::X0_Pair && "Cannot write to X0_Pair");
+
     // Emit an ADDI for both parts of GPRPair.
     BuildMI(MBB, MBBI, DL, get(RISCV::ADDI),
             TRI->getSubReg(DstReg, RISCV::sub_gpr_even))
-        .addReg(TRI->getSubReg(SrcReg, RISCV::sub_gpr_even),
-                getKillRegState(KillSrc))
+        .addReg(EvenReg, getKillRegState(KillSrc))
         .addImm(0);
     BuildMI(MBB, MBBI, DL, get(RISCV::ADDI),
             TRI->getSubReg(DstReg, RISCV::sub_gpr_odd))
-        .addReg(TRI->getSubReg(SrcReg, RISCV::sub_gpr_odd),
-                getKillRegState(KillSrc))
+        .addReg(OddReg, getKillRegState(KillSrc))
         .addImm(0);
     return;
   }

@topperc topperc changed the title [RISCV] Print using dummy_reg_pair_with_x0 in more places. [RISCV] Prevent using dummy_reg_pair_with_x0 in more places. May 23, 2025
@topperc topperc merged commit cd60ee9 into llvm:main May 23, 2025
9 of 11 checks passed
@topperc topperc deleted the pr/more-dummy-x0-copies branch May 23, 2025 20:40
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.

3 participants