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

[LuoYuanke via Phabricator via llvm-commits]

LuoYuanke marked 3 inline comments as done.
LuoYuanke added a comment.

I'll update the patch according to Phoebe and Craig's comments.



================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:41813-41814
+      [](SDValue &Op) -> bool {
+        if ((Op.getOpcode() == ISD::ZERO_EXTEND ||
+             Op.getOpcode() == ISD::SIGN_EXTEND) &&
+            Op.getOperand(0).getValueType().getScalarSizeInBits() <= 8)
----------------
pengfei wrote:
> How about `ANY_EXTEND` ? The same below.
This check the opcode, so we need check both zero extend and sign extend. I'm not sure if any extend also works, because the upper bits is undefined. What's the signed bit for any extend?


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:42156-42157
+  EVT ExtractVT = Extract->getValueType(0);
+  // Verify the type we're extracting is i32, as the output element type of
+  // vpdpbusd and vpdpwssd is i32.
+  if (ExtractVT != MVT::i32)
----------------
pengfei wrote:
> Can we generate i32 first then do the truncation?
I'm not sure if the result overflow, truncating back to i16 or some other types remain the same value. How about leave it as an enhancement?


================
Comment at: llvm/test/CodeGen/X86/dpbusd.ll:13
+; AVXVNNI-NEXT:    vextracti128 $1, %ymm0, %xmm1
+; AVXVNNI-NEXT:    vpaddd %xmm1, %xmm0, %xmm0
+; AVXVNNI-NEXT:    vpshufd {{.*#+}} xmm1 = xmm0[2,3,2,3]
----------------
pengfei wrote:
> This is the only and a strange diff with the AVX512 code. Is there anything wrong in one of each?
This test doesn't generate vpdpbusd instruction, so the AVX512VNNI and AVX512VL generate the same code. For other test case, AVX512VNNI can only use zmm register, but AVX512VNNI + AVX512VL can use xmm register. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116039