[PATCH] D30176: [X86][SSE] Attempt to extract vector elements through target shuffles

[Simon Pilgrim via Phabricator via llvm-commits]

RKSimon added inline comments.


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:28922
+      ((SrcIdx == 0 && Subtarget.hasSSE2()) || Subtarget.hasSSE41())) {
+    assert(SrcSVT == VT && "Unexpected extraction type");
+    return DAG.getNode(ISD::EXTRACT_VECTOR_ELT, dl, SrcSVT, SrcOp,
----------------
filcab wrote:
> Seems to me this would still be valid with different types (extract_element with a different type), but we can't do anything in that case. If so, shouldn't we `return SDValue();` instead of asserting?
It shouldn't happen as we test for after legalization at the start of the function, but I can replace it with an early out.


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:34441
+  case X86ISD::PEXTRB:
+    return combineExtractVectorElt_SSE(N, DAG, DCI, Subtarget);
   case ISD::INSERT_SUBVECTOR:
----------------
filcab wrote:
> Is this an optimization or a correctness thing? Why can't you simply call `combineExtractVectorElt` since it will try that combine anyway?
combineExtractVectorElt has a load of other combines inside it (notably load extraction) that can't handle PEXTRW/PEXTRB and their implicit zeroing of the upper bits of their i32 result. I'll make this clearer in the TODO at the top of combineExtractVectorElt_SSE


Repository:
  rL LLVM

https://reviews.llvm.org/D30176