Re: [PATCH] doc: update looping constructs

[Paul Koning <paulkoning at comcast dot net>]

> On Jul 12, 2018, at 1:42 PM, Jeff Law <law@redhat.com> wrote:
> 
> On 07/12/2018 11:17 AM, Paul Koning wrote:
>> 
>> 
>>> On Jul 12, 2018, at 12:55 PM, Jeff Law <law@redhat.com> wrote:
>>> 
>>> On 07/11/2018 06:20 PM, Paul Koning wrote:
>>>> This patch removes the obsolete documentation for
>>>> decrement_and_branch_until_zero.  It also adds detail to the
>>>> description for doloop_end.  In particular, it describes the
>>>> required form of the conditional branch part of the pattern.
>>>> 
>>>> Ok for trunk?
>>>> 
>>>> paul
>>>> 
>>>> ChangeLog:
>>>> 
>>>> 2018-07-11  Paul Koning  <ni1d@arrl.net>
>>>> 
>>>> * doc/rtl.texi (REG_NONNEG): Remove decrement and branch until 
>>>> zero reference, add doloop_end instead. * doc/md.texi
>>>> (decrement_and_branch_until_zero): Remove. (Looping patterns):
>>>> Remove decrement_and_branch_until_zero.  Add detail for
>>>> doloop_end.
>>> OK.  I wonder if we can eliminate REG_NONNEG at this point.  I'm
>>> not sure if it's really being used anymore.  I see a reference in
>>> the old m68k dbra pattern, but that pattern could well be dead at
>>> this point.
>> 
>> The original reasoning is that you'd need the note if you have a
>> machine instruction that does a loop until negative, and you want to
>> confirm that the input is a valid loop count (not negative --
>> otherwise you'd loop once rather than zero times).  If you have a
>> loop instruction that works this way, that still matters.  If someone
>> wants to convert m68000 to use doloop_end, for example.  Or likewise
>> for vax, which has an unnamed looping pattern that could be used but
>> obviously isn't at the moment (since it's unnamed).
> Right.  And that is now dbra works on the m68k.  However I've got little
> confidence we're generating that instruction anymore.  Though I guess
> it's easy enough to check since I can bootstrap m68k with qemu :-)
> 
> It's always struck me as lame that we had to rely on the magic note and
> in the world of doloop.c we ought to be able to express things much more
> clearly.

Agreed.

I'm pretty sure dbra is no longer working.  It certainly didn't do anything when I tried to add it in pdp11, and when I asked about it on the gcc list I was told that it was removed a decade ago, or thereabouts.

The new technique works fine.  The only drawback is the "register can't be the induction variable" limitation, which makes sense if special registers are used but would be nice not to have on machines where the register used is a general register.

>> Then again... does the code that generates this insn do the checking,
>> i.e., avoid generating doloop_end unless it can confirm that the
>> register is nonnegative?  It doesn't seem to, at least not the code
>> in "doloop_modify" that actually inserts the regnote.  But how can
>> that be valid?  doloop_end by definition loops at least once.  If you
>> pass it a negative count, the loop is run once for "ge" comparisons,
>> and MAXUINT + count times for insns that use the "ne" condition
>> (i.e., all the current ones).  So one would assume that's not
>> possible, i.e., you couldn't come through there unless REG_NONNEG is
>> guaranteed true.
>> 
>> Currently the only doloop_end insns I can find all have the "ne"
>> comparison so the regnote isn't generated.  I'd like to leave this
>> question separate from the doc cleanup.  If we want to remove the
>> note, that's easy enough, and the doc change would be pretty obvious
>> as well.
>> 
>> So is this patch OK to commit?
> SOrry I wasn't clear.  I'm perfectly content to look at killing
> REG_NONNEG independent of the doc cleanup.
> 
> The doc patch is fine to commit now.

Thanks, done.

	paul