On Fri, Mar 4, 2016 at 11:34 AM, Jakub Jelinek <ja...@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 <ja...@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.
