[PATCH] D43608: [X86] Use setcc ISD opcode for AVX512 integer comparisons all the way to isel

[Simon Pilgrim via Phabricator via llvm-commits]

RKSimon accepted this revision.
RKSimon added a comment.
This revision is now accepted and ready to land.

LGTM with a couple of minors



================
Comment at: lib/Target/X86/X86InstrAVX512.td:3239
 let Predicates = [HasBWI, NoVLX] in {
+  let AddedComplexity = 2 in {
   defm : axv512_icmp_packed_no_vlx_lowering<X86pcmpgtm, "VPCMPGTB", v32i8x_info, v64i8_info>;
----------------
Add a comment explaining the AddedComplexity?


================
Comment at: lib/Target/X86/X86InstrAVX512.td:3180
 let Predicates = [HasAVX512, NoVLX] in {
+  let AddedComplexity = 2 in {
   defm : axv512_icmp_packed_no_vlx_lowering<X86pcmpgtm, "VPCMPGTD", v8i32x_info, v16i32_info>;
----------------
craig.topper wrote:
> This is needed because the explicit CondCode match doesn't seem to increase the pattern complexity the way an explicit CondCode match does.
Can you put this in a comment here?


================
Comment at: test/CodeGen/X86/broadcastm-lowering.ll:143
 ; AVX512CD-NEXT:    kmovw %k0, %eax
-; AVX512CD-NEXT:    vpxor %xmm0, %xmm0, %xmm0
-; AVX512CD-NEXT:    vpinsrb $0, %eax, %xmm0, %xmm0
-; AVX512CD-NEXT:    vpinsrb $8, %eax, %xmm0, %xmm0
-; AVX512CD-NEXT:    vinserti128 $1, %xmm0, %ymm0, %ymm0
+; AVX512CD-NEXT:    movzbl %al, %eax
+; AVX512CD-NEXT:    vmovq %rax, %xmm0
----------------
craig.topper wrote:
> I'm not sure why this changed. I think its because vector lowering didn't change anything. It used to change setcc to pcmpm. But it doesn't now, so the post vector lowering DAG combine doesn't run. And there's a target independent DAG combine, reduceBuildVecExtToExtBuildVec, that is only allowed to run after legalize vector ops. And if the post vector ops dag combine doesn't run, then the build vector is lowered before the last DAG combine runs so reduceBuildVecExtToExtBuildVec doesn't get called.
Isn't this actually better?


https://reviews.llvm.org/D43608