[PATCH] D100788: [SystemZ] Support i128 inline asm operands

[Ulrich Weigand via Phabricator via llvm-commits]

uweigand added a comment.

Now it's looking mostly good to me; a couple of (mainly cosmetic) remaining comments inline.



================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:8606
 
           MVT RegVT = AsmNodeOperands[CurOp+1].getSimpleValueType();
           SmallVector<unsigned, 4> Regs;
----------------
Maybe avoid using AsmNodeOperands[CurOp+1] twice by taking the type of the TiedReg below?


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:8621
                                "inline asm error: This value type register "
                                "class is not natively supported!");
             return;
----------------
The wording of the error message now no longer makes sense.   In fact, I think with the new code it no longer can even happen for RC to be null, so I guess the whole error can be removed.


================
Comment at: llvm/lib/Target/SystemZ/SystemZISelLowering.h:428
+                  Optional<MVT> RegisterVT) const override {
+    if (VT == MVT::i128 &&
+        RegisterVT.hasValue() && RegisterVT.getValue() == MVT::Untyped)
----------------
A brief comment would be helpful here.


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

https://reviews.llvm.org/D100788