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

[Richard Biener <rguenther@suse.de>]

On Fri, 26 Mar 2021, Richard Sandiford wrote:

> Richard Biener <rguenther@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.

The issue is that we compute a vector type for a use that may differ
from what we'd compute for it in the context of its definition (or
in the context of another use).  Any such "local" decision is likely
flawed and I'd rather simplify further doing the only decision on
the definition side - if there's a disconnect between the number
of lanes (and thus altering the VF won't help) then we have to give
up anyway.

Richard.