[PATCH] D16729: [X86][SSE] Add general 32-bit LOAD + VZEXT_MOVL support to EltsFromConsecutiveLoads

[Elena Demikhovsky via llvm-commits]

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

LGTM


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:5651
@@ -5648,3 +5650,3 @@
     SDValue ResNode =
         DAG.getMemIntrinsicNode(X86ISD::VZEXT_LOAD, DL, Tys, Ops, MVT::i64,
                                 LDBase->getPointerInfo(),
----------------
What happens here in 32-bit mode, where i64 is illegal?

================
Comment at: lib/Target/X86/X86ISelLowering.cpp:5674
@@ +5673,3 @@
+  if (IsConsecutiveLoad && FirstLoadedElt == 0 && LoadSize == 32 &&
+      ((VT.is128BitVector() && TLI.isTypeLegal(MVT::v4f32)) ||
+       (VT.is256BitVector() && TLI.isTypeLegal(MVT::v8f32)) ||
----------------
I think that all these checks are not necessary. (VT.getSizeInBits() >= 128) should be enough.


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:5678
@@ +5677,3 @@
+    MVT VecVT = MVT::getVectorVT(MVT::f32, VT.getSizeInBits() / 32);
+    SDValue V = CreateLoad(MVT::f32, LDBase);
+    V = DAG.getNode(ISD::SCALAR_TO_VECTOR, DL, VecVT, V);
----------------
What happens if you don't put hardcoded MVT::f32, but choose between f32 and i32 according to VT ?

================
Comment at: lib/Target/X86/X86InstrAVX512.td:3054
@@ +3053,3 @@
+                   (v4i32 (scalar_to_vector (loadi32 addr:$src))), (iPTR 0)))),
+            (SUBREG_TO_REG (i32 0), (VMOVDI2PDIZrm addr:$src), sub_xmm)>;
+  def : Pat<(v16f32 (X86vzmovl (insert_subvector undef,
----------------
Why v8i64 is not handled in this patterns?


Repository:
  rL LLVM

http://reviews.llvm.org/D16729