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

Chandler Carruth chandlerc at gmail.com
Mon Jan 12 09:24:03 PST 2015

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;
ab wrote:
> 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.
We're so inconsistent here that I don't really care about what the old code happened to name things provided we're consistent going forward and converge on that. Given that, I'd prefer the minimal amount of abbreviation that is sane, and I think spelling out "load" but abbreviating "ext" is a fine compromise there.

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.
ab wrote:
> 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.)
Please don't add duplicated code and then factor it afterward.

Either do the minimal amount of factoring in this change so that we don't have to review 2x the code, or do the factoring first, and then the functional change.



More information about the llvm-commits mailing list