[PATCH] D63973: [MachineVerifier] Improve checks of target instructions operands.

[Florian Hahn via Phabricator via llvm-commits]

fhahn accepted this revision.
fhahn added a comment.
This revision is now accepted and ready to land.

LGTM, with a few small suggestions. Thanks for your patience. Please wait with committing until early next week, in case people on Thanksgiving break have additional comments.



================
Comment at: llvm/lib/CodeGen/MachineVerifier.cpp:1614
     // e.g., LDM_RET in the arm back end.
-    if (MO->isReg() &&
-        !(MI->isVariadic() && MONum == MCID.getNumOperands()-1)) {
+    bool IsOptional = (MI->isVariadic() && MONum == MCID.getNumOperands() - 1);
+    if (MO->isReg() && !IsOptional) {
----------------
nit: no braces needed.


================
Comment at: llvm/lib/CodeGen/MachineVerifier.cpp:1615
+    bool IsOptional = (MI->isVariadic() && MONum == MCID.getNumOperands() - 1);
+    if (MO->isReg() && !IsOptional) {
       if (MO->isDef() && !MCOI.isOptionalDef())
----------------
Personally, I think it would be slightly better to guard the both blocks with a single `if (!Optional)` and a comment like `’// Check non-variadic operands only.` as both blocks apply to non-variadic operands.



================
Comment at: llvm/lib/CodeGen/MachineVerifier.cpp:1625
+      if (MCOI.OperandType == MCOI::OPERAND_REGISTER &&
+          (!MO->isReg() && !MO->isFI()))
+        report("Expected a register operand.", MO, MONum);
----------------
nit: no braces needed.


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

https://reviews.llvm.org/D63973