* Re: regression tester mail...
[not found] ` <20020726124106.GJ4629@atrey.karlin.mff.cuni.cz>
@ 2002-07-26 8:29 ` Jan Hubicka
0 siblings, 0 replies; only message in thread
From: Jan Hubicka @ 2002-07-26 8:29 UTC (permalink / raw)
To: Jan Hubicka; +Cc: Geoff Keating, Eric Christopher, gcc-bugs, rth, gcc-patches
> >
> > mips-elf gcc.sum gcc.c-torture/execute/921029-1.c
> >
> > is this:
> >
> > +Sun Jul 21 00:54:54 CEST 2002 Jan Hubicka <jh@suse.cz>
> > +
> > + * gcse.c: Include cselib.h
> > + (constptop_register): Break out from ...
> > + (cprop_insn): ... here; kill basic_block argument.
> > + (do_local_cprop, local_cprop_pass): New functions.
> > + (one_cprop_pass): Call local_cprop_pass.
> > +
> Hi,
> this bug is not handled by the other three patches I've sent and
> exposses another latent problem, this time in cprop_jump.
>
> The problem is sequence like:
> (set (reg A) (xor (reg A) (reg B)))
> (set (pc) (if_then_else (eq (reg A) (0))) ... )
>
> Now the cprop_jump proceeds by substituing the first instruction into
> the conditional and it notices reg B to be 0. The result of
> simplification is (of course) the ellimination of xor and exactly same
> conditional as previously. Then it validates the replacement of the
> conditional for it's equivalent and is happy.
>
> This uncovers thinko - when the setcc instruction modifies it's operand,
> it is of course invalid to recompute it afterwards again and expect the
> same result. The attached patch fixes the problem and also adds extra
> check to avoid substituting value for the same and infinite loop.
>
> Checked to fix the mips problem and regtested/bootstrapped i386.
> In case this patch is OK, can you please apply it as well?
>
> Honza
>
> Fri Jul 26 14:34:49 CEST 2002 Jan Hubicka <jh@suse.cz>
> * gcse.c (cprop_jump): Avoid false positives; verify that the
> transformation is valid.
Uhm, there is another bug in the code. The copy propagation is doing
the work as for the jump. In the between of setcc and jump there may be
other instructions copying registers around, so we need to verify that
the substitution suggested by copy propagation is valid at the setcc
execution point as well.
Note that the rtx_equal_p test is now probably redundant. I can't think
of situation where simplification will result in exactly same expression
as previously and the other checks will pass (an don't have any testcase
of course), but it is probably better to be safe then sorry.
Restarting bootstrapping/regtesting with the updated patch.
Honza
Index: gcse.c
===================================================================
RCS file: /cvs/gcc/egcs/gcc/gcse.c,v
retrieving revision 1.215
diff -c -3 -p -r1.215 gcse.c
*** gcse.c 25 Jul 2002 18:57:30 -0000 1.215
--- gcse.c 26 Jul 2002 12:54:26 -0000
*************** cprop_jump (bb, setcc, jump, from, src)
*** 4093,4099 ****
/* First substitute in the INSN condition as the SET_SRC of the JUMP,
then substitute that given values in this expanded JUMP. */
! if (setcc != NULL)
{
rtx setcc_set = single_set (setcc);
new_set = simplify_replace_rtx (SET_SRC (set),
--- 4093,4101 ----
/* First substitute in the INSN condition as the SET_SRC of the JUMP,
then substitute that given values in this expanded JUMP. */
! if (setcc != NULL
! && !modified_between_p (from, setcc, jump)
! && !modified_between_p (src, setcc, jump))
{
rtx setcc_set = single_set (setcc);
new_set = simplify_replace_rtx (SET_SRC (set),
*************** cprop_jump (bb, setcc, jump, from, src)
*** 4107,4113 ****
/* If no simplification can be made, then try the next
register. */
! if (rtx_equal_p (new, new_set))
return 0;
/* If this is now a no-op delete it, otherwise this must be a valid insn. */
--- 4109,4115 ----
/* If no simplification can be made, then try the next
register. */
! if (rtx_equal_p (new, new_set) || rtx_equal_p (new, SET_SRC (set)))
return 0;
/* If this is now a no-op delete it, otherwise this must be a valid insn. */
*************** cprop_jump (bb, setcc, jump, from, src)
*** 4115,4120 ****
--- 4117,4127 ----
delete_insn (jump);
else
{
+ /* Ensure the value computed inside the jump insn to be equivalent
+ to one computed by setcc. */
+ if (setcc
+ && modified_in_p (new, setcc))
+ return 0;
if (! validate_change (jump, &SET_SRC (set), new, 0))
return 0;
^ permalink raw reply [flat|nested] only message in thread
only message in thread, other threads:[~2002-07-26 12:58 UTC | newest]
Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <1027407719.1276.8.camel@ghostwheel.cygnus.com>
[not found] ` <jmznwii8lc.fsf@desire.geoffk.org>
[not found] ` <20020726124106.GJ4629@atrey.karlin.mff.cuni.cz>
2002-07-26 8:29 ` regression tester mail Jan Hubicka
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).