[PATCH] [CodeGen] Add hooks/combine to form vector extloads, and enable it on X86.

[Ahmed Bougacha]

================
Comment at: include/llvm/Target/TargetLowering.h:1512
@@ +1511,3 @@
+  /// Return true if folding a load into ExtVal is profitable.
+  virtual bool isVectorExtLdDesirable(SDValue ExtVal) const {
+    return false;
----------------
chandlerc wrote:
> Do we you "ExtLd" in APIs elsewhere? this kind of extreme abbreviation, especially if deviating from other abbreviations, makes the APIs really hard to understand and find.
I agree, and originally wanted to use the closer-to-ubiquitous LoadExt. However, there's enableExtLdPromotion, which is the most similar API so should be similarly named.

I guess I could rename both to LoadExt instead, how does that sound?  That would stay consistent with ISD::LoadExtType, and the various *LoadExt* legality functions.

================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:5334-5335
@@ +5333,4 @@
+        SDLoc dl(N);
+        unsigned NumSplits =
+            DstVT.getVectorNumElements() / SplitDstVT.getVectorNumElements();
+        unsigned Stride = SplitSrcVT.getSizeInBits() / 8;
----------------
arsenm wrote:
> I don't think this will work correctly for 3 vectors, but it probably doesn't matter
You're right,  I added a check for isPow2VectorType.

================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:5660
@@ -5598,1 +5659,3 @@
 
+  // fold (zext (load x)) -> (zext (truncate (zextload x)))
+  // Only on illegal but splittable vectors.
----------------
chandlerc wrote:
> This code is extremely similar to the sext case. Can it be factored out with an appropriate predicate or input instruction tag or something?
It should be trivial to factor it out.  Other similar combines as well (I'll take care of it separately.)

================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:5694-5698
@@ +5693,7 @@
+        for (unsigned Idx = 0; Idx < NumSplits; Idx++) {
+          SDValue SplitLoad = DAG.getExtLoad(
+              ExtType, dl, SplitDstVT, LN0->getChain(), BasePTR,
+              LN0->getPointerInfo().getWithOffset(Idx * Stride), SplitSrcVT,
+              LN0->isVolatile(), LN0->isNonTemporal(), LN0->isInvariant(),
+              LN0->getAlignment(), LN0->getAAInfo());
+
----------------
arsenm wrote:
> It might be helpful to add a new version of getExtLoad based on the version that takes the MMO and an offset rather than having to pass all of the separate arguments
I thought so as well, but two things would make it confusing IMO: 1) the offset operand is for indexed loads only, which is a concept different from an ADDed base pointer. Also, less importantly, 2) we're not sure we can reuse the previous MMO, as the types may differ.
Or maybe you meant something else?

================
Comment at: test/CodeGen/X86/widen_load-2.ll:76-79
@@ -75,4 +75,6 @@
 ; CHECK-LABEL: add3i16:
-; CHECK:         pmovzxwd (%{{.*}}), %[[R0:xmm[0-9]+]]
-; CHECK-NEXT:    pmovzxwd (%{{.*}}), %[[R1:xmm[0-9]+]]
+; CHECK:         movq     (%rsi), %xmm0
+; CHECK-NEXT:    pmovzxwd %xmm0, %xmm0
+; CHECK-NEXT:    movq     (%rdx), %xmm1
+; CHECK-NEXT:    pmovzxwd %xmm1, %xmm1
 ; CHECK-NEXT:    paddd    %[[R0]], %[[R1]]
----------------
chandlerc wrote:
> This is both weird and unfortunate... Do understand why this happens?
This was actually a rebasing mistake, and doesn't happen on a clean apply of the patch. Sorry for the noise.

http://reviews.llvm.org/D6904

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/