[PATCH] D120476: [LoongArch] Add basic support to AsmParser

[WÁNG Xuěruì via Phabricator via llvm-commits]

xen0n added a comment.

Looks good ISA implementation-wise; I haven't independently reviewed the instruction encodings, but the immediate operand handling is consistent with manual syntax and binutils behavior.



================
Comment at: llvm/lib/Target/LoongArch/LoongArchInstrInfo.td:65
+
+def simm12 : Operand<GRLenVT>, ImmLeaf<GRLenVT, [{return isInt<12>(Imm);}]> {
+  let ParserMatchClass = SImmAsmOperand<12>;
----------------
I may have missed this in the previous patches, but why does only this kind of imm get the `ImmLeaf` treatment? I briefly looked at the other targets and they seem to mark most of their commonly used immediate kinds as such.


================
Comment at: llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchMCTargetDesc.cpp:122
 
     // Register the MCInstPrinter.
     TargetRegistry::RegisterMCInstPrinter(*T, createLoongArchMCInstPrinter);
----------------
Considering that every line in this function is self-documenting, do we really need any of them? At least with the addition below, *this* line becomes inaccurate...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120476