Re: [new-ra] "#if DENIS"

[Denis Chertykov <denisc at overta dot ru>]

Michael Matz <matz@suse.de> writes:

> Hi,
> 
> On 23 Jul 2003, Denis Chertykov wrote:
> 
> > No. ra_modified_insns also contain insns modified only in current pass.
> 
> Doh.  My bad ;)
> 
> > > Yes of course.  But it simply wasn't working.  The allocator crashed
> > > sometimes when presented with the new or changed instructions results from
> > > those later pre-reloads.
> >
> > Ugh. I lose your think. :(
> 
> I just wanted to say, that I disabled to actually make pre-reloads after
> the first pass.  I did this, because doing pre-reloads in later passes
> confused the incremental machinery to a point where inconsistent
> structures were built (which later lead to abort() in sanity checks).
> 
> IIRC (it is months ago, I don't remember the specifics) the problem had to
> do with new instructions emitted, or with the changed instructions, which
> lead to wrong collection of register references in df.c, which then
> resulted in some webs containing old register references, but not all of
> the new one which were emitted by pre-reloads.
> 
> > Few mails ago I have asked "why you don't use ra_validate_change".
> > I asked because only ra_validate_change can guarantee that changes will be
> > valid i.e. reload or pre-reload will not be needed.
> 
> This is independend from the above point, and I think I answered that by
> mentioning that I changed validate_change() to do what I wanted.
> 
> > i1 use p10
> > i21 (INSN DELETED)
> > i2 use stack-slot-for-p11'
> >
> > All insns in this case will be valid in any pass.
> 
> Do you argument that there should have been no later pre-reloads generated
> anyway in later coloring rounds, because all instructions were already
> basically valid?

Yes.

> This was definitely false, there were pre-reloads generated.  You
> could also mean, that this was only a result of me using
> validate_change() (the changed one), not ra_validate_change(), and I
> don't remember if this was the case.

It is the case.

> One could simply activate generation of pre-reloads again in later
> passes and try a full bootstrap on x86.  I believe that would
> trigger it.

I have experimented with validate_change and ra_validate_change.
I have removed your fragment of code which check ra_pass inside
pre-reload and added new fragment which call abort if pre-reload
required after first pass of allocation. (See the patch at end of
mail. Patch against CVS tree.)
Also, I have substituted all calls to validate_change by calls to
ra_validate_change and got the bootstrap failure while stage2 compiler
tries to compile crt... it's bad, but while stage1 compiler compile
the stage2 compiler no pre-reload happened after firts pass of
allocation.

It's very important because only usage of validate_change results to
wrong insns and pre-reload after first pass.

I don't know why stage2 compiler isn't work.
May be I have damaged your idea of spill-slot == stack-slot or may be
ra_validate_change have an error. I will try to fix.

I have checked GCC by c-torture and founded that only execution of
builtin-constant.c failed.(IMHO it's independent from allocator)

Also, I have applied a small syntax fix to CVS tree.

Denis.


The patch:

Index: pre-reload.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/Attic/pre-reload.c,v
retrieving revision 1.1.2.16
diff -c -3 -p -r1.1.2.16 pre-reload.c
*** pre-reload.c	2 Jul 2003 14:59:51 -0000	1.1.2.16
--- pre-reload.c	1 Aug 2003 11:03:06 -0000
*************** collect_insn_info (ra_info, insn, def_re
*** 3205,3216 ****
    for (i = 0; i < noperands; i++)
      goal_alt[i].win |= goal_alt[i].match_win;
  
-   /* In the later passes we don't want to create new reload insns, but just
-      record register classes.  */
-   if (ra_pass > 1)
-     for (i = 0; i < noperands; i++)
-       goal_alt[i].win = 1;
- 
    /* If the best alternative is with operands 1 and 2 swapped,
       consider them swapped before reporting the reloads.  Update the
       operand numbers of any reloads already pushed.  */
--- 3205,3210 ----
*************** pre_reload_collect (ra_info, modified)
*** 4383,4388 ****
--- 4377,4387 ----
  		{
  		  rtx before = PREV_INSN (insn);
  		  rtx after = NEXT_INSN (insn);
+ 
+ 		  /* In the later passes we don't want to create new reload
+ 		     insns, but just record register classes.  */
+ 		  if (ra_pass > 1)
+ 		    abort ();
  
  		  if (rtl_dump_file)
  		    {
Index: ra-rewrite.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/ra-rewrite.c,v
retrieving revision 1.1.2.20
diff -c -3 -p -r1.1.2.20 ra-rewrite.c
*** ra-rewrite.c	1 Aug 2003 10:53:05 -0000	1.1.2.20
--- ra-rewrite.c	1 Aug 2003 11:03:28 -0000
*************** insert_stores (new_deaths)
*** 799,805 ****
  #else
  		  if ((DF_REF_FLAGS (info.defs[n]) & DF_REF_ALREADY_SPILLED)
  		      || (! (rdef && RA_REF_ADDRESS_P (rdef))
! 			  && validate_change (insn, DF_REF_LOC (info.defs[n]),
  					      slot, 0)))
  		    {
  		      df_insn_modify (df, bb, insn);
--- 799,805 ----
  #else
  		  if ((DF_REF_FLAGS (info.defs[n]) & DF_REF_ALREADY_SPILLED)
  		      || (! (rdef && RA_REF_ADDRESS_P (rdef))
! 			  && ra_validate_change (insn, DF_REF_LOC (info.defs[n]),
  					      slot, 0)))
  		    {
  		      df_insn_modify (df, bb, insn);
*************** emit_loads (ri, nl_first_reload, last_bl
*** 1146,1156 ****
  		  /* For an rmw web we want to try to change the use and the
  		     def inplace to the mem-ref.  If that doesn't work, only
  		     try to handle the use.  */
! 		  validate_change (web->last_use_insn,
  				   DF_REF_LOC (web->last_use), slot, 1);
! 		  validate_change (web->last_use_insn,
  				   DF_REF_LOC (info.defs[n]), slot, 1);
! 		  if (apply_change_group ())
  		    {
  		      DF_REF_FLAGS (info.defs[n]) |= DF_REF_ALREADY_SPILLED;
  		      done = 1;
--- 1146,1156 ----
  		  /* For an rmw web we want to try to change the use and the
  		     def inplace to the mem-ref.  If that doesn't work, only
  		     try to handle the use.  */
! 		  ra_validate_change (web->last_use_insn,
  				   DF_REF_LOC (web->last_use), slot, 1);
! 		  ra_validate_change (web->last_use_insn,
  				   DF_REF_LOC (info.defs[n]), slot, 1);
! 		  if (ra_apply_change_group ())
  		    {
  		      DF_REF_FLAGS (info.defs[n]) |= DF_REF_ALREADY_SPILLED;
  		      done = 1;
*************** emit_loads (ri, nl_first_reload, last_bl
*** 1160,1166 ****
  	  /* No rmw web or spilling the def too didn't work,
  	     so handle just the use here.  */
  	  if (!done && !(ruse && RA_REF_ADDRESS_P (ruse))
! 	      && validate_change (web->last_use_insn,
  				  DF_REF_LOC (web->last_use), slot, 0))
  	    done = 1;
  	  if (done)
--- 1160,1166 ----
  	  /* No rmw web or spilling the def too didn't work,
  	     so handle just the use here.  */
  	  if (!done && !(ruse && RA_REF_ADDRESS_P (ruse))
! 	      && ra_validate_change (web->last_use_insn,
  				  DF_REF_LOC (web->last_use), slot, 0))
  	    done = 1;
  	  if (done)