luismarques added a comment.
This patch is nearly there! Just address the remaining review comments and it
LGTM.
BTW, please mark all addressed inline comments as done. I think a few were
missed, and it's helpful for a large patch like this.
================
Comment at: llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp:953-956
+static MCRegister convertGPRToGPRPair(MCRegister Reg) {
+ assert(isGPRPair(Reg) && "Invalid register");
+ return (Reg - RISCV::X0) / 2 + RISCV::X0_ZERO;
+}
----------------
Nitpicking: perhaps this could use better terminology. You're asserting that
`Reg` is a GPRPair, and then you convert `Reg` to a GPRPair, but the return
value would fail an assert of `isGPRPair`...
================
Comment at: llvm/lib/Target/RISCV/RISCVRegisterInfo.td:69
+def sub_lo : SubRegIndex<32>;
+def sub_hi : SubRegIndex<32, 32>;
----------------
jrtc27 wrote:
> This assumes RV32, and is not clear it applies to register pairs
What's the best way to address this?
================
Comment at: llvm/test/MC/RISCV/rv32zpsfoperand-invalid.s:6-9
+# Paired register operand
+
+# CHECK-ERROR: error: invalid operand for instruction
+smal a0, a1, a2
----------------
It would be nice to have a more specific error message, indicating the need for
an even register operand. But not a deal-breaker, IMO.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D95588/new/
https://reviews.llvm.org/D95588
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
