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.
> > On Sun, Jul 21, 2002 at 09:38:43PM +0200, Jan Hubicka wrote:
> > > 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).
> >
> > Your reasoning is incorrect. REG_EQUIV is a _local_ equivalence,
> > free to be killed at any moment. REG_EQUAL is the global equivalence.
>
> Duh, you are right. I am always swapping meaning of the notes.
> >
> > Irritatingly, the test case isn't failing for me at the moment, so
> > I can't make any more progress here.
>
> You need to revert the code regarding to REG_EQUIV note in the gcse
> cprop code. I will re-check what is going on.
I've just c hecked and the REG_EQUIV is global one, so I believe my
analysis holds. To reproduce the bug you need to revert patch I sent on
the beggining of the thread.
Whats about simply trying to stop using the REG_EQUIV as Geoff
suggested. Are they buying us that much?
I don't think his suggestion about alias analysis obsoletting them is
correct as I don't think alias analysis keeps track of objects on the
stack whose address has not been taken so they can not be referenced via
pointer, but still I would not expect the code to be big win.
Other proper fix would be probably to keep track on whether address of
argument is taken (I didn't found any in the tree structure) and kill
the EQUIVs once RTL is produced. This is done by sibcall code partly,
so may not be that dificult to generalize it.
Andreas, can you please benchmark the attached patch? I guess just
specint is fine, but please include the code sizes.
I am leaving tonight and will be away till 9th, so hope this problem to
be resolved easilly. I feel unahppy to keep regression around while I
will be offline.
Index: function.c
===================================================================
RCS file: /cvs/gcc/egcs/gcc/function.c,v
retrieving revision 1.368
diff -c -3 -p -r1.368 function.c
*** function.c 21 May 2002 20:37:40 -0000 1.368
--- function.c 25 Jul 2002 10:54:24 -0000
*************** assign_parms (fndecl)
*** 4964,5014 ****
else
parm_reg_stack_loc[REGNO (parmreg)] = stack_parm;
- /* Mark the register as eliminable if we did no conversion
- and it was copied from memory at a fixed offset,
- and the arg pointer was not copied to a pseudo-reg.
- If the arg pointer is a pseudo reg or the offset formed
- an invalid address, such memory-equivalences
- as we make here would screw up life analysis for it. */
- if (nominal_mode == passed_mode
- && ! did_conversion
- && stack_parm != 0
- && GET_CODE (stack_parm) == MEM
- && stack_offset.var == 0
- && reg_mentioned_p (virtual_incoming_args_rtx,
- XEXP (stack_parm, 0)))
- {
- rtx linsn = get_last_insn ();
- rtx sinsn, set;
-
- /* Mark complex types separately. */
- if (GET_CODE (parmreg) == CONCAT)
- /* Scan backwards for the set of the real and
- imaginary parts. */
- for (sinsn = linsn; sinsn != 0;
- sinsn = prev_nonnote_insn (sinsn))
- {
- set = single_set (sinsn);
- if (set != 0
- && SET_DEST (set) == regno_reg_rtx [regnoi])
- REG_NOTES (sinsn)
- = gen_rtx_EXPR_LIST (REG_EQUIV,
- parm_reg_stack_loc[regnoi],
- REG_NOTES (sinsn));
- else if (set != 0
- && SET_DEST (set) == regno_reg_rtx [regnor])
- REG_NOTES (sinsn)
- = gen_rtx_EXPR_LIST (REG_EQUIV,
- parm_reg_stack_loc[regnor],
- REG_NOTES (sinsn));
- }
- else if ((set = single_set (linsn)) != 0
- && SET_DEST (set) == parmreg)
- REG_NOTES (linsn)
- = gen_rtx_EXPR_LIST (REG_EQUIV,
- stack_parm, REG_NOTES (linsn));
- }
-
/* For pointer data type, suggest pointer register. */
if (POINTER_TYPE_P (TREE_TYPE (parm)))
mark_reg_pointer (parmreg,
--- 4964,4969 ----