Skip to content

[RISCV] Set CopyCost on register classes [NFC] #131185

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
Mar 13, 2025

Conversation

preames
Copy link
Collaborator

@preames preames commented Mar 13, 2025

This change sets up CopyCost to reflect the cost of copying a vector register group, a vector segment, or a register pair.

This is NFC because the only actual usage of this field in tree is simply checking that cost is not negative. That heuristic is around ABI copy elimination during instruction emission + scheduling, and doesn't immediately seem like it should apply for us.

This change sets up CopyCost to reflect the cost of copying a vector
register group, a vector segment, or a register pair.

This is NFC because the only actual usage of this field in tree is
simply checking that cost is not negative.  That heuristic is around
ABI copy elimination during instruction emission + scheduling, and
doesn't immediately seem like it should apply for us.
@llvmbot
Copy link
Member

llvmbot commented Mar 13, 2025

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

Author: Philip Reames (preames)

Changes

This change sets up CopyCost to reflect the cost of copying a vector register group, a vector segment, or a register pair.

This is NFC because the only actual usage of this field in tree is simply checking that cost is not negative. That heuristic is around ABI copy elimination during instruction emission + scheduling, and doesn't immediately seem like it should apply for us.


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

1 Files Affected:

  • (modified) llvm/lib/Target/RISCV/RISCVRegisterInfo.td (+3-2)
diff --git a/llvm/lib/Target/RISCV/RISCVRegisterInfo.td b/llvm/lib/Target/RISCV/RISCVRegisterInfo.td
index a5dfb5ba1a2fc..bbb1e82fbadaa 100644
--- a/llvm/lib/Target/RISCV/RISCVRegisterInfo.td
+++ b/llvm/lib/Target/RISCV/RISCVRegisterInfo.td
@@ -226,6 +226,7 @@ class RISCVRegisterClass<list<ValueType> regTypes, int align, dag regList>
   int NF = 1;
 
   let Size = !if(IsVRegClass, !mul(VLMul, NF, 64), 0);
+  let CopyCost = !if(IsVRegClass, !mul(VLMul, NF), 1);
 
   let TSFlags{0} = IsVRegClass;
   let TSFlags{3-1} = !logtwo(VLMul);
@@ -343,7 +344,7 @@ let RegAltNameIndices = [ABIRegAltName] in {
   }
 }
 
-let RegInfos = XLenPairRI,
+let RegInfos = XLenPairRI, CopyCost = 2,
     DecoderMethod = "DecodeGPRPairRegisterClass" in {
 def GPRPair : RISCVRegisterClass<[XLenPairVT, XLenPairFVT], 64, (add
     X10_X11, X12_X13, X14_X15, X16_X17,
@@ -357,7 +358,7 @@ def GPRPair : RISCVRegisterClass<[XLenPairVT, XLenPairFVT], 64, (add
 def GPRPairNoX0 : RISCVRegisterClass<[XLenPairVT, XLenPairFVT], 64, (sub GPRPair, X0_Pair)>;
 } // let RegInfos = XLenPairRI, DecoderMethod = "DecodeGPRPairRegisterClass"
 
-let RegInfos = XLenPairRI in
+let RegInfos = XLenPairRI, CopyCost = 2 in
 def GPRPairC : RISCVRegisterClass<[XLenPairVT, XLenPairFVT], 64, (add
   X10_X11, X12_X13, X14_X15, X8_X9
 )>;

Copy link
Collaborator

@topperc topperc left a comment

Choose a reason for hiding this comment

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

LGTM.

@preames preames merged commit bbb244c into llvm:main Mar 13, 2025
11 of 12 checks passed
@preames preames deleted the pr-riscv-register-class-copy-cost branch March 13, 2025 20:45
frederik-h pushed a commit to frederik-h/llvm-project that referenced this pull request Mar 18, 2025
This change sets up CopyCost to reflect the cost of copying a vector
register group, a vector segment, or a register pair.

This is NFC because the only actual usage of this field in tree is
simply checking that cost is not negative. That heuristic is around ABI
copy elimination during instruction emission + scheduling, and doesn't
immediately seem like it should apply for us.
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