[X86] Fix a codegen crash in getSetCCResultType

[pengfei]

Summary

This patch fixes some crashes coming from
X86ISelLowering::getSetCCResultType, which would occasionally return
an EVT constructed from an invalid MVT, which has a null Type pointer.

This patch refers to D95434.

[craig.topper]

I don’t think I understand. VT is an EVT. Why does changeVectorElementType involve MVT?

[craig.topper]

Oh I see. changeVectorElementType assumes that if the type is simple and type to change to is simple it will succeed in MVT. That seems like a poor implementation. Maybe it should check the result and fallback to the extended implementation of it fails on MVT?

This patch seems fine though. LGTM

[frasercrmck]

In D97036#2574085, @craig.topper wrote:

Oh I see. changeVectorElementType assumes that if the type is simple and type to change to is simple it will succeed in MVT. That seems like a poor implementation. Maybe it should check the result and fallback to the extended implementation of it fails on MVT?

Yeah that's.. questionable. I'd +1 that change.

This change, though, LGTM.

[frasercrmck]

If I'm not mistaken, changeTypeToInteger and changeVectorElementTypeToInteger could in theory suffer from the same problem. Though less likely, as any MVT is most likely going to have an MVT with the same-sized integer type.

[pengfei]

Thanks Craig and Fraser.

[craig.topper]

In D97036#2574089, @frasercrmck wrote:

In D97036#2574085, @craig.topper wrote:

Oh I see. changeVectorElementType assumes that if the type is simple and type to change to is simple it will succeed in MVT. That seems like a poor implementation. Maybe it should check the result and fallback to the extended implementation of it fails on MVT?

Yeah that's.. questionable. I'd +1 that change.

This change, though, LGTM.

Grrr. We can't make this change. The extended implementation relies on there being an LLVMTy to get context from. I think there's a bug in EVT::changeVectorElementType if the vector type is simple, but the element type is not. We'll fall into extended implementation, but LLVMTy is null.

llvm/lib/Target/X86/X86ISelLowering.cpp

EVT X86TargetLowering::getSetCCResultType(const DataLayout &DL,
   if (Subtarget.hasAVX512()) {
     // Figure out what this type will be legalized to.
     EVT LegalVT = VT;
     while (getTypeAction(Context, LegalVT) != TypeLegal)
       LegalVT = getTypeToTransformTo(Context, LegalVT);
 
     // If we got a 512-bit vector then we'll definitely have a vXi1 compare.
     if (LegalVT.getSimpleVT().is512BitVector())
-      return VT.changeVectorElementType(MVT::i1);
+      return EVT::getVectorVT(Context, MVT::i1, VT.getVectorElementCount());
 
     if (LegalVT.getSimpleVT().isVector() && Subtarget.hasVLX()) {
       // If we legalized to less than a 512-bit vector, then we will use a vXi1
       // compare for vXi32/vXi64 for sure. If we have BWI we will also support
       // vXi16/vXi8.
       MVT EltVT = LegalVT.getSimpleVT().getVectorElementType();
       if (Subtarget.hasBWI() || EltVT.getSizeInBits() >= 32)
-        return VT.changeVectorElementType(MVT::i1);
+        return EVT::getVectorVT(Context, MVT::i1, VT.getVectorElementCount());
     }
   }
 
   return VT.changeVectorElementTypeToInteger();
 }
 
 /// Helper for getByValTypeAlignment to determine
 /// the desired ByVal argument alignment.

llvm/test/CodeGen/X86/vec3-setcc-crash.ll

+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=i686-unknown-unknown -mattr=+avx512vl | FileCheck %s --check-prefix=X86
+; RUN: llc < %s -mtriple=x86_64-unknown-unknown -mattr=+avx512vl | FileCheck %s --check-prefix=X64
+
+define void @vec3_setcc_crash(<3 x i32>* %in, <3 x i32>* %out) {
+; X86-LABEL: vec3_setcc_crash:
+; X86:       # %bb.0:
+; X86-NEXT:    movl {{[0-9]+}}(%esp), %eax
+; X86-NEXT:    movl {{[0-9]+}}(%esp), %ecx
+; X86-NEXT:    vmovdqa (%ecx), %xmm0
+; X86-NEXT:    vptestnmd %xmm0, %xmm0, %k1
+; X86-NEXT:    vmovdqa32 %xmm0, %xmm0 {%k1} {z}
+; X86-NEXT:    vpextrd $2, %xmm0, 8(%eax)
+; X86-NEXT:    vpextrd $1, %xmm0, 4(%eax)
+; X86-NEXT:    vmovd %xmm0, (%eax)
+; X86-NEXT:    retl
+;
+; X64-LABEL: vec3_setcc_crash:
+; X64:       # %bb.0:
+; X64-NEXT:    vmovdqa (%rdi), %xmm0
+; X64-NEXT:    vptestnmd %xmm0, %xmm0, %k1
+; X64-NEXT:    vmovdqa32 %xmm0, %xmm0 {%k1} {z}
+; X64-NEXT:    vpextrd $2, %xmm0, 8(%rsi)
+; X64-NEXT:    vmovq %xmm0, (%rsi)
+; X64-NEXT:    retq
+  %a = load <3 x i32>, <3 x i32>* %in
+  %cmp = icmp eq <3 x i32> %a, zeroinitializer
+  %c = select <3 x i1> %cmp, <3 x i32> %a, <3 x i32> zeroinitializer
+  store <3 x i32> %c, <3 x i32>* %out
+  ret void
+}