[PATCH] D12149: [AArch64] Turn on by default interleaved access vectorization

[Renato Golin via llvm-commits]

rengolin added a comment.

In http://reviews.llvm.org/D12149#228259, @llvm-commits wrote:

> I’m not sure about this LG and have a number of questions:
>
> 1. Has the review of the actual interleave code been finished?


Yes, all stride / interleaved access for ARM and AArch64 have been reviewed and committed.

> 2. What is the compile-time impact?


AFAIK unnoticeable. The validation phase drops out pretty quickly when strides are not possible, as much as everything else.

> 3. Could you share detailed performance data on SPEC ref input (per benchmark) and perhaps some other suites you run on a regular basis?


That's easier said than done. SPEC and other benchmarks licenses are silly in that you never know how much shared is too much, until you pass that threshold.

But one thing is for sure, no one shares "detailed performance data". Ever.

In this specific case, Silviu hasn't shared any SPEC results simply because they have not changed with any statistical significance, and that's thoroughly expected, since there aren't many cases of stride vectorization opportunities in SPEC. There are, however, in other benchmarks, which they did run, and which they have seen improvements. (sorry, I can't say more than that).

It is in the interest of ARM to do as much benchmarking as possible and to be *very* accurate and responsible about it, including compile time, so I trust their investigation quality. That's why it looks good to me.

> 4. Could count the number of times interleave vectorization per benchmark and see if there is a correlation to the run-time data you measure?


LNT has some, SPEC has close to none, others rely heavily on it. To be honest, the numbers are pretty much what I expected.

> 5. Do you expect impact on other architectures (not just ARM, ARM64 etc.). Data?


This is just enabled for ARM and AArch64, so no other architecture will ever see this happening. It's up to other people to enable it and customise to their architecture, and certainly not for this patch.

Keep in mind that what Silviu is enabling here is a development version of the stride vectrorizer, so we can start tracking performance and fixing the corner cases. Release 3.7 is already branched and release 3.8 is a looong way away, so we'll have plenty of time to fix any issues that come up on ARM and AArch64.

All the other issues, including experimental testing of the features (by turning stride with a flag) has been done for weeks now, and all looks well. So, it's only natural to move from experimental to development stage, and keep a good number of months between development and production stages, when 3.8 branch out.

In the unlikely event that the stride vectorization is causing enough trouble that we can't fix for 3.8, we'll disable it again by default and release a stable product, but trunk will have it on by default so that people of all sides can find problems with it.

I hope that sheds some light on your doubts.

cheers,
--renato


http://reviews.llvm.org/D12149