Skip to content

[RISCV] Change the InstFormat for Zicbop prefetch instructions to InstFormatOther. #148934

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
Jul 15, 2025

Conversation

topperc
Copy link
Collaborator

@topperc topperc commented Jul 15, 2025

The lower 5-bits of the immediate are not part of the address unlike other InstFormatS instructions.

We use InstFormatS in RISCVRegisterInfo::needsFrameBaseReg and RISCVRegisterInfo::getFrameIndexInstrOffset which is not aware of this special encoding. Force the format to InstFormatOther so those functions will ignore it.

InstFormatS is also used by relocation emission, but I don't believe we ever emit these instructions with a relocation because of the encoding.

…tFormatOther.

The lower 5-bits of the immediate are not part of the address unlike
other InstFormatS instructions.

We use InstFormatS in RISCVRegisterInfo::needsFrameBaseReg
and RISCVRegisterInfo::getFrameIndexInstrOffset which is not aware of
this special encoding. Force the format to InstFormatOther so those
functions will ignore it.

InstFormatS is also used by relocation emission, but I don't believe
we ever emit these instructions with a relocation because of the encoding.
@llvmbot
Copy link
Member

llvmbot commented Jul 15, 2025

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

Author: Craig Topper (topperc)

Changes

The lower 5-bits of the immediate are not part of the address unlike other InstFormatS instructions.

We use InstFormatS in RISCVRegisterInfo::needsFrameBaseReg and RISCVRegisterInfo::getFrameIndexInstrOffset which is not aware of this special encoding. Force the format to InstFormatOther so those functions will ignore it.

InstFormatS is also used by relocation emission, but I don't believe we ever emit these instructions with a relocation because of the encoding.


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

2 Files Affected:

  • (modified) llvm/lib/Target/RISCV/RISCVInstrFormats.td (+3-1)
  • (modified) llvm/lib/Target/RISCV/RISCVInstrInfoZicbo.td (+1)
diff --git a/llvm/lib/Target/RISCV/RISCVInstrFormats.td b/llvm/lib/Target/RISCV/RISCVInstrFormats.td
index b6b64b57b1b3e..e23001a3a0bff 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrFormats.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrFormats.td
@@ -193,7 +193,9 @@ class RVInstCommon<dag outs, dag ins, string opcodestr, string argstr,
   let AsmString = opcodestr # !if(!empty(argstr), "", "\t" # argstr);
   let Pattern = pattern;
 
-  let TSFlags{4-0} = format.Value;
+  InstFormat Format = format;
+
+  let TSFlags{4-0} = Format.Value;
 
   // Defaults
   RISCVVConstraint RVVConstraint = NoConstraint;
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfoZicbo.td b/llvm/lib/Target/RISCV/RISCVInstrInfoZicbo.td
index 878b85b141578..0723b2f568a79 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfoZicbo.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfoZicbo.td
@@ -41,6 +41,7 @@ class Prefetch_ri<bits<5> optype, string opcodestr>
               opcodestr, "${imm12}(${rs1})"> {
   let Inst{11-7} = 0b00000;
   let rs2 = optype;
+  let Format = InstFormatOther; // this does not follow the normal S format.
 }
 
 //===----------------------------------------------------------------------===//

Copy link
Member

@lenary lenary left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks.

@topperc topperc merged commit b64d7ba into llvm:main Jul 15, 2025
10 of 11 checks passed
@topperc topperc deleted the pr/prefetch-format branch July 15, 2025 21:49
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