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

[Ahmed Bougacha]

Ping!

-Ahmed

On Mon, Jan 12, 2015 at 9:24 AM, Chandler Carruth <chandlerc at gmail.com> wrote:
> ================
> 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.

Yep: LoadExt for TLI, and <something>ExtLoad for SelectionDAG.

> ================
> 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.

Fair enough, sorry. The factoring of other combines is unrelated and
not in the patch.

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