[PATCH] D157233: [SPARC][IAS] Add complete set of v9 ASI load, store & swap forms

[Sergei Barannikov via Phabricator via llvm-commits]

barannikov88 added a reviewer: jyknight.
barannikov88 added inline comments.


================
Comment at: llvm/lib/Target/Sparc/AsmParser/SparcAsmParser.cpp:1117
+  std::unique_ptr<SparcOperand> Mask;
+  if (parseSparcAsmOperand(Mask) != MatchOperand_Success) {
+    Error(S, "malformed ASI tag");
----------------
Can you use `getParser().parseExpression()` here?
Why is the variable named Mask?



================
Comment at: llvm/lib/Target/Sparc/AsmParser/SparcAsmParser.cpp:1118
+  if (parseSparcAsmOperand(Mask) != MatchOperand_Success) {
+    Error(S, "malformed ASI tag");
+    return MatchOperand_ParseFail;
----------------
Missing test case.



================
Comment at: llvm/lib/Target/Sparc/AsmParser/SparcAsmParser.cpp:1124
+      ASIVal < 0 || ASIVal > 255) {
+    Error(S, "invalid ASI number, must be between 0 and 255");
+    return MatchOperand_ParseFail;
----------------
Missing test case.


================
Comment at: llvm/lib/Target/Sparc/AsmParser/SparcAsmParser.cpp:1210
       std::unique_ptr<SparcOperand> Op;
-      ResTy = parseSparcAsmOperand(Op, false);
-      if (ResTy != MatchOperand_Success || !Op)
-        return MatchOperand_ParseFail;
-      Operands.push_back(std::move(Op));
+      if (parseSparcAsmOperand(Op) == MatchOperand_Success && Op &&
+          Op->isReg() && Op->getReg() == Sparc::ASI) {
----------------
parseSparcAsmOperand is overkill here, you can simply check for 'asi' identifier.
There should be negative test cases with invalid register name (e.g. %g0), percent followed by a non-identifier (e.g. %0), invalid token (e.g. $asi).



================
Comment at: llvm/lib/Target/Sparc/AsmParser/SparcAsmParser.cpp:1218
+          if (OldMemOp.getMemOffsetReg() != Sparc::G0)
+            return MatchOperand_ParseFail;
+          Operands[Operands.size() - 2] = SparcOperand::MorphToMEMri(
----------------
Please add a negative test for this code path.



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

https://reviews.llvm.org/D157233