[PATCH] D125525: [LegalizeTypes][VP] Add integer promotion support for VP_SIGN_EXTEND and VP_ZERO_EXTEND

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp:748
+        N->getOpcode() == ISD::VP_ZERO_EXTEND) {
+      assert((N->getOpcode() == ISD::VP_SIGN_EXTEND ||
+              N->getOpcode() == ISD::VP_ZERO_EXTEND) &&
----------------
This assert isn't useful, it's repeating the `if` condition.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp:752
+
+      Res = N->getOpcode() == ISD::VP_SIGN_EXTEND
+                ? SExtPromotedInteger(N->getOperand(0))
----------------
This handling is quite different than what was done for the none-VP version. Are we only doing this so getNode will squash the de-generate SIGN_EXTEND/ZERO_EXTEND that we get when `NVT == Res.getValueType()`?


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp:1983
+                       DAG.getValueType(N->getOperand(0).getValueType()));
+  } else {
+    assert(N->getOpcode() == ISD::VP_SIGN_EXTEND &&
----------------
No need for `else` if the `if` returned


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp:1986
+           "Expected VP_SIGN_EXTEND opcode");
+    Op = SExtPromotedInteger(N->getOperand(0));
+    return DAG.getNode(N->getOpcode(), dl, N->getValueType(0), Op,
----------------
Using SExtPromotedInteger creates a SIGN_EXTEND_INREG node. But that doesn't preserve the mask and VL. Should we have VP_SIGN_EXTEND_INREG?  @frasercrmck @simoll 


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp:2129
+    return DAG.getZeroExtendInReg(Op, dl, N->getOperand(0).getValueType());
+  } else {
+    assert(N->getOpcode() == ISD::VP_ZERO_EXTEND &&
----------------
No need for `else` if the `if` returned


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp:2132
+           "Expected VP_ZERO_EXTEND opcode");
+    Op = ZExtPromotedInteger(N->getOperand(0));
+    return DAG.getNode(N->getOpcode(), dl, N->getValueType(0), Op,
----------------
ZExtPromotedInteger uses ISD::AND, should we have a VP_AND here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125525