This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


On Thu, 20 Jan 2005, Ulrich Weigand wrote:
> A crucial optimization for the bzip2 case is thus to combine a
> mem-to-reg load into such an AND and form a mem-to-reg zero-extend
> pattern from it.  However, the AND in question necessarily uses a
> paradoxical subreg, so we have this situation:

No, I don't think it's necessary, or at least that the bugs
that make it seem necessary should be fixed.

> Combine would now substitute the (mem ...) for (reg 79) in
> insn 226, and further optimization would detect that the
> (and (subreg (mem))) pattern is actually equivalent to a
> zero-extend.

And that's my beef: it should have been expressed as a
zero_extend here too.

> Thus, I do not like this patch ;-)  In fact, I rather disagree
> with the reasoning leading up to it.

What reasoning?  Do you mean that it's ok to widen MEM accesses
or do you mean that paradoxical mem subregs are a valid method
to express sign- and zero extension?

> In short, a paradoxical subreg of MEM is not invalid RTL, it is
> at worst RTL that is not directly implementable on the target
> machine.

It's a way to describe something that is better expressed by a
zero_extend, *therefore* it is wrong.

> After combine has finished the whole simplifcation process it
> will re-recognize the insn.  If a target doesn't want to accept
> this particular construct, it should be rejected *here*, if it
> is still present, not earlier.

Well, the widening of a mem access must be stopped at the point
where the widening, the change to a real access happens; it
can't be done later, because then we no longer know that it's
widened. :-)

>  (Alternatively, it might be
> accepted, and later on cleaned up by reload -- there is code in
> reload to do so, have you investigated why it didn't fix your
> problem?)

This suggestion suggests that you did not look at the whole of
the analysis.  This was a secondary patch, for a bug discovered
through the *other* bug.  The whole point of *this* patch is to
stop a paradoxical mem subreg from being *widened*.  If the
s390x port relies on that happening, I suggest that's incorrect.

I don't see your argument why actually widening is ok.  Perhaps
I missed the forest for the trees?

I guess I have to revisit, hopefully this week-end.

brgds, H-P


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]