Skip to content

Conversation

@arichardson
Copy link
Member

I have a change to validate the operand classes emitted in the AsmParser
and that caused llvm/test/MC/RISCV/rv32p-valid.s to fail due to the rd_wb
register using a different register class from rd:
PWADDA_H operand 1 register X6 is not a member of register class GPRPair
This happens because tablegen's AsmMatcherEmitter emits code to literally
copy over the tied registers and does not feed them through the equivalent
of RISCVAsmParser::validateTargetOperandClass() which would allow adjusting
these operand classes.

Ideally we would handle this in tablegen (or at least add an error), but
the tied operand handling logic is rather complex and I don't understand
it yet. For now just update the rd register class to match rd_wb.

Created using spr 1.3.8-beta.1
@arichardson arichardson requested review from lenary, realqhc and topperc and removed request for realqhc and topperc December 10, 2025 23:31
@llvmbot
Copy link
Member

llvmbot commented Dec 10, 2025

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

Author: Alexander Richardson (arichardson)

Changes

I have a change to validate the operand classes emitted in the AsmParser
and that caused llvm/test/MC/RISCV/rv32p-valid.s to fail due to the rd_wb
register using a different register class from rd:
PWADDA_H operand 1 register X6 is not a member of register class GPRPair
This happens because tablegen's AsmMatcherEmitter emits code to literally
copy over the tied registers and does not feed them through the equivalent
of RISCVAsmParser::validateTargetOperandClass() which would allow adjusting
these operand classes.

Ideally we would handle this in tablegen (or at least add an error), but
the tied operand handling logic is rather complex and I don't understand
it yet. For now just update the rd register class to match rd_wb.


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

1 Files Affected:

  • (modified) llvm/lib/Target/RISCV/RISCVInstrInfoP.td (+1-1)
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfoP.td b/llvm/lib/Target/RISCV/RISCVInstrInfoP.td
index bba9f961b9639..7250a48bfe895 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfoP.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfoP.td
@@ -437,7 +437,7 @@ class RVPTernary_rrr<bits<4> f, bits<2> w, bits<3> funct3, string opcodestr>
 let hasSideEffects = 0, mayLoad = 0, mayStore = 0 in
 class RVPWideningTernary_rrr<bits<4> f, bits<2> w, string opcodestr>
     : RVPWideningBase<w, 0b1, (outs GPRPairRV32:$rd_wb),
-                     (ins GPR:$rd, GPR:$rs1, GPR:$rs2), opcodestr> {
+                     (ins GPRPairRV32:$rd, GPR:$rs1, GPR:$rs2), opcodestr> {
   let Inst{30-27} = f;
 
   let Constraints = "$rd = $rd_wb";

@arichardson arichardson requested a review from topperc December 10, 2025 23:33
@arichardson
Copy link
Member Author

Not sure if there was a reason to have these classes different in #154088.

The PR to add validation is: #171739 and this was the one issue it spotted.

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

@arichardson arichardson merged commit c5470e0 into main Dec 11, 2025
12 checks passed
@arichardson arichardson deleted the users/arichardson/spr/risc-vmc-fix-tied-operand-register-class-mismatch-in-p-extension branch December 11, 2025 00:39
llvm-sync bot pushed a commit to arm/arm-toolchain that referenced this pull request Dec 11, 2025
…-extension

I have a change to validate the operand classes emitted in the AsmParser
and that caused llvm/test/MC/RISCV/rv32p-valid.s to fail due to the rd_wb
register using a different register class from rd:
`PWADDA_H operand 1 register X6 is not a member of register class GPRPair`
This happens because tablegen's AsmMatcherEmitter emits code to literally
copy over the tied registers and does not feed them through the equivalent
of RISCVAsmParser::validateTargetOperandClass() which would allow adjusting
these operand classes.

Ideally we would handle this in tablegen (or at least add an error), but
the tied operand handling logic is rather complex and I don't understand
it yet. For now just update the rd register class to match rd_wb.

Pull Request: llvm/llvm-project#171738
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