[PATCH] D36454: [X86] Changes to extract Horizontal addition operation for AVX-512.

[Craig Topper via Phabricator via llvm-commits]

craig.topper added a comment.

Why this be solved by just combining extract_subvectors through things like binops and onto their inputs.

Like this:
combine (extract_subvector (add X, Y)) -> (narrower_add (extract_subvector X, extract_subvector Y))



================
Comment at: lib/Target/X86/X86ISelLowering.cpp:30189
+      SDValue NewEltIdx =
+          DAG.getConstant((ExtractedElt - StartIdx), dl, MVT::i32);
+
----------------
I think element indices should use DAG.getIntPtrConstant to make the type correct. It's not supposed to be i32.


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:33793
 
+  bool ExtractLHSSubVec = false;
+  ConstantSDNode *CSD = nullptr;
----------------
Can we teach combineExtractSubvector to narrow an extract of an add by extracting from the inputs? Then we shouldn't need any change here because we'll already have the narrow add?


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:33886
   // Check that the masks correspond to performing a horizontal operation.
-  for (unsigned l = 0; l != NumElts; l += NumLaneElts) {
+  for (unsigned L = 0; L != NumElts; L += NumLaneElts) {
     for (unsigned i = 0; i != NumLaneElts; ++i) {
----------------
Why is this variable now upper case?


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:33936
 
+  auto NewOpcode = IsFadd ? X86ISD::FHADD : X86ISD::FHSUB;
+
----------------
Why was this moved out of the if.


================
Comment at: test/CodeGen/X86/madd.ll:299
+; AVX2-NEXT:    vpaddd %xmm1, %xmm0, %xmm0
 ; AVX2-NEXT:    vphaddd %ymm0, %ymm0, %ymm0
 ; AVX2-NEXT:    vmovd %xmm0, %eax
----------------
Why is this horizontal add not narrowed?


https://reviews.llvm.org/D36454