[PATCH] D141984: [RISCV][MC] Add support for experimental zfa extension(FLI instruction not included)

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp:1859
 
+OperandMatchResultTy RISCVAsmParser::parseRTZArg(OperandVector &Operands) {
+  if (getLexer().isNot(AsmToken::Identifier)) {
----------------
joshua-arch1 wrote:
> craig.topper wrote:
> > joshua-arch1 wrote:
> > > joshua-arch1 wrote:
> > > > craig.topper wrote:
> > > > > Can we reuse parseFRMArg and check the value in isRTZArg()? Add a `DiagnosticType` to `RTZArg` and diagnose in RISCVAsmParser::MatchAndEmitInstruction
> > > > I think if we reuse parseFRMArg and check the value in isRTZArg(), the code quantity will be larger, since we need to add extra logic in isRTZArg() like what you did in initial isFRMArg() implementation. 
> > > Since we have moved FRM parsing to a custom operand parser, it's briefer and more consistent to check all types of FRM operands in operand parsers instead of in isFRMArg()/isRTZArg(). 
> > This only seems to have been partially done?
> > This only seems to have been partially done?
> 
> Did you mean 'reuse parseFRMArg and check the value in isRTZArg()'? I didn't do that. You can check my reply to your last comment.
Yes.

We can use parseFRMArg for parsing. And in isRTZArg() check `isFRMArg() && FRM.FRM == RISCVFPRndMode::RTZ;'


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141984/new/

https://reviews.llvm.org/D141984