[PATCH] D116039: [X86] Combine reduce (add (mul x, y)) to VNNI instruction.

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added a comment.

Please fix the patch description - both extensions there are signed, is that actually the specification for the intrinsic?



================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:41790
+  // (dpbusd (zext a), (sext, b))
+  if ((IsFreeTruncation(Op0) && NumNonSignBits(Op0) <= 8) &&
+      (IsFreeTruncation(Op1) && NumNonSignBits(Op1) < 8))
----------------
lebedev.ri wrote:
> This still does not handle the commutative variant.
I'm not sure i follow.
Why is this okay with negative numbers?


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:41790-41791
+  // (dpbusd (zext a), (sext, b))
+  if ((IsFreeTruncation(Op0) && NumNonSignBits(Op0) <= 8) &&
+      (IsFreeTruncation(Op1) && NumNonSignBits(Op1) < 8))
+    return true;
----------------
This still does not handle the commutative variant.


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:41791
+  if ((IsFreeTruncation(Op0) && NumNonSignBits(Op0) <= 8) &&
+      (IsFreeTruncation(Op1) && NumNonSignBits(Op1) < 8))
+    return true;
----------------
You want `ComputeMinSignedBits() <= 8` to check for sext-like


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116039