[PATCH] Another fix for decide_alg (PR target/70062)

[Uros Bizjak]

On Fri, Mar 4, 2016 at 11:34 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Fri, Mar 04, 2016 at 11:16:44AM +0100, Uros Bizjak wrote:
>> On Fri, Mar 4, 2016 at 11:06 AM, Jakub Jelinek <jakub@redhat.com> wrote:
>> > On Fri, Mar 04, 2016 at 10:59:48AM +0100, Uros Bizjak wrote:
>> >> I don't like the fact that *dynamic_check is set to max (which is 0
>> >> with your testcase) when recursion avoidance code already set it to
>> >> "something reasonable", together with loop_1_byte alg. What do you
>> >> think about attached (lightly tested) patch?
>> >
>> > But that can still set *dynamic_check to 0 if the recursive call has
>> > not set *dynamic_check.
>> > So, perhaps we want *dynamic_check = max ? max : 128;
>> > ?
>>
>> I believe that recursive call set *dynamic_check to zero for a reason.
>> The sent patch deals with recursion breaking issues only, leaving all
>> other functionality as it was before. So, your issue is IMO orthogonal
>> to the PR70062 and should be fixed (if at all) in a separate patch.
>
> The recursive call should never set *dynamic_check to 0, only to
> -1 or 128 (the last one newly, since my fix).
> And, my issue is IMO not orghogonal to that, either *dynamic_check == 0
> is ok, or it is not.
> Perhaps better would be to add an extra argument to decide_alg, false
> for toplevel invocation, true for recursive invocation, and if recursive
> call, just never try to recurse again (thus we could revert the previous
> change) and don't set *dynamic_check to anything in that case during the
> recursion.
> And then at the caller side decide what we want for max == -1 and what
> for max == 0 with *dynamic_check.

I think that would work, and considering that we have only one
non-recursive callsite of decide_alg, it looks like the cleanest
solution.

Uros.