This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: 6 GCC regressions, 2 new, with your patch on 2002-07-20T23:26:42Z.
- From: Jan Hubicka <jh at suse dot cz>
- To: Roger Sayle <roger at eyesopen dot com>, gcc-patches at gcc dot gnu dot org,rth at cygnus dot com
- Cc: Jan Hubicka <jh at suse dot cz>, gcc-regression at gcc dot gnu dot org
- Date: Sun, 21 Jul 2002 21:38:43 +0200
- Subject: Re: 6 GCC regressions, 2 new, with your patch on 2002-07-20T23:26:42Z.
- References: <200207210301.g6L31ih19173@maat.sfbay.redhat.com> <Pine.LNX.4.33.0207202158420.21322-100000@www.eyesopen.com>
>
> Hi Jan,
> > There are 2 new failures, and 4 failures that existed before and after
> > that patch; 0 failures have been fixed.
> >
> > The new failures are:
> > mips-elf gcc.sum gcc.c-torture/execute/921029-1.c
> > native gcc.sum gcc.dg/20010912-1.c
>
> These look like they're yours. I'm guessing its a bug in cselib or
> your new cprop code. In 20010912-1.c, the routine foo is miscompiled:
>
> >> int foo (int x, char *y)
> >> {
> >> int a;
> >> char *b = y;
> >> a = bar (x, &y);
> >> if (a)
> >> {
> >> y = b;
> ^
> This assignment is incorrectly removed. The call to "bar" changes
> the value of y, which isn't correctly noticed by cprop...
This is really interesting case. The beggining of function contains:
(insn 4 3 6 0 (nil) (set (reg:SI 62)
(mem/f:SI (plus:SI (reg/f:SI 16 argp)
(const_int 4 [0x4])) [4 y+0 S4 A32])) 45 {*movsi_1}
(nil)
(expr_list:REG_EQUIV (mem/f:SI (plus:SI (reg/f:SI 16 argp)
(const_int 4 [0x4])) [4 y+0 S4 A32])
(nil)))
And later also:
(insn 12 6 21 0 0x401a3a80 (set (reg/v/f:SI 64)
(mem/f:SI (plus:SI (reg/f:SI 16 argp)
(const_int 4 [0x4])) [4 S4 A8])) 45 {*movsi_1} (nil)
(nil))
cselib notices the equivalence and cprop actually does the replacement
of reg 64 by reg 62. But the REG_EQUIV note is not really valid, as the
value of memory copy y is not valid during the function (overwriten by
bar call). Just before register allocation the load of register 62
appears to be moved just before the assignment. I am not sure who does
the movement but it is valid given the information about equivalency
with memory.
cse.c seems to contain code preventing such a substitution as gcse does,
so I am attaching simple patch that does the same for cprop and I will
install it as obvious as I am not quite sure whether I will be able to
read my email tomorrow.
Sun Jul 21 21:35:20 CEST 2002 Jan Hubicka <jh@suse.cz>
* gcse.c (do_local_cprop): Do not extend lifetimes of registers set by
do_local_cprop.
Index: gcse.c
===================================================================
RCS file: /cvs/gcc/egcs/gcc/gcse.c,v
retrieving revision 1.210
diff -c -3 -p -r1.210 gcse.c
*** gcse.c 20 Jul 2002 22:56:05 -0000 1.210
--- gcse.c 21 Jul 2002 19:34:59 -0000
*************** do_local_cprop (x, insn, alter_jumps)
*** 4312,4320 ****
for (l = val->locs; l; l = l->next)
{
rtx this_rtx = l->loc;
if (CONSTANT_P (this_rtx))
newcnst = this_rtx;
! if (REG_P (this_rtx) && REGNO (this_rtx) >= FIRST_PSEUDO_REGISTER)
newreg = this_rtx;
}
if (newcnst && constprop_register (insn, x, newcnst, alter_jumps))
--- 4312,4329 ----
for (l = val->locs; l; l = l->next)
{
rtx this_rtx = l->loc;
+ rtx note;
+
if (CONSTANT_P (this_rtx))
newcnst = this_rtx;
! if (REG_P (this_rtx) && REGNO (this_rtx) >= FIRST_PSEUDO_REGISTER
! /* Don't copy propagate if it has attached REG_EQUIV note.
! At this point this only function parameters should have
! REG_EQUIV notes and if the argument slot is used somewhere
! explicitly, it means address of parameter has been taken,
! so we should not extend the lifetime of the pseudo. */
! && (!(note = find_reg_note (l->setting_insn, REG_EQUIV, NULL_RTX))
! || GET_CODE (XEXP (note, 0)) != MEM))
newreg = this_rtx;
}
if (newcnst && constprop_register (insn, x, newcnst, alter_jumps))