[PATCH] D144936: [SPARC][IAS] Recognize more SPARCv9 instructions/pseudoinstructions

[Jessica Clarke via Phabricator via llvm-commits]

jrtc27 added inline comments.


================
Comment at: llvm/lib/Target/Sparc/AsmParser/SparcAsmParser.cpp:667
+  if (!is64Bit())
+    return Error(IDLoc, "setx is only available in 64-bit mode");
+
----------------
Shouldn't this be impossible due to the predicate, and thus be an assert?


================
Comment at: llvm/lib/Target/Sparc/AsmParser/SparcAsmParser.cpp:674
+  // Small positive immediates effectively becomes a `set`.
+  if (ImmValue > 0LL && ImmValue <= 4294967295LL)
+    return expandSET(Inst, IDLoc, Instructions);
----------------
`isUInt<32>(ImmValue)`?


================
Comment at: llvm/lib/Target/Sparc/AsmParser/SparcAsmParser.cpp:680
+  if (IsImm && IsSmallNegative) {
+    MCInst TmpInst;
+    const MCExpr *Expr = MCConstantExpr::create(ImmValue, getContext());
----------------
Use MCInstBuilder throughout? Avoids all the `{ ... }` below, too...


================
Comment at: llvm/lib/Target/Sparc/AsmParser/SparcAsmParser.cpp:1149
+    ASIVal = StringSwitch<int64_t>(Parser.getTok().getString())
+                 .Case("ASI_N", 0x4)
+                 .Case("ASI_N_L", 0xC)
----------------
Use a TableGen GenericTable for this to avoid having to define the forward mapping here and the backward mapping later? (See RISCVSystemOperands.td for an example).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144936