[PATCH] D38756: [x86] use an insert op to put one variable element into a constant of vectors

[Simon Pilgrim via Phabricator via llvm-commits]

RKSimon added inline comments.


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:7682
+    // variable scalar element and its index for use in the insertelement.
+    SmallVector<Constant *, 16> ConstVecOps(NumElems);
+    SDValue VarElt;
----------------
Worth splatting this with UndefValue::get(EltType) ?


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:7685
+    SDValue InsIndex;
+    LLVMContext *Context = DAG.getContext();
+    Type *EltType = Op.getValueType().getScalarType().getTypeForEVT(*Context);
----------------
We have other uses of DAG.GetContext - maybe worth pulling this out?


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:7708
+    // a new build vector here. If the build vector contains illegal constants,
+    // it could get split back up into a series of insert elements.
+    SDValue LegalDAGConstVec = LowerConstantPool(DAGConstVec, DAG);
----------------
This is causing a lot of shorter loads (movsd etc.) to become full width loads - either add a TODO or create a helper with a TODO that we can use to create shorter loads (broadcast/VZEXT_LOAD etc) with a future commit.


https://reviews.llvm.org/D38756