[PATCH] D32273: [X86][AVX512] Make i1 illegal in the CodeGen

[Guy Blank via Phabricator via llvm-commits]

guyblank added inline comments.


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:6990
       SplatIdx = idx;
-    else if (In != Op.getOperand(SplatIdx))
+    else if (IsSplat && In != Op.getOperand(SplatIdx))
       IsSplat = false;
----------------
craig.topper wrote:
> Why did this if statement need to change?
just leftovers from other changes i had here which weren't needed.


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:14037
 
-  if (Op.getSimpleValueType() == MVT::i1)
+  if (VecVT.getVectorElementType() == MVT::i1)
     return ExtractBitFromMaskVector(Op, DAG);
----------------
craig.topper wrote:
> What's the different between this and the original line? Doesn't Op's VT have to match the elements of the vector?
since i1 is illegal, Op VT would be i8 when extracting from i1 vectors.

it is ok for the return type of extract vector element to be wider than the vector elements. 


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:29618
+    SDValue CondNew =
+        DAG.getNode(ISD::XOR, DL, Cond.getValueType(), Cond,
+                    DAG.getConstant(APInt::getAllOnesValue(8), DL, CondVT));
----------------
craig.topper wrote:
> Can we reuse CondVT here instead of calling Cond.getValueType() again?
> 
> Why cant' we use DAG.getAllOnesConstant anymore? Does the constant have different with than CondVT's element type now? What does that do?
getAllOnesConstant probably just got lost in a merge. there is no issue with it


https://reviews.llvm.org/D32273