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

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: llvm/lib/Target/LoongArch/AsmParser/LoongArchAsmParser.cpp:147
+  bool isUImm3() const { return isUImm<3>(); }
+  bool isUImm5() const { return isUImm<5>(); }
+  bool isUImm6() const { return isUImm<6>(); }
----------------
These functions are unused. Usually it's better to add them in the patches that need them.

If the function is so simple, perhaps just use `isUImm<3>`. It just requires 2 more characters.


================
Comment at: llvm/lib/Target/LoongArch/AsmParser/LoongArchAsmParser.cpp:311
+/// Looks at a token type and creates the relevant operand from this
+/// information, adding to Operands. If operand was parsed, returns false, else
+/// true.
----------------
`If operand was parsed, returns false, else ...` => Return true upon an error.


================
Comment at: llvm/lib/Target/LoongArch/AsmParser/LoongArchAsmParser.cpp:316
+  // Attempt to parse token as a register.
+  if (parseRegister(Operands) == MatchOperand_Success)
+    return false;
----------------
The code self explains.
```
if (parseRegister(Operands) == MatchOperand_Success || parseImmediate(Operands) == MatchOperand_Success)
  return false;
```


================
Comment at: llvm/lib/Target/LoongArch/AsmParser/LoongArchAsmParser.cpp:336
+  if (getLexer().is(AsmToken::EndOfStatement))
+    return false;
+
----------------
This should be consumed. Not consuming it may cause some diagnostic line:column glitch, but I haven't tested.

`parseOptionalToken` can consume the token.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120476