(Resending; this bounced) On Sat, Jan 06, 2018 at 07:39:46PM +0000, Richard Sandiford wrote: > James Greenhalgh <james.greenha...@arm.com> writes: > > On Fri, Nov 24, 2017 at 03:59:58PM +0000, Richard Sandiford wrote: > >> Richard Sandiford <richard.sandif...@linaro.org> writes: > >> > This series adds support for ARM's Scalable Vector Extension. > >> > More details on SVE can be found here: > >> > > >> > > >> > https://developer.arm.com/products/architecture/a-profile/docs/arm-architecture-reference-manual-supplement-armv8-a > >> > > >> > There are four parts for ease of review, but it probably makes > >> > sense to commit them as one patch. > >> > > >> > The series plugs SVE into the current vectorisation framework without > >> > adding any new features to the framework itself. This means for example > >> > that vector loops still handle full vectors, with a scalar epilogue loop > >> > being needed for the rest. Later patches add support for other features > >> > like fully-predicated loops. > >> > > >> > The patches build on top of the various series that I've already posted. > >> > Sorry that there were so many, and thanks again for all the reviews. > >> > > >> > Tested on aarch64-linux-gnu without SVE and aarch64-linux-gnu with SVE > >> > (in the default vector-length agnostic mode). Also tested with > >> > -msve-vector-bits=256 and -msve-vector-bits=512 to select 256-bit > >> > and 512-bit SVE registers. > >> > >> Here's an update based on an off-list discussion with the maintainers. > >> Changes since v1: > >> > >> - Changed the names of the modes from 256-bit vectors to "VNx" > >> + a 128-bit mode name, e.g. V32QI -> VNx16QI. > >> > >> - Added an "sve" attribute and used it in the "enabled" attribute. > >> This allows generic aarch64.md patterns to disable things related > >> to SVE on non-SVE targets; previously this was implicit through the > >> constraints. > >> > >> - Improved the consistency of the constraint names, specifically: > >> > >> Ua?: addition contraints (already used for Uaa) > >> Us?: general scalar constraints (already used for various other scalars) > >> Ut?: memory constraints (unchanged from v1) > >> vs?: vector SVE constraints (mostly unchanged, but now includes FP > >> as well as integer constraints) > >> > >> There's still the general "Dm" (minus one) constraint, for consistency > >> with "Dz" (zero). > >> > >> - Added missing register descriptions above FIXED_REGISTERS. > >> > >> - "should"/"is expected to" -> "must". > >> > >> - Added more commentary to things like regmode_natural_size. > >> > >> I also did a before and after comparison of the testsuite output > >> for base AArch64 (but using the new FIRST_PSEUDO_REGISTER definition > >> to avoid changes to hash values). There were no differences. > > > > I seem to have lost 4/4 in my mailer. Would you mind pinging it if I have > > any action to take? Also, please ping any other SVE parts I've missed that > > you haven't pinged in recent days. > > 4/4 was the unwinder support, which you've already reviewed (thanks): > > https://gcc.gnu.org/ml/gcc-patches/2017-11/msg00251.html > > There are two other AArch64 patches that I'll ping in a sec.
These, and 1/4 will now have to wait for later this week I'm afraid, I hope I'll have a chance by midweek. I'll also see how far I can get with the Armv8.4-A review this week to avoid further sequencing issues with who goes first. Thanks for the preemptive rebase. > There are also quite a few patches that add target-independent > support for something and also add corresponding SVE code > to config/aarch64 and/or code quality tests to gcc.target/aarch64. > I think the full list of those is: > > Patches with config/aarch64 code: > > https://gcc.gnu.org/ml/gcc-patches/2017-11/msg02066.html > https://gcc.gnu.org/ml/gcc-patches/2017-11/msg02068.html > https://gcc.gnu.org/ml/gcc-patches/2017-11/msg01484.html > https://gcc.gnu.org/ml/gcc-patches/2017-11/msg01485.html > https://gcc.gnu.org/ml/gcc-patches/2017-11/msg01491.html > https://gcc.gnu.org/ml/gcc-patches/2017-11/msg01494.html > https://gcc.gnu.org/ml/gcc-patches/2017-11/msg01497.html > https://gcc.gnu.org/ml/gcc-patches/2017-11/msg01506.html > https://gcc.gnu.org/ml/gcc-patches/2017-11/msg01570.html > https://gcc.gnu.org/ml/gcc-patches/2017-11/msg01575.html I think I've put an OK on most of these. The review was overall straightforward - sorry for missing the action left on me earlier. > Patches with gcc.target/aarch64 tests but no config/aarch64 changes, > with the tests being in the spirit of the ones added in the original > SVE patch: > > https://gcc.gnu.org/ml/gcc-patches/2017-11/msg00752.html > https://gcc.gnu.org/ml/gcc-patches/2017-11/msg01446.html > https://gcc.gnu.org/ml/gcc-patches/2017-11/msg01489.html > https://gcc.gnu.org/ml/gcc-patches/2017-11/msg01490.html > https://gcc.gnu.org/ml/gcc-patches/2017-11/msg01498.html > https://gcc.gnu.org/ml/gcc-patches/2017-11/msg01499.html > https://gcc.gnu.org/ml/gcc-patches/2017-11/msg01572.html > https://gcc.gnu.org/ml/gcc-patches/2017-11/msg01573.html > https://gcc.gnu.org/ml/gcc-patches/2017-11/msg01577.html > > The target-independent pieces have already been reviewed (except where > I'll ping seperately). And these are also OK. With two comments on the overall strategy - as I've mentioned elsewhere I find the -march=armv8-a+sve to be too restrictive for our testing efforts. I'd prefer to just add SVE by a target pragma if we can. Additionally, I'd be happy with the whole AArch64 testsuite being organised in to folders (doing this might also make it easier for us to turn all SVE tests off with older assemblers, by using the skipping the exp file if we can't find support). For now, I've OK'd the tests. They would still be OK in my mind with either or both of my suggestions above. Thanks, James
