[PATCH] D42353: [Codegen] support of 'nof' flag lowering on X86 target

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: CodeGen/SelectionDAG/LegalizeVectorOps.cpp:755
+  if (!TLI.isOperationLegalOrCustom(ISD::FDIV, DstVT)) {
+    EVT HalfVT = EVT::getVectorVT(*DAG.getContext(), EltVT, NumElts / 2);
+    DstVT = EVT::getVectorVT(*DAG.getContext(), FloatVT, NumElts);
----------------
What guarantees HalfVT is a legal type for the target?


================
Comment at: CodeGen/X86/combine-sdiv.ll:265
 ;
-; AVX-LABEL: combine_vec_sdiv_by_pow2b:
-; AVX:       # %bb.0:
-; AVX-NEXT:    vpextrd $1, %xmm0, %eax
-; AVX-NEXT:    movl %eax, %ecx
-; AVX-NEXT:    sarl $31, %ecx
-; AVX-NEXT:    shrl $30, %ecx
-; AVX-NEXT:    addl %eax, %ecx
-; AVX-NEXT:    sarl $2, %ecx
-; AVX-NEXT:    vpinsrd $1, %ecx, %xmm0, %xmm1
-; AVX-NEXT:    vpextrd $2, %xmm0, %eax
-; AVX-NEXT:    movl %eax, %ecx
-; AVX-NEXT:    sarl $31, %ecx
-; AVX-NEXT:    shrl $29, %ecx
-; AVX-NEXT:    addl %eax, %ecx
-; AVX-NEXT:    sarl $3, %ecx
-; AVX-NEXT:    vpinsrd $2, %ecx, %xmm1, %xmm1
-; AVX-NEXT:    vpextrd $3, %xmm0, %eax
-; AVX-NEXT:    movl %eax, %ecx
-; AVX-NEXT:    sarl $31, %ecx
-; AVX-NEXT:    shrl $28, %ecx
-; AVX-NEXT:    addl %eax, %ecx
-; AVX-NEXT:    sarl $4, %ecx
-; AVX-NEXT:    vpinsrd $3, %ecx, %xmm1, %xmm0
-; AVX-NEXT:    retq
+; FIX IT: X86 support variable shift of vectors need to catch this
+;   in DAGCombine
----------------
FIXME and TODO are the common prefixes for comments like this. No one is ever going to look for "FIX IT"


================
Comment at: Target/X86/X86ISelLowering.cpp:38113
+  // we prefer this only in case of AVX2\AVX512 
+  return (Subtarget.hasAVX2() || Subtarget.hasAVX512());
+}    
----------------
hasAVX2 implies hasAVX512. But what's special about AVX2 here?


Repository:
  rL LLVM

https://reviews.llvm.org/D42353