On 06/11/2023 11:24, Richard Sandiford wrote:
Stamatis Markianos-Wright <stam.markianos-wri...@arm.com> writes:
One of the main reasons for reading the arm bits was to try to answer
the question: if we switch to a downcounting loop with a GE condition,
how do we make sure that the start value is not a large unsigned
number that is interpreted as negative by GE? E.g. if the loop
originally counted up in steps of N and used an LTU condition,
it could stop at a value in the range [INT_MAX + 1, UINT_MAX].
But the loop might never iterate if we start counting down from
most values in that range.
Does the patch handle that?
So AFAICT this is actually handled in the generic code in `doloop_valid_p`:
This kind of loops fail because of they are "desc->infinite", then no
loop-doloop conversion is attempted at all (even for standard dls/le loops)
Thanks to that check I haven't been able to trigger anything like the
behaviour you describe, do you think the doloop_valid_p checks are
robust enough?
The loops I was thinking of are provably not infinite though. E.g.:
for (unsigned int i = 0; i < UINT_MAX - 100; ++i)
...
is known to terminate. And doloop conversion is safe with the normal
count-down-by-1 approach, so I don't think current code would need
to reject it. I.e. a conversion to:
unsigned int i = UINT_MAX - 101;
do
...
while (--i != ~0U);
would be safe, but a conversion to:
int i = UINT_MAX - 101;
do
...
while ((i -= step, i > 0));
wouldn't, because the loop body would only be executed once.
I'm only going off the name "infinite" though :) It's possible that
it has more connotations than that.
Thanks,
Richard
Ack, yep, I see what you mean now, and yep, that kind of loop does
indeed pass through doloop_valid_p
Interestingly , in the v8-M Arm ARM this is done with:
```
boolean IsLastLowOverheadLoop(INSTR_EXEC_STATE_Type state)
// This does not check whether a loop is currently active.
// If the PE were in a loop, would this be the last one?
return UInt(state.LoopCount) <= (1 << (4 - LTPSIZE));
```
So architecturally the asm we output would be ok (except maybe the
"branch too far subs;bgt;lctp" fallback at
`predicated_doloop_end_internal` (maybe that should be `bhi`))... But
now GE: isn't looking like an accurate representation of this operation
in the compiler.
I'm wondering if I should try to make `predicated_doloop_end_internal`
contain a comparison along the lines of:
(gtu: (plus: (LR) (const_int -num_lanes)) (const_int num_lanes_minus_1))
I'll give that a try :)
The only reason I'd chosen to go with GE earlier, tbh, was because of
the existing handling of GE in loop-doloop.cc
Let me know if any other ideas come to your mind!
Cheers,
Stam
