Hi Eric
On 08/11/2019 19:16, Eric Botcazou wrote:
>> I was fiddling around with the loop unrolling pass and noticed a check
>> in decide_unroll_* functions (in the patch). The comment on top of this
>> check says
>> "/* If we were not asked to unroll this loop, just return back silently.
>> */"
>> However the check returns when loop->unroll == 0 rather than 1.
>>
>> The check was added in r255106 where the ChangeLog suggests that the
>> actual intention was probably to check the value 1 and not 0.
>
> No, this is intended, 0 is the default value of the field, not 1. And note
> that decide_unroll_constant_iterations, decide_unroll_runtime_iterations and
> decide_unroll_stupid *cannot* be called with loop->unroll == 1 because of this
> check in decide_unrolling:
Thanks for the explanation. However, I do not understand why are we
returning with the default value. The comment for "unroll" is a bit
ambiguous for value 0.
/* The number of times to unroll the loop. 0 means no information given,
just do what we always do. A value of 1 means do not unroll the loop.
A value of USHRT_MAX means unroll with no specific unrolling factor.
Other values means unroll with the given unrolling factor. */
unsigned short unroll;
What "do we always do"?
Thanks
Sudi
>
> if (loop->unroll == 1)
> {
> if (dump_file)
> fprintf (dump_file,
> ";; Not unrolling loop, user didn't want it unrolled\n");
> continue;
> }
>
>> Tested on aarch64-none-elf with one new regression:
>> FAIL: gcc.dg/pr40209.c (test for excess errors)
>> This fails because the changes cause the loop to unroll 3 times using
>> unroll_stupid and that shows up as excess error due -fopt-info. This
>> option was added in r202077 but I am not sure why this particular test
>> was chosen for it.
>
> That's a regression, there should be no unrolling.
>
