This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PING][PATCH][REVISED] Fix PR middle-end/PR28690, modify swap_commutative_operands_p
- From: Peter Bergner <bergner at vnet dot ibm dot com>
- To: Jakub Jelinek <jakub at redhat dot com>
- Cc: Ian Lance Taylor <iant at google dot com>, "H. J. Lu" <hjl at lucon dot org>, Richard Guenther <richard dot guenther at gmail dot com>, Paolo Bonzini <bonzini at gnu dot org>, Dave Korn <dave dot korn at artimi dot com>, Rask Ingemann Lambertsen <rask at sygehus dot dk>, gcc-patches at gcc dot gnu dot org, Pat Haugen <pthaugen at us dot ibm dot com>
- Date: Mon, 09 Jul 2007 16:51:42 -0500
- Subject: Re: [PING][PATCH][REVISED] Fix PR middle-end/PR28690, modify swap_commutative_operands_p
- References: <20070621125523.GA12802@lucon.org> <84fc9c000706210616s4ae15ce9yf8767bdcdfcb5919@mail.gmail.com> <20070621141603.GA2063@lucon.org> <20070621141737.GA16120@lucon.org> <1182449655.6798.34.camel@otta> <20070626230126.GA14180@lucon.org> <m33b0ethxv.fsf@localhost.localdomain> <1183687236.5585.16.camel@otta> <m3abua3uzb.fsf@localhost.localdomain> <1183734467.5585.21.camel@otta> <20070709211357.GF7012@devserv.devel.redhat.com>
On Mon, 2007-07-09 at 17:13 -0400, Jakub Jelinek wrote:
> On Fri, Jul 06, 2007 at 10:07:47AM -0500, Peter Bergner wrote:
> > Great, thanks! Given it's the weekend and I won't be around much, I'm
> > going to wait until Monday before committing this just in case there's
> > any fallout on ports other than the tested {power6464,x86_64}-linux.
>
> A modified version of the above patch (conditionalized using a TARGET
> macro, so that it doesn't affect non-ppc or non-power6 tuning on ppc)
> on redhat/gcc-4_1-branch caused a regression, see
> http://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=247256
> for details.
It's a good thing I held off committing the patch then!
> While I haven't tried to reproduce this with your patch on the trunk,
> I think it is fairly likely it will hit the same problem, either on
> the same testcase or on some other one.
> When swap_commutative_operand_p (which was always false before when
> both arguments were REGs), but now prefers lower REGNOs first
> fights against simplify_plus_minus which on the other side prefers
> whatever was originally first in such cases, we can end up with
> endless recursion.
Your testcase doesn't fail using current mainline with the patch,
but I think you're right that it could. I'll apply both of your
patches on top of mine and do another bootstrap and regtesting.
Thanks for finding and fixing this!
Would it be a good idea to add a comment to both locations stating that
their swapping criteria is interdependent?
Peter