cse vs libcalls

Jim Wilson wilson@cygnus.com
Wed Nov 3 17:33:00 GMT 1999


Your recent cse.c change broke the ia64 port.  Redundant compares/branches
aren't being optimized away anymore, and this is causing later optimization
passes to fail.
	
The problem here is that your patch accidentally disabled adding the SET_SRC to
src_elt.  The code computing src_elt does something like this:
  if src_eqv and src_eqv_elt != 0 then
    compute src_eqv_elt
    for all sets
      if SET_SRC and src_eqv are the same
        put src_eqv_elt in src_elt
  for all sets
    if SET_SRC and SRC_DEST are not the same
      if src_elt != 0 and there is no REG_RETVAL note
	put both SET_SRC and src_eqv_elt in src_elt
      if src_const
	also put src_const in src_elt
    else
      put only src_eqv_elt in src_elt
You disabled the "SET_SRC and src_eqv are the same" check.  However, since
there is a later check for src_elt != 0, this has the side effect of preventing
SET_SRC from being put in src_elt.

The real problem here is with the check for a REG_RETVAL note.  Normally,
we want to put both the SET_SRC and the src_eqv_elt in src_elt.  But if the
SET_SRC is a return register, this would be wrong.  The existing code does
nothing in this case, but it should still be putting src_eqv_elt in src_elt.
That is, the code
      if src_elt != 0 and there is no REG_RETVAL note
	put both SET_SRC and src_eqv_elt in src_elt
should instead be
      if src_elt != 0
	if no REG_RETVAL note
	  put both SET_SRC and src_eqv_elt in src_elt
	else
	  put only src_eqv_elt in src_elt

We can now see that the first check "SET_SRC and src_eqv are the same"
is just a short cut.  If these two values are the same, then there is no
point in adding both of them to src_elt.  We only need to add one.  Hence
this code was correct before your patch.

This patch makes the ia64 port work again.  I also tested it against your
original testcase using the m32r port, and it also solves your original
problem.  This patch is mostly an indentation change, so it isn't as big
as it looks.

I have not installed this patch yet, in case you wanted to comment on it.

Wed Nov  3 16:56:33 1999  Jim Wilson  <wilson@cygnus.com>

	* cse.c (cse_insn): Revert previous change.  When computing src_elt,
	if REG_RETVAL check succeeds, then put classp in src_elt.

Index: cse.c
===================================================================
RCS file: /cvs/cvsfiles/devo/gcc/cse.c,v
retrieving revision 1.184
diff -p -r1.184 cse.c
*** cse.c	1999/11/01 02:19:45	1.184
--- cse.c	1999/11/04 00:55:57
*************** cse_insn (insn, libcall_insn)
*** 5741,5749 ****
  	 does not yet have an elt, and if so set the elt of the set source
  	 to src_eqv_elt.  */
        for (i = 0; i < n_sets; i++)
! 	if (n_sets == 1
! 	    || (sets[i].rtl && sets[i].src_elt == 0
! 		&& rtx_equal_p (SET_SRC (sets[i].rtl), src_eqv)))
  	  sets[i].src_elt = src_eqv_elt;
      }
  
--- 5741,5748 ----
  	 does not yet have an elt, and if so set the elt of the set source
  	 to src_eqv_elt.  */
        for (i = 0; i < n_sets; i++)
! 	if (sets[i].rtl && sets[i].src_elt == 0
! 	    && rtx_equal_p (SET_SRC (sets[i].rtl), src_eqv))
  	  sets[i].src_elt = src_eqv_elt;
      }
  
*************** cse_insn (insn, libcall_insn)
*** 5771,5799 ****
  	    enum machine_mode mode
  	      = GET_MODE (src) == VOIDmode ? GET_MODE (dest) : GET_MODE (src);
  
! 	    /* Don't put a hard register source into the table if this is
! 	       the last insn of a libcall.  */
! 	    if (sets[i].src_elt == 0
! 		&& (GET_CODE (src) != REG
! 		    || REGNO (src) >= FIRST_PSEUDO_REGISTER
! 		    || ! find_reg_note (insn, REG_RETVAL, NULL_RTX)))
  	      {
! 		register struct table_elt *elt;
! 
! 		/* Note that these insert_regs calls cannot remove
! 		   any of the src_elt's, because they would have failed to
! 		   match if not still valid.  */
! 		if (insert_regs (src, classp, 0))
  		  {
! 		    rehash_using_reg (src);
! 		    sets[i].src_hash = HASH (src, mode);
  		  }
! 		elt = insert (src, classp, sets[i].src_hash, mode);
! 		elt->in_memory = sets[i].src_in_memory;
! 		elt->in_struct = sets[i].src_in_struct;
! 		sets[i].src_elt = classp = elt;
  	      }
- 
  	    if (sets[i].src_const && sets[i].src_const_elt == 0
  		&& src != sets[i].src_const
  		&& ! rtx_equal_p (sets[i].src_const, src))
--- 5770,5802 ----
  	    enum machine_mode mode
  	      = GET_MODE (src) == VOIDmode ? GET_MODE (dest) : GET_MODE (src);
  
! 	    if (sets[i].src_elt == 0)
  	      {
! 		/* Don't put a hard register source into the table if this is
! 		   the last insn of a libcall.  In this case, we only need
! 		   to put src_eqv_elt in src_elt.  */
! 		if (GET_CODE (src) != REG
! 		    || REGNO (src) >= FIRST_PSEUDO_REGISTER
! 		    || ! find_reg_note (insn, REG_RETVAL, NULL_RTX))
  		  {
! 		    register struct table_elt *elt;
! 
! 		    /* Note that these insert_regs calls cannot remove
! 		       any of the src_elt's, because they would have failed to
! 		       match if not still valid.  */
! 		    if (insert_regs (src, classp, 0))
! 		      {
! 			rehash_using_reg (src);
! 			sets[i].src_hash = HASH (src, mode);
! 		      }
! 		    elt = insert (src, classp, sets[i].src_hash, mode);
! 		    elt->in_memory = sets[i].src_in_memory;
! 		    elt->in_struct = sets[i].src_in_struct;
! 		    sets[i].src_elt = classp = elt;
  		  }
! 		else
! 		  sets[i].src_elt = classp;
  	      }
  	    if (sets[i].src_const && sets[i].src_const_elt == 0
  		&& src != sets[i].src_const
  		&& ! rtx_equal_p (sets[i].src_const, src))


More information about the Gcc-patches mailing list