Skip to content

[RISCV] Prevent copying dummy_reg_pair_with_x0 in RISCVMakeCompressible. #141261

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

Merged
merged 2 commits into from
May 23, 2025

Conversation

topperc
Copy link
Collaborator

@topperc topperc commented May 23, 2025

dummy_reg_pair_with_x0 is the odd subregister of X0_Pair, but it isn't a real register. We need to copy X0 instead since X0_Pair reads as 0.

@topperc topperc requested a review from lenary May 23, 2025 17:19
@llvmbot
Copy link
Member

llvmbot commented May 23, 2025

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

Author: Craig Topper (topperc)

Changes

dummy_reg_pair_with_x0 is the odd subregister of X0_Pair, but it isn't a real register. We need to copy X0 instead since X0_Pair reads as 0.


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

2 Files Affected:

  • (modified) llvm/lib/Target/RISCV/RISCVMakeCompressible.cpp (+10-2)
  • (modified) llvm/test/CodeGen/RISCV/make-compressible-zilsd.mir (+30)
diff --git a/llvm/lib/Target/RISCV/RISCVMakeCompressible.cpp b/llvm/lib/Target/RISCV/RISCVMakeCompressible.cpp
index 1e2bdb10aa810..7ed2e67635517 100644
--- a/llvm/lib/Target/RISCV/RISCVMakeCompressible.cpp
+++ b/llvm/lib/Target/RISCV/RISCVMakeCompressible.cpp
@@ -452,13 +452,21 @@ bool RISCVMakeCompressibleOpt::runOnMachineFunction(MachineFunction &Fn) {
             .addReg(RegImm.Reg);
       } else if (RISCV::GPRPairRegClass.contains(RegImm.Reg)) {
         assert(RegImm.Imm == 0);
+        MCRegister EvenReg = TRI.getSubReg(RegImm.Reg, RISCV::sub_gpr_even);
+        MCRegister OddReg;
+        // We need to special case odd reg for X0_PAIR.
+        if (RegImm.Reg == RISCV::X0_Pair)
+          OddReg = RISCV::X0;
+        else
+          OddReg = TRI.getSubReg(RegImm.Reg, RISCV::sub_gpr_odd);
+        assert(NewReg != RISCV::X0_Pair && "Cannot write to X0_Pair");
         BuildMI(MBB, MI, MI.getDebugLoc(), TII.get(RISCV::ADDI),
                 TRI.getSubReg(NewReg, RISCV::sub_gpr_even))
-            .addReg(TRI.getSubReg(RegImm.Reg, RISCV::sub_gpr_even))
+            .addReg(EvenReg)
             .addImm(0);
         BuildMI(MBB, MI, MI.getDebugLoc(), TII.get(RISCV::ADDI),
                 TRI.getSubReg(NewReg, RISCV::sub_gpr_odd))
-            .addReg(TRI.getSubReg(RegImm.Reg, RISCV::sub_gpr_odd))
+            .addReg(OddReg)
             .addImm(0);
       } else {
         assert((RISCV::FPR32RegClass.contains(RegImm.Reg) ||
diff --git a/llvm/test/CodeGen/RISCV/make-compressible-zilsd.mir b/llvm/test/CodeGen/RISCV/make-compressible-zilsd.mir
index c5ac599d8d53f..48f489458f93b 100644
--- a/llvm/test/CodeGen/RISCV/make-compressible-zilsd.mir
+++ b/llvm/test/CodeGen/RISCV/make-compressible-zilsd.mir
@@ -10,6 +10,14 @@
     ret void
   }
 
+  define void @store_common_value_double_zero(ptr %a, ptr %b, ptr %c) #0 {
+  entry:
+    store double 0.0, ptr %a, align 8
+    store double 0.0, ptr %b, align 8
+    store double 0.0, ptr %c, align 8
+    ret void
+  }
+
   define void @store_common_ptr_double(double %a, double %b, double %d, ptr %p) #0 {
   entry:
     store volatile double %a, ptr %p, align 8
@@ -117,6 +125,28 @@ body:             |
     SD_RV32 killed renamable $x16_x17, killed renamable $x12, 0 :: (store (s64) into %ir.c)
     PseudoRET
 
+...
+---
+name:            store_common_value_double_zero
+tracksRegLiveness: true
+body:             |
+  bb.0.entry:
+    liveins: $x10, $x11, $x12
+
+    ; RV32-LABEL: name: store_common_value_double_zero
+    ; RV32: liveins: $x10, $x11, $x12
+    ; RV32-NEXT: {{  $}}
+    ; RV32-NEXT: $x14 = ADDI $x0, 0
+    ; RV32-NEXT: $x15 = ADDI $x0, 0
+    ; RV32-NEXT: SD_RV32 $x14_x15, killed renamable $x10, 0 :: (store (s64) into %ir.a)
+    ; RV32-NEXT: SD_RV32 $x14_x15, killed renamable $x11, 0 :: (store (s64) into %ir.b)
+    ; RV32-NEXT: SD_RV32 $x14_x15, killed renamable $x12, 0 :: (store (s64) into %ir.c)
+    ; RV32-NEXT: PseudoRET
+    SD_RV32 $x0_pair, killed renamable $x10, 0 :: (store (s64) into %ir.a)
+    SD_RV32 $x0_pair, killed renamable $x11, 0 :: (store (s64) into %ir.b)
+    SD_RV32 $x0_pair, killed renamable $x12, 0 :: (store (s64) into %ir.c)
+    PseudoRET
+
 ...
 ---
 name:            store_common_ptr_double

topperc added a commit to topperc/llvm-project that referenced this pull request May 23, 2025
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 merged commit a2d717d into llvm:main May 23, 2025
13 checks passed
@topperc topperc deleted the pr/reg-pair-x0-compressible branch May 23, 2025 18:35
topperc added a commit that referenced this pull request 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 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.
sivan-shani pushed a commit to sivan-shani/llvm-project that referenced this pull request Jun 3, 2025
…le. (llvm#141261)

dummy_reg_pair_with_x0 is the odd subregister of X0_Pair, but it isn't a
real register. We need to copy X0 instead since X0_Pair reads as 0.
sivan-shani pushed a commit to sivan-shani/llvm-project that referenced this pull request Jun 3, 2025
…1273)

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 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.
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