Re: [PATCH] [RFC] vect: Fix infinite loop while determining peeling amount

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

Richard Biener <richard.guenther@gmail.com> writes:
> On Mon, Jul 27, 2020 at 11:09 AM Richard Sandiford
> <richard.sandiford@arm.com> wrote:
>>
>> Richard Biener via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
>> > On Wed, Jul 22, 2020 at 5:18 PM Stefan Schulze Frielinghaus via
>> > Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
>> >>
>> >> This is a follow up to commit 5c9669a0e6c respectively discussion
>> >> https://gcc.gnu.org/pipermail/gcc-patches/2020-June/549132.html
>> >>
>> >> In case that an alignment constraint is less than the size of a
>> >> corresponding scalar type, ensure that we advance at least by one
>> >> iteration.  For example, on s390x we have for a long double an alignment
>> >> constraint of 8 bytes whereas the size is 16 bytes.  Therefore,
>> >> TARGET_ALIGN / DR_SIZE equals zero resulting in an infinite loop which
>> >> can be reproduced by the following MWE:
>> >
>> > But we guard this case with vector_alignment_reachable_p, so we shouldn't
>> > have ended up here and the patch looks bogus.
>>
>> The above sounds like it ought to count as reachable alignment though.
>> If a type requires a lower alignment than its size, then that's even
>> more easily reachable than a type that requires the same alignment as
>> the size.  I guess at one extreme, a target alignment of 1 is always
>> reachable.
>
> Well, if the element alignment is 8 but its size is 16 then when presumably
> the desired vector alignment is a multiple of 16 we can never reach it.
> Isn't this the case here?

If the desired vector alignment (TARGET_ALIGN) is a multiple of 16 then
TARGET_ALIGN / DR_SIZE will be nonzero and the problem the patch is
fixing wouldn't occur.  I agree that we might never be able to reach
that alignment if the pointer starts out misaligned by 8 bytes.

But I think that's why it makes sense for the target to only ask
for 8-byte alignment for vectors too, if it can cope with it.  8-byte
alignment should always be achievable if the scalars are ABI-aligned.
And if the target does ask for only 8-byte alignment, TARGET_ALIGN /
DR_SIZE would be zero and the loop would never progress, which is the
problem that the patch is fixing.

It would even make sense for the target to ask for 1-byte alignment,
if the target doesn't care about alignment at all.

Thanks,
Richard