[PATCH] D39895: [RISCV] MC layer support for the standard RV32D instruction set extension

Alex Bradbury via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Nov 21 04:36:33 PST 2017


asb added inline comments.


================
Comment at: lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp:427
+  RISCVOperand &Op = static_cast<RISCVOperand &>(AsmOp);
+  if (Op.isReg()) {
+    unsigned Reg = Op.getReg();
----------------
apazos wrote:
> consider early exit, return  Match_InvalidOperand from here if Op not Reg
I considered that, but I don't think it's really a net improvement. You still need a return Match_InvalidOperand at the end ocatch the case where if (IsRegFPR32 && Kidn == MCK_FPR64) is false. I also think it's a little cleaner to use early return consistently for success or failure if feasible (e.g. in the current code, we only use early return for Match_Success which means it's easy to trace the control flow for failure). Using early return for !Op.isReg() would look like this. I prefer the current code, but let me know if you feel otherwise!

```
unsigned RISCVAsmParser::validateTargetOperandClass(MCParsedAsmOperand &AsmOp,
                                                    unsigned Kind) {
  RISCVOperand &Op = static_cast<RISCVOperand &>(AsmOp);
  if (!Op.isReg())
    return Match_InvalidOperand;

  unsigned Reg = Op.getReg();
  bool IsRegFPR32 =
      RISCVMCRegisterClasses[RISCV::FPR32RegClassID].contains(Reg);

  // As the parser couldn't differentiate an FPR32 from an FPR64, coerce the
  // register from FPR32 to FPR64 if necessary.
  if (IsRegFPR32 && Kind == MCK_FPR64) {
    Op.Reg.RegNum = convertFPR32ToFPR64(Reg);
    return Match_Success;
  }
  return Match_InvalidOperand;
}
```


https://reviews.llvm.org/D39895





More information about the llvm-commits mailing list