Skip to content

[RISCV] Add a command line option to disable cost per use for compressed registers. #83320

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 1 commit into from
Feb 28, 2024

Conversation

topperc
Copy link
Collaborator

@topperc topperc commented Feb 28, 2024

I've seen cases where the cost per use increase the number of spills. Disabling improves the codegen for #79918.

I propose adding this option to allow easier experimentation.

…sed registers.

I've seen cases where the cost per use increase the number of spills.
Disabling improves the codegen for llvm#79918.

I propose adding this option to allow easier experimentation.
@llvmbot
Copy link
Member

llvmbot commented Feb 28, 2024

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

Author: Craig Topper (topperc)

Changes

I've seen cases where the cost per use increase the number of spills. Disabling improves the codegen for #79918.

I propose adding this option to allow easier experimentation.


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

1 Files Affected:

  • (modified) llvm/lib/Target/RISCV/RISCVRegisterInfo.cpp (+6-1)
diff --git a/llvm/lib/Target/RISCV/RISCVRegisterInfo.cpp b/llvm/lib/Target/RISCV/RISCVRegisterInfo.cpp
index 9d1f01dffaaf47..a68674b221d38e 100644
--- a/llvm/lib/Target/RISCV/RISCVRegisterInfo.cpp
+++ b/llvm/lib/Target/RISCV/RISCVRegisterInfo.cpp
@@ -30,6 +30,8 @@
 
 using namespace llvm;
 
+static cl::opt<bool> DisableCostPerUse("riscv-disable-cost-per-use",
+                                       cl::init(false), cl::Hidden);
 static cl::opt<bool>
     DisableRegAllocHints("riscv-disable-regalloc-hints", cl::Hidden,
                          cl::init(false),
@@ -712,7 +714,10 @@ void RISCVRegisterInfo::getOffsetOpcodes(const StackOffset &Offset,
 
 unsigned
 RISCVRegisterInfo::getRegisterCostTableIndex(const MachineFunction &MF) const {
-  return MF.getSubtarget<RISCVSubtarget>().hasStdExtCOrZca() ? 1 : 0;
+  return MF.getSubtarget<RISCVSubtarget>().hasStdExtCOrZca() &&
+                 !DisableCostPerUse
+             ? 1
+             : 0;
 }
 
 // Add two address hints to improve chances of being able to use a compressed

Copy link
Collaborator

@preames preames left a comment

Choose a reason for hiding this comment

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

LGTM

@topperc topperc merged commit c345198 into llvm:main Feb 28, 2024
@topperc topperc deleted the pr/disable-cost-per-use branch February 28, 2024 23:06
@dtcxzyw
Copy link
Member

dtcxzyw commented Feb 29, 2024

@topperc Could you please provide some performance data with the flag enabled?
Looks like enabling the flag leads to ~16% execution time regression and ~20% code size regression.

@topperc
Copy link
Collaborator Author

topperc commented Feb 29, 2024

@topperc Could you please provide some performance data with the flag enabled? Looks like enabling the flag leads to ~16% execution time regression and ~20% code size regression.

I can try to get some numbers. I only had some specific examples where it seemed to cause something bad. Here's an example where enabling it reduces the number of spills https://godbolt.org/z/M4YTad6cf though it does increase the code size.

mylai-mtk pushed a commit to mylai-mtk/llvm-project that referenced this pull request Jul 12, 2024
…sed registers. (llvm#83320)

I've seen cases where the cost per use increase the number of spills.
Disabling improves the codegen for llvm#79918.

I propose adding this option to allow easier experimentation.
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