[PATCH] D79546: [VE] Implements minimum MC layer for VE (4/4)

[Simon Moll via Phabricator via llvm-commits]

simoll added a comment.

Thanks for splitting up the patches!
The VEAsmParser class is still relatively large but the tests seem to cover all cases and i don't think it makes sense to slice up a testable unit only to drive down the patch size.
This patch gives us mclayer support for 'LEA' type instructions in objects without relocations, correct? (Whether i am right or not, you might want to add to the commit message what functionality this patch provides).
LGTM except for some nits.



================
Comment at: llvm/lib/Target/VE/AsmParser/VEAsmParser.cpp:557
+  unsigned IndexReg = 0;
+  ;
+
----------------
Stray `;`


================
Comment at: llvm/lib/Target/VE/AsmParser/VEAsmParser.cpp:612
+                                               StringRef Mnemonic) {
+  LLVM_DEBUG(dbgs() << "parseOpeand\n");
+  OperandMatchResultTy ResTy = MatchOperandParserImpl(Operands, Mnemonic);
----------------
typo `Operand`


================
Comment at: llvm/lib/Target/VE/AsmParser/VEAsmParser.cpp:622
+  switch (getLexer().getKind()) {
+  case AsmToken::LParen:
+    // FIXME: Parsing "(" + %vreg + ", " + %vreg + ")"
----------------
I suppose there should be a TODO here for the `(m)1` and `(m)0` immediates?


================
Comment at: llvm/lib/Target/VE/AsmParser/VEAsmParser.cpp:647
+VEAsmParser::parseVEAsmOperand(std::unique_ptr<VEOperand> &Op) {
+  LLVM_DEBUG(dbgs() << "parseVEAsmOpeand\n");
+  SMLoc S = Parser.getTok().getLoc();
----------------
typo `Operand`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79546