Combine performance regression (was: Fix PR target/18701)

Roger Sayle roger@eyesopen.com
Thu Jan 20 22:23:00 GMT 2005


On Thu, 20 Jan 2005, Hans-Peter Nilsson wrote:
> > I made was to believe that the comment above this hunk actually
> > described it's behaviour; "Don't change the mode of the MEM..."
>
> It doesn't?

Look at the code that immediately follows this clause.  It doesn't
do anything, it just returns.  No where in combine_simplify_rtx for
paradoxical SUBREGs do we create a MEM with a different mode from
the one it originally had.

You patch doesn't prevent us falling through to a potentially
unsafe transformation, instead it prevents combine from ever
creating a paradoxical subreg of a MEM.

> No, we've been through that already.  The reason is that "widen
> the mode used to access the MEM of a paradoxical subreg" is a
> tautology.  And it's not stopping paradoxical subregs
> everywhere, just from being simplified into a widened access
> *here*.

No.

The RTL "subreg:SI (mem:QI foo))" performs a QImode read from memory.
The RTL "(mem:SI foo)" performs an SImode read from memory.  The mode
used to access memory is the mode attached to the MEM.  Placing a
SUBREG around a MEM access does not access memory outside of the
object refered to by the MEM.


The comment you added with your patch was:

      /* Don't change the mode of the MEM if that would change the meaning
         of the address.  Similarly, don't allow widening, as that may
         access memory outside the defined object or using an address
         that is invalid for a wider mode.  */

but a "SUBREG:outer (MEM:inner ...)" has absolutely no affect on which
memory locations are accessed and in which mode unless there's a serious
bug in your backend.  In PR 18701, you showed an example of MEM:mode1
being changed to MEM:mode2, and this should obviously be prevented which
is what I thought your patch did.  As it turns out, your forbidding
the creation of (SUBREG:mode2 (MEM:mode1 ...)) which is perfectly
allowable.

As for claiming that you patch doesn't disallow the creation of
paradoxical subregs of MEMs thoughout combine, Ulrich's performance
regression provides an obvious counter example.

> Can we go back to the RTL in my original analysis and say why it
> would be ok to widen the access?

Placing a SUBREG around a MEM doesn't widden it.  Changing the
mode of the MEM does.  You've diagnosed the problem correctly, but
you've not correctly identified/disabled the errant transformation
that was causing it.  In fact, I think the other fix for PR18701
that you committed may have been the true cause and the correct fix.


Roger
--



More information about the Gcc-patches mailing list