Richard Biener <rguent...@suse.de> writes:
> On Wed, 12 Jan 2022, Richard Sandiford wrote:
>
>> Richard Biener <rguent...@suse.de> writes:
>> > On Wed, 12 Jan 2022, Andre Vieira (lists) wrote:
>> >
>> >> Hi,
>> >>
>> >> This a fix for the regression caused by '[vect] Re-analyze all modes for
>> >> epilogues'. The earlier patch assumed there was always at least one other
>> >> mode
>> >> than VOIDmode, but that does not need to be the case.
>> >> If we are dealing with a target that does not define more modes for
>> >> 'autovectorize_vector_modes', the behaviour before the patch would be to
>> >> try
>> >> to create an epilogue for the same autodetected_vector_mode, which unless
>> >> the
>> >> target supported partial vectors would always fail. So as a fix I suggest
>> >> trying to vectorize the epilogue with the preferred_simd_mode for QI,
>> >> mimicking autovectorize_vector_mode, which will be skipped if it is not a
>> >> vector_mode (since that already should indicate partial vectors aren't
>> >> possible) or if no partial vectors are supported and its pessimistic
>> >> NUNITS is
>> >> larger than the main loop's VF.
>> >>
>> >> Currently bootstrapping and regression testing, otherwise OK for trunk?
>> >> Can
>> >> someone verify this fixes the issue for PR103971 on powerpc?
>> >
>> > Why not simply start at mode_i = 0 which means autodetecting the mode
>> > to use for the epilogue? That appears to be a much simpler solution to
>> > me, including for targets where there are more than one element in the
>> > vector.
>>
>> VOIDmode doesn't tell us anything about what the autodetected mode
>> will be, so current short-circuit:
>>
>> /* If the target does not support partial vectors we can shorten the
>> number of modes to analyze for the epilogue as we know we can't pick a
>> mode that has at least as many NUNITS as the main loop's vectorization
>> factor, since that would imply the epilogue's vectorization factor
>> would be at least as high as the main loop's and we would be
>> vectorizing for more scalar iterations than there would be left. */
>> if (!supports_partial_vectors
>> && maybe_ge (GET_MODE_NUNITS (vector_modes[mode_i]), first_vinfo_vf))
>> {
>> mode_i++;
>> if (mode_i == vector_modes.length ())
>> break;
>> continue;
>> }
>>
>> wouldn't be effective.
>
> Well, before this change we simply did
>
> - /* Handle the case that the original loop can use partial
> - vectorization, but want to only adopt it for the epilogue.
> - The retry should be in the same mode as original. */
> - if (LOOP_VINFO_EPIL_USING_PARTIAL_VECTORS_P (first_loop_vinfo))
> ...
> - else
> - {
> - mode_i = first_loop_next_i;
> - if (mode_i == vector_modes.length ())
> - return first_loop_vinfo;
> - }
>
> and thus didn't bother with epilogue vectorization. I think we should
> then just restore this behavior, not doing epilogue vectorization
> if vector_modes.length () == 1?
Yeah, but that case didn't need epilogue vectorisation before. This
series is adding support for unrolling, and targets with a single vector
size will benefit from epilogues in that case.
Thanks,
Richard
