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

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:41860
+  // Extend or truncate to MVT::i8 first.
+  SDValue ZExt0 = Ext0;
+  SDValue SExt1 = Ext1;
----------------
Just name the paramaters ZExt0 and SExt1 and get rid of Ext0 and Ext1?


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:41870
+  // Find the appropriate width for the DotProduct.
+  EVT InVT = ZExt0.getValueType();
+  // VPDPBUSD(<16 x i32>C, <16 x i8>A, <16 x i8>B). For each dst element
----------------
This just Vi8VT right?


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:41895
+  // AVXVNNI/AVX512VNNI.
+  auto DpBuilder = [&](SelectionDAG &DAG, const SDLoc &DL,
+                       ArrayRef<SDValue> Ops) {
----------------
Is this lambda getting anything via `&` in the capture list or can it just be `[]`


================
Comment at: llvm/lib/Target/X86/X86PartialReduction.cpp:100
+      (IsFreeTruncation(RHS) &&
+       ComputeMinSignedBits(RHS, *DL, 0, nullptr, nullptr) <= 8))
+    return true;
----------------
Why pass `0, nullptr, nullptr` when those have default values?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116039