[PATCH] D41811: X86: Add pattern matching for PMADDWD

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:37042
+static SDValue matchPMADDWD(SelectionDAG &DAG, SDValue Op0, SDValue Op1,
+                              EVT VT, const X86Subtarget &Subtarget) {
+  // Example of pattern we try to detect:
----------------
Second line is indented 2 extra spaces.


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:37054
+
+  if (VT != MVT::v4i32 && (VT != MVT::v8i32 || !Subtarget.hasAVX2()) &&
+      (VT != MVT::v16i32 || !Subtarget.hasBWI()))
----------------
Do you need a hasSSE2 check on the v4i32? I don't see one before this call in combineAdd.


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:37062
+      SDValue Op, ArrayRef<unsigned> ExpectedIndices, SDValue &RetMul) {
+    if (Op->getOpcode() != ISD::BUILD_VECTOR)
+      return false;
----------------
What ensures the multiply has exactly 2X the elements of the build_vector? Couldn't it have more? Which would cause the truncate later to fail.


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:37066
+    for (unsigned i = 0, e = ExpectedIndices.size(); i != e; ++i) {
+      //TODO: Be more tollerant to undefs.
+      if (Op->getOperand(i)->getOpcode() != ISD::EXTRACT_VECTOR_ELT)
----------------
tolerant*


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:37087
+  SDValue L, R;
+  const unsigned ExpectedEvenIndices[] = {0,  2,  4,  6,  8,  10, 12, 14,
+                                          16, 18, 20, 22, 24, 26, 28, 30};
----------------
Is there anything that guarantees even indices will be on the LHS?


https://reviews.llvm.org/D41811