Skip to content

[RISCV] Correctly account for the copy cost of GPR pairs in RISCVMakeCompressible. #141251

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

GPR pairs require 2 ADDIs to copy, so we need to be updating more instructions to get a benefit.

@llvmbot
Copy link
Member

llvmbot commented May 23, 2025

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

Author: Craig Topper (topperc)

Changes

GPR pairs require 2 ADDIs to copy, so we need to be updating more instructions to get a benefit.


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

2 Files Affected:

  • (modified) llvm/lib/Target/RISCV/RISCVMakeCompressible.cpp (+5-3)
  • (modified) llvm/test/CodeGen/RISCV/make-compressible-zilsd.mir (+25)
diff --git a/llvm/lib/Target/RISCV/RISCVMakeCompressible.cpp b/llvm/lib/Target/RISCV/RISCVMakeCompressible.cpp
index 1e2bdb10aa810..54e959b59010f 100644
--- a/llvm/lib/Target/RISCV/RISCVMakeCompressible.cpp
+++ b/llvm/lib/Target/RISCV/RISCVMakeCompressible.cpp
@@ -332,9 +332,11 @@ static Register analyzeCompressibleUses(MachineInstr &FirstMI,
   // are required for a code size reduction. If no base adjustment is required,
   // then copying the register costs one new c.mv (or c.li Rd, 0 for "copying"
   // the zero register) and therefore two uses are required for a code size
-  // reduction.
-  if (MIs.size() < 2 || (RegImm.Imm != 0 && MIs.size() < 3))
-    return RISCV::NoRegister;
+  // reduction. For GPR pairs, we need 2 ADDIs to copy so we need three users.
+  unsigned CopyCost = RISCV::GPRPairRegClass.contains(RegImm.Reg) ? 2 : 1;
+  assert((RegImm.Imm == 0 || CopyCost == 1) && "GPRPair should have zero imm");
+  if (MIs.size() <= CopyCost || (RegImm.Imm != 0 && MIs.size() <= 2))
+    return Register();
 
   // Find a compressible register which will be available from the first
   // instruction we care about to the last.
diff --git a/llvm/test/CodeGen/RISCV/make-compressible-zilsd.mir b/llvm/test/CodeGen/RISCV/make-compressible-zilsd.mir
index c5ac599d8d53f..930501dcb378e 100644
--- a/llvm/test/CodeGen/RISCV/make-compressible-zilsd.mir
+++ b/llvm/test/CodeGen/RISCV/make-compressible-zilsd.mir
@@ -62,6 +62,13 @@
     ret void
   }
 
+  define void @store_common_value_double_no_opt2(ptr %a, i32 %b, double %c, double %d, double %e) #0 {
+  entry:
+    store volatile double %e, ptr %a, align 8
+    store volatile double %e, ptr %a, align 8
+    ret void
+  }
+
   define void @store_common_ptr_double_no_opt(double %a, i32 %b, i32 %c, i32 %d, i32 %e, ptr %p) #0 {
   entry:
     store volatile double %a, ptr %p, align 8
@@ -216,6 +223,24 @@ body:             |
     SD_RV32 killed renamable $x16_x17, killed renamable $x10, 0 :: (store (s64) into %ir.a)
     PseudoRET
 
+...
+---
+name:            store_common_value_double_no_opt2
+tracksRegLiveness: true
+body:             |
+  bb.0.entry:
+    liveins: $x10, $x16, $x17
+
+    ; RV32-LABEL: name: store_common_value_double_no_opt2
+    ; RV32: liveins: $x10, $x16, $x17
+    ; RV32-NEXT: {{  $}}
+    ; RV32-NEXT: SD_RV32 renamable $x16_x17, renamable $x10, 0 :: (volatile store (s64) into %ir.a)
+    ; RV32-NEXT: SD_RV32 killed renamable $x16_x17, killed renamable $x10, 0 :: (volatile store (s64) into %ir.a)
+    ; RV32-NEXT: PseudoRET
+    SD_RV32 renamable $x16_x17, renamable $x10, 0 :: (volatile store (s64) into %ir.a)
+    SD_RV32 killed renamable $x16_x17, killed renamable $x10, 0 :: (volatile store (s64) into %ir.a)
+    PseudoRET
+
 ...
 ---
 name:            store_common_ptr_double_no_opt

@topperc topperc merged commit 75d62ee into llvm:main May 23, 2025
11 of 13 checks passed
@topperc topperc deleted the pr/zilsd-compressible-cost branch May 23, 2025 18:37
sivan-shani pushed a commit to sivan-shani/llvm-project that referenced this pull request Jun 3, 2025
…Compressible. (llvm#141251)

GPR pairs require 2 ADDIs to copy, so we need to be updating more
instructions to get a benefit.
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