Richard Biener <rguent...@suse.de> writes: > On Wed, 24 Mar 2021, Stam Markianos-Wright wrote: > >> Hi all, >> >> This patch resolves bug: >> >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96974 >> >> This is achieved by forcing a re-calculation of *stmt_vectype_out if an >> incompatible combination of TYPE_VECTOR_SUBPARTS is detected, but with an >> extra introduced max_nunits ceiling. >> >> I am not 100% sure if this is the best way to go about fixing this, because >> this is my first look at the vectorizer and I lack knowledge of the wider >> context, so do let me know if you see a better way to do this! >> >> I have added the previously ICE-ing reproducer as a new test. >> >> This is compiled as "g++ -Ofast -march=armv8.2-a+sve -fdisable-tree-fre4" for >> GCC11 and "g++ -Ofast -march=armv8.2-a+sve" for GCC10. >> >> (the non-fdisable-tree-fre4 version has gone latent on GCC11) >> >> Bootstrapped and reg-tested on aarch64-linux-gnu. >> Also reg-tested on aarch64-none-elf. > > I don't think this is going to work well given uses will expect > a vector type that's consistent here. > > I think giving up is for the moment the best choice, thus replacing > the assert with vectorization failure. > > In the end we shouldn't require those nunits vectypes to be > separately computed - we compute the vector type of the defs > anyway and in case they're invariant the vectorizable_* function > either can deal with the type mix or not anyway.
I agree this area needs simplification, but I think the direction of travel should be to make the assert valid. I agree this is probably the pragmatic fix for GCC 11 and earlier though. Also, IMO it's a bug that we use OImode, CImode or XImode for plain vectors. We should only need to use them for LD[234] and ST[234] arrays. Thanks, Richard > > That said, the goal should be to simplify things here. > > Richard. > >> >> gcc/ChangeLog: >> >> * tree-vect-stmts.c (get_vectype_for_scalar_type): Add new >> parameter to core function and add new function overload. >> (vect_get_vector_types_for_stmt): Add re-calculation logic. >> >> gcc/testsuite/ChangeLog: >> >> * g++.target/aarch64/sve/pr96974.C: New test. >>
