[PATCH] D68203: Add support for (expressing) vscale.

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen marked 4 inline comments as done.
sdesmalen added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp:5139
+      int64_t MulImm = cast<ConstantSDNode>(N1->getOperand(0))->getSExtValue();
+      return getVScale(DL, VT, MulImm * N2C->getSExtValue());
+    }
----------------
efriedma wrote:
> efriedma wrote:
> > This multiply can overflow.
> This multiply can still overflow.  (If it's supposed to wrap, please use an unsigned multiply.)
Sorry, I'm not really sure I understand how the multiply above can overflow. The code checks the NSW flag on the MUL node, so if `VSCALE(Imm) *N2C` does not wrap, why would `Imm * N2C` wrap?

Perhaps int64_t is the wrong choice here though and we should instead be using APInt for the VScale operand (for the general case where VT > int64_t)


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

https://reviews.llvm.org/D68203