[PATCH] D56281: [DAGCombiner] reduce buildvec of zexted extracted element to shuffle

[Sanjay Patel via Phabricator via llvm-commits]

spatel marked 2 inline comments as done.
spatel added inline comments.


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:16136
+  SDValue Zext = BV->getOperand(ZextElt);
+  if (Zext.getOpcode() != ISD::ZERO_EXTEND || !Zext.hasOneUse() ||
+      Zext.getOperand(0).getOpcode() != ISD::EXTRACT_VECTOR_ELT ||
----------------
RKSimon wrote:
> Should ANY_EXTEND be handled as well? SimplifyDemandedBits can reduce ZERO_EXTEND -> ANY_EXTEND more aggressively these days.
That seems ok, and I can update with a draft of that change. But any idea how to make a test that would provide coverage for that pattern? I'm not showing any existing test diffs. Alternatively, I can add a TODO enhancement comment while trying to find a way to make that happen.


================
Comment at: test/CodeGen/X86/buildvec-extract.ll:422
+; AVX-NEXT:    vpshufb {{.*#+}} xmm0 = xmm0[2,3],zero,zero,zero,zero,zero,zero,zero,zero,zero,zero,zero,zero,xmm0[6,7]
 ; AVX-NEXT:    retq
   %e = extractelement <8 x i16> %x, i32 1
----------------
RKSimon wrote:
> Please can you investigate what's happening here? The xmm0[6,7] at the end seems really weird....
Hmm - I didn't notice that before. 

There's some shuffle lowering madness that I haven't stepped through yet that creates several nodes before this becomes the single pshufb. 

I filed PR40306 to track this:
https://bugs.llvm.org/show_bug.cgi?id=40306


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

https://reviews.llvm.org/D56281