[PATCH] D41096: [X86] Initial support for prefer-vector-width function attribute

[Hal Finkel via Phabricator via llvm-commits]

hfinkel added a comment.

In https://reviews.llvm.org/D41096#954642, @craig.topper wrote:

> Normally I would prefer additive features too.
>
> For the prefer-avx256 feature in this patch, I need the preference to only apply with -march=skylake-avx512/native and not with -mavx512f or with -march=knl. So I think that means it needs to be set in the default features in skylake-avx512 cpu definition. And the prefer-vector-width attribute needs to remove it if the user specifies a higher preference
>
> For the next patch that will deal with legalization. I think i still have to do something weird because I can't see the features implied by the CPU name string in getSubtargetImpl. So I can't just blindly enable a feature flag for 512-bit register support there. So I need to add something like "+requires512bitvectors" into the feature string based on the attribute, but the lack of a "required-vector-width" attribute implies we don't know for sure. So maybe its better to have "+no512bitvectors" if the attribute is present and set to value 256 or less. Then in X86ISelLowering we would enable 512 bit types with "hasAVX512 && !(no512bitvectors && prefer-avx256)"
>
> Do you see any better way?


I would add a target feature, like:

  def FeatureAVX512WideVectors : SubtargetFeature<"avx512-wide-vectors", "HasAVX512WideVectors", "true", "Enable use of 512-bit Vector Registers", [FeatureAVX512]>;

Then I'd add this feature, separately, to the relevant CPU definitions (KNLFeatures and SKXFeatures).

Then, in getSubtargetImpl, if the CPU name is skylake-avx512 or skx, then look at the required-vector-width attribute. If it is present, and is <= 256, then add "-avx512-wide-vectors" to the feature string.

I think gives us what we want with only additive features.

> and not with -mavx512f

Do you mean that you want -march=skylake -mavx512f should effectively set a preference for 512-bit vectors? If so, I think we can handle that in the frontend (by just setting the preference there appropriately).


https://reviews.llvm.org/D41096