[PATCH] D31965: [SLP] Enable 64-bit wide vectorization for Cyclone

[Adam Nemet via Phabricator via llvm-commits]

anemet added a comment.

In https://reviews.llvm.org/D31965#724841, @rengolin wrote:

> In https://reviews.llvm.org/D31965#724804, @anemet wrote:
>
> > Rolling it out for Cyclone-only is just a way to get this going in a controllable manner.  Other subtargets can roll it this out as people find the time to benchmark and tune this.
>
>
> Right. I'd like to at least try a more generic approach first, and only fall back to Cyclone-only if we get odd results on other cores.


Sure, this is a good compromise.

> ARM run the test-suite in benchmarking mode on AArch64, so they can pick up spikes if later on we add less generic code in generic section.
> 
> So, if the approach is generally good overall (I think it is), and we validate that on some cores, then we should let it in for all targets and monitor the performance as we go.
> 
> In that sense, @evandro, can you also have a look at the Exynos range? @mssimpso maybe check the Kyro line?
> 
> My rationale is that too many improvements are done by specific companies that are beneficial to the whole architecture, using the same reason "others can tune later if they wish". This encourages a culture of "works on my hardware", which makes the back-ends stiff to changes and proliferate decisions like relying on "isCyclone", or creation of target features that are only ever used for one target and other things that we have been cleaning up since last year.

Just as a positive counter example, when I added SW prefetch support last year, half of the ARM subtargets followed suit and added the knobs to enable it on their target.

Of course for that patch, we had to enable per subtarget since each needed to supply micro-architectural details.

> If we can get people from different sub-arches involved at an early stage, everybody wins.

Agreed!

Adam

> cheers,
> --renato




https://reviews.llvm.org/D31965