[PATCH] D66456: [DAGCombiner][X86] Teach visitCONCAT_VECTORS to combine (concat_vectors (concat_vectors X, Y), undef)) -> (concat_vectors X, Y, undef, undef)

[Craig Topper via Phabricator via llvm-commits]

craig.topper marked 2 inline comments as done.
craig.topper added inline comments.


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:43259
     // subvector into a zero vector.
-    if (SubVec.getOpcode() == ISD::EXTRACT_SUBVECTOR && IdxVal == 0 &&
+    if (SubVec.getOpcode() == ISD::EXTRACT_SUBVECTOR &&
         isNullConstant(SubVec.getOperand(1)) &&
----------------
RKSimon wrote:
> Are we sure that this works in the general IdxVal case?
I think so. We know we were inserting a subvector and some zeroes into a zero vector. This just shrinks the subvector of the insertion so we don’t transfer the zeroes. Since everything but the new subvector is going to be zero that should be fine. The only thing that should matter is that the extract and inner insert use the same index so we know the extract covers the whole subvector and possibly some zeroes. Does that sound right?


================
Comment at: llvm/test/CodeGen/X86/avx512vl-vec-masked-cmp.ll:7593
 ; NoVLX-NEXT:    kmovw %k0, %eax
-; NoVLX-NEXT:    andl $3, %eax
+; NoVLX-NEXT:    movzbl %al, %eax
 ; NoVLX-NEXT:    vzeroupper
----------------
RKSimon wrote:
> This looks like we're missing a computeKnownBitsForTargetNode handling for a X86ISD opcode?
There aren’t any target nodes here. The kshifts should be coming from isel for an insert_subvector. I think we’re probably missing a combine for insert_subvector into zero followed by an insert into undef. Maybe with an extract between them.


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

https://reviews.llvm.org/D66456