[PATCH] slp tree vectorizer: Re-calculate vectorization factor in the case of invalid choices [PR96974]

[Richard Biener]

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.

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.
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)