Re: [PATCH] tree-optimization/102139 - fix SLP DR base alignment

[Richard Sandiford <richard.sandiford@arm.com>]

Richard Biener <rguenther@suse.de> writes:
> On Wed, 1 Sep 2021, Richard Sandiford wrote:
>
>> Richard Biener <richard.guenther@gmail.com> writes:
>> > On Tue, Aug 31, 2021 at 11:26 AM Richard Biener via Gcc-patches
>> > <gcc-patches@gcc.gnu.org> wrote:
>> >>
>> >> When doing whole-function SLP we have to make sure the recorded
>> >> base alignments we compute as the maximum alignment seen for a
>> >> base anywhere in the function is actually valid at the point
>> >> we want to make use of it.
>> 
>> Ah, yeah, the danger of optimisations that silently rely on the
>> then-current restrictions :-(
>
> Yeah.
>
>> >> To make this work we now record the stmt the alignment was derived
>> >> from in addition to the DRs innermost behavior and we use a
>> >> dominance check to verify the recorded info is valid when doing
>> >> BB vectorization.
>> >>
>> >> Note this leaves a small(?) hole for the case where we have sth
>> >> like
>> >>
>> >>     unaligned DR
>> >>     call (); // does not return
>> >>     aligned DR
>> >>
>> >> since we'll derive an aligned access for the earlier DR but the
>> >> later DR is never actually reached since the call does not
>> >> return.  To plug this hole one option (for the easy backporting)
>> >> would be to simply not use the base-alignment recording at all.
>> >> Alternatively we'd have to store the dataref grouping 'id' somewhere
>> >> in the DR itself and use that to handle this particular case.
>> >
>> > It turns out this isn't too difficult so the following is a patch adjusted
>> > to cover that case together with a testcase.
>> >
>> > Bootstrapped and tested on x86_64-unknown-linux-gnu.
>> >
>> > OK?
>> 
>> TBH I know nothing about this group id scheme, so I'm not really
>> in a position to comment.  But it LGTM from the (few) bits I do understand.
>> 
>> I guess we're leaving the same easter egg for loop optimisation if
>> we ever support early exits, but I'm not sure what to do about that.
>
> We're currently not recording alignment from masked DRs
> (aka DR_IS_CONDITIONAL_IN_STMT), I suppose we'd need to mark
> all stmts after early exits in this way then since in the end
> they will have to be masked on the early exit.
>
> So we _might_ be fine there ...

Yeah, for a pure SVE-like implementation that's probably true.  But we
also have the option of vectorising an early exit by branching if the
condition is true for *any* element, then handling the remaining
elements with an epilogue.

It would be quite a different approach from what we're doing now,
and wouldn't necessarily require up-front if-conversion.  But the
point is that it's theoretically possible, just as whole-function
SLP was theoretically possible (but seemingly some way off) when
this code was written :-)

Thanks,
Richard