This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] MC layer support for the standard RV32D instruction set extension
ClosedPublic

Authored by asb on Nov 10 2017, 3:39 AM.

Details

Summary

As the FPR32 and FPR64 registers have the same names, use validateTargetOperandClass in RISCVAsmParser to coerce a parsed FPR32 to an FPR64 when necessary. The rest of this patch is very similar to the RV32F patch.

CCing @dylanmckay and @venkatra, as both AVR and Sparc backends do a similar sort of coercion in validateTargetOperandClass.

Diff Detail

Repository
rL LLVM

Event Timeline

asb created this revision.Nov 10 2017, 3:39 AM
sameer.abuasal edited reviewers, added: sabuasal; removed: sameer.abuasal.Nov 13 2017, 11:49 AM
apazos added inline comments.Nov 16 2017, 5:47 PM
lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
427 ↗(On Diff #122415)

consider early exit, return Match_InvalidOperand from here if Op not Reg

lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp
124 ↗(On Diff #122415)

extra {}

asb updated this revision to Diff 123765.Nov 21 2017, 4:36 AM
asb marked an inline comment as done.

Rebase and remove extra {} around single statement if block.

asb added inline comments.Nov 21 2017, 4:36 AM
lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
427 ↗(On Diff #122415)

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;
}
reames accepted this revision.Dec 4 2017, 6:49 PM
reames added a subscriber: reames.

LGTM w/changes applied.

lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
427 ↗(On Diff #122415)

The early return variant is cleaner, or at least, much more idiomatic and thus easier to read quickly. :)

This revision is now accepted and ready to land.Dec 4 2017, 6:49 PM
This revision was automatically updated to reflect the committed changes.