Re: Recent changes to cse.c

[Daniel Berlin <dan at cgsoftware dot com>]

Andreas Jaeger <aj@suse.de> writes:

> David Edelsohn <dje@watson.ibm.com> writes:
> 
>>>>>>> "Mike Lerwill" writes:
>> 
>>Mike> Following the recent changes there is a problem with compiling cse.c for
>>Mike> targets that define HAVE_cc0.
>> 
>>Mike> Specifically in the function set_live_p which uses insn which is not
>>Mike> declared anywhere.
>> 
>> 	Okay, the clock has started.  If this is not fixed within 48
>> hours, the patch which caused the problem is an automatic candidate to be
>> reverted. 
> 
> Nope, the clock has not started yet - at least not in my
> understanding.  Mark's note read:
> 
>     If a patch is committed which introduces a regression [1], on any
>     target which the Steering Committee considers to be important [2],
>     and the problem is reported to the original poster, and 48 hours
>     pass without either the original poster or any other party
> 
> And where is this problem reported to the original poster?  It's not
> even located yet.  IMO somebody first have to look into this and nail
> down exactly which patch broke this - and then we have to see if it
> breaks on an important target.
> 
> IMO the clock hasn't started *yet* but it might start after some people
> have done a bit more analysis,


This is caused by Richard Henderson's change.  However, since i
noticed the failure in df.c, and came up with an incorrect patch
(never applied), that eventually led to him making this change instead, I take
all the blame. 
There are about 8 small variations on how to fix this simply (depending on
whether you want to just pass the insn in all the time, pass the insn
and the set, etc), i just chose one of them. Some might feel a "better" fix would be
to make insn_live_p do the check for CC0 machines instead, rather than
pass the insn to set_live_p.

That said, the fix follows, non-bootstrapped and non-checked. My machine is
having a bit of instability with a new kernel right now (It's also
preventing a last make check so i can submit the store motion stuff), and i'm in
the middle of some work i wanted to finish tonight, so someone
else will have to perform a bootstrap and regression test on this
patch before it can be checked in, if they want it done
tonight. Sorry.



 cse.c (set_live_p): Add new argument, for the insn. CC0 targets want
 to do a check against the next insn when trying to determine if a
 set is live.
 (insn_live_p): Pass in insn to set_live_p.

Index: cse.c
===================================================================
RCS file: /cvs/gcc/egcs/gcc/cse.c,v
retrieving revision 1.191
diff -c -3 -p -w -B -b -r1.191 cse.c
*** cse.c	2001/07/16 20:54:43	1.191
--- cse.c	2001/07/17 05:24:46
*************** static int check_dependence	PARAMS ((rtx
*** 690,696 ****
  
  static void flush_hash_table	PARAMS ((void));
  static bool insn_live_p		PARAMS ((rtx, int *));
! static bool set_live_p		PARAMS ((rtx, int *));
  static bool dead_libcall_p	PARAMS ((rtx, int *));
  
  /* Dump the expressions in the equivalence class indicated by CLASSP.
--- 690,696 ----
  
  static void flush_hash_table	PARAMS ((void));
  static bool insn_live_p		PARAMS ((rtx, int *));
! static bool set_live_p		PARAMS ((rtx, rtx, int *));
  static bool dead_libcall_p	PARAMS ((rtx, int *));
  
  /* Dump the expressions in the equivalence class indicated by CLASSP.
*************** count_reg_usage (x, counts, dest, incr)
*** 7486,7493 ****
  
  /* Return true if set is live.  */
  static bool
! set_live_p (set, counts)
       rtx set;
       int *counts;
  {
  #ifdef HAVE_cc0
--- 7486,7494 ----
  
  /* Return true if set is live.  */
  static bool
! set_live_p (set, insn, counts)
       rtx set;
+      rtx insn;
       int *counts;
  {
  #ifdef HAVE_cc0
*************** insn_live_p (insn, counts)
*** 7527,7533 ****
  {
    int i;
    if (GET_CODE (PATTERN (insn)) == SET)
!     return set_live_p (PATTERN (insn), counts);
    else if (GET_CODE (PATTERN (insn)) == PARALLEL)
      for (i = XVECLEN (PATTERN (insn), 0) - 1; i >= 0; i--)
        {
--- 7528,7534 ----
  {
    int i;
    if (GET_CODE (PATTERN (insn)) == SET)
!     return set_live_p (PATTERN (insn), insn, counts);
    else if (GET_CODE (PATTERN (insn)) == PARALLEL)
      for (i = XVECLEN (PATTERN (insn), 0) - 1; i >= 0; i--)
        {
*************** insn_live_p (insn, counts)
*** 7535,7541 ****
  
  	if (GET_CODE (elt) == SET)
  	  {
! 	    if (set_live_p (elt, counts))
  	      return true;
  	  }
  	else if (GET_CODE (elt) != CLOBBER && GET_CODE (elt) != USE)
--- 7536,7542 ----
  
  	if (GET_CODE (elt) == SET)
  	  {
! 	    if (set_live_p (elt, insn, counts))
  	      return true;
  	  }
  	else if (GET_CODE (elt) != CLOBBER && GET_CODE (elt) != USE)








> 
> Andreas
> -- 
>  Andreas Jaeger
>   SuSE Labs aj@suse.de
>    private aj@arthur.inka.de
>     http://www.suse.de/~aj

-- 
"I used to own an ant farm but had to give it up.  I couldn't
find tractors small enough to fit it.
"-Steven Wright