-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[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
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
@llvm/pr-subscribers-backend-risc-v Author: Craig Topper (topperc) Changesdummy_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:
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
|
lenary
approved these changes
May 23, 2025
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
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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.