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

[LuoYuanke via Phabricator via llvm-commits]

LuoYuanke added inline comments.


================
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:
> lebedev.ri wrote:
> > lebedev.ri wrote:
> > > This still does not handle the commutative variant.
> > I'm not sure i follow.
> > Why is this okay with negative numbers?
> https://alive2.llvm.org/ce/z/WGxXrz vs https://alive2.llvm.org/ce/z/UbWhNv
> I'm not sure i follow.
> Why is this okay with negative numbers?

Here is description for VPDPBUSD from https://software.intel.com/content/www/us/en/develop/download/intel-architecture-instruction-set-extensions-programming-reference.html?wapkw=instruction. The first operand is unsigned, and the second operand is signed.


```
Multiplies the individual unsigned bytes of the first source operand by the corresponding signed bytes of the second source operand, producing intermediate signed word results. The word results are then summed and accumulated in the destination dword element size operand.
```


================
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;
----------------
LuoYuanke wrote:
> lebedev.ri wrote:
> > lebedev.ri wrote:
> > > lebedev.ri wrote:
> > > > This still does not handle the commutative variant.
> > > I'm not sure i follow.
> > > Why is this okay with negative numbers?
> > https://alive2.llvm.org/ce/z/WGxXrz vs https://alive2.llvm.org/ce/z/UbWhNv
> > I'm not sure i follow.
> > Why is this okay with negative numbers?
> 
> Here is description for VPDPBUSD from https://software.intel.com/content/www/us/en/develop/download/intel-architecture-instruction-set-extensions-programming-reference.html?wapkw=instruction. The first operand is unsigned, and the second operand is signed.
> 
> 
> ```
> Multiplies the individual unsigned bytes of the first source operand by the corresponding signed bytes of the second source operand, producing intermediate signed word results. The word results are then summed and accumulated in the destination dword element size operand.
> ```
> This still does not handle the commutative variant.

Sorry, I don't understand it very well. I do it with " std::swap(Op0, Op1)" in line 41764;


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