Re: Tweak BB analysis for dr_analyze_innermost

[Richard Sandiford <richard dot sandiford at linaro dot org>]

"Bin.Cheng" <amker.cheng@gmail.com> writes:
> On Wed, Jun 28, 2017 at 5:56 PM, Richard Sandiford
> <richard.sandiford@linaro.org> wrote:
>> "Bin.Cheng" <amker.cheng@gmail.com> writes:
>>> Question is what would happen if simple_iv succeeds with non-ZERO step
>>> when called with nest==NULL?  The patch skips simple_iv and forces
>>> ZERO step?
>>
>> Yeah, I mentioned that in the covering note:
>>
>>   The handling seemed strange though.  If the DR was part of a loop,
>>   we still tried to express the base and offset values as IVs, potentially
>>   giving a nonzero step.  If that failed for any reason, we'd revert to
>>   using the original base and offset, just as we would if we hadn't asked
>>   for an IV in the first place.
> Hmm, it says "If that failed for any reason, .... revert to using the
> original base and offset, just as we would if we hadn't asked for an
> IV in the first place", but question is what would happen if simple_iv
> succeeds with nest==NULL.  After change, the successful simple_iv is
> bypassed.  It's likely this case can't happen in reality.

I suspect we're really in violent agreement here :-) but the reason
I was surprised at the question was that you seemed to be treating
the nest==NULL && simple_iv/nonzero-step combination as a corner case
that hadn't really been addressed, whereas removing that combination is
the main point of the patch.  The reason for removing it isn't that it
can't happen (bb-slp-pr65935.c proves that it did, to detrimental effect).
The reason is that IMO it isn't useful.  I don't think it makes sense to
set the step to a nonzero value for "BB analysis".

E.g., to ask a few rhetorical questions: why should we do that only for
"simple" IVs when analysis will still succeed for complex IVs and leave
the step zero?  Why should the simple_iv case be restricted to constant
steps for nest==NULL, rather than allow steps that are invariant in the
containing loop, like nest!=NULL does?  Why should the IV analysis be
based on the immediately containing loop rather than some other loop,
if the caller has asked for BB analysis rather than analysis wrt a
particular loop?

The reason I went back to the revision that introduced this behavior
was to check whether it really did need the nest==NULL && simple_iv/
nonzero-step combination.  And AFAICT it didn't.  I think it was just
accidental.

> Also the patch is a simplification for me and I don't have any objection here.

Thanks,
Richard