[PATCH] D56987: [Intrinsic] Expand vector SMULFIX to MUL on zero scale

[Bjorn Pettersson via Phabricator via llvm-commits]

bjope added a comment.

How common would it be that the scale is zero? Is that really expected in reality or just in this kind of handwritten test cases?

Regardless, this patch does not introduce that much complexity so it does LGTM.



================
Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:5373
+  if (VT.isVector())
+    // Do not scalarize here.
+    return SDValue();
----------------
nit: This comment is a little bit confusing (does "here" refer to this method or the context for the legalization). Returning SDValue() will result in an unroll, so by doing that I think we will end up scalarizing the mulfix operation. I'd expect a comment explaining that we return SDValue() to get scalarization, but now the comment could be read as "avoid scalarization by returning SDValue()".

Maybe you can change the wording somehow, or simply remove the comment.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D56987