This is the mail archive of the
mailing list for the GCC project.
Re: Combine performance regression (was: Fix PR target/18701)
- From: Hans-Peter Nilsson <hp at bitrange dot com>
- To: Ulrich Weigand <uweigand at de dot ibm dot com>
- Cc: roger at eyesopen dot com, gcc-patches at gcc dot gnu dot org
- Date: Thu, 20 Jan 2005 17:13:41 -0500 (EST)
- Subject: Re: Combine performance regression (was: Fix PR target/18701)
- References: <200501201736.j0KHa4T1027939@53v30g15.boeblingen.de.ibm.com>
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
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
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
> (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
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.