Re: optimization/10185: [3.3/3.4 regression] Wrong code with 3-int-structs & optimization

[Jan Hubicka <jh@suse.cz>]

The following reply was made to PR optimization/10185; it has been noted by GNATS.

From: Jan Hubicka <jh@suse.cz>
To: Janis Johnson <janis187@us.ibm.com>
Cc: Steven Bosscher <s.bosscher@student.tudelft.nl>,
	rassahah@neofonie.de, gcc-gnats@gcc.gnu.org, gcc-bugs@gcc.gnu.org,
	nobody@gcc.gnu.org, gcc-prs@gcc.gnu.org, jh@suse.cz
Subject: Re: optimization/10185: [3.3/3.4 regression] Wrong code with 3-int-structs & optimization
Date: Mon, 24 Mar 2003 19:26:18 +0100

 > The regression appeared with this patch:
 > 
 > Sun Jul 21 00:54:54 CEST 2002  Jan Hubicka  <jh@suse.cz>
 > 
 > 	* gcse.c: Include cselib.h
 > 	(constptop_register): Break out from ...
 > 	(cprop_insn): ... here; kill basic_block argument.
 > 	(do_local_cprop, local_cprop_pass): New functions.
 > 	(one_cprop_pass): Call local_cprop_pass.
 > 
 > My testing used the submitter's test case with -O2 on
 > i686-pc-linux-gnu.
 
 As described in the audit, the bug is really latent problem in BIV
 discovery code.  This patch fixes the reason why gcse now manifest it.
 I am now using cselib to discover noop moves and remove them more
 aggresivly than delete_trivially_dead_insns can.  15 such noops are
 found in combine.c compilation and resulting insn chain is reduced by 7
 instructions, so this arrise in real programs too.
 
 Bootstrapped/regtested x86-64.  OK?
 
 Mon Mar 24 19:00:35 CET 2003  Jan Hubicka  <jh@suse.cz>
 	* gcse.c (local_cprop_pass):  Do not copy propagate noop sets;
 	attempt to turn set into noop.
 	* cselib.c (cselib_set_noop_p): New function.
 	* cselib.h (cselib_set_noop_p): Declare.
 Index: gcse.c
 ===================================================================
 RCS file: /cvs/gcc/gcc/gcc/gcse.c,v
 retrieving revision 1.239
 diff -c -3 -p -r1.239 gcse.c
 *** gcse.c	8 Mar 2003 09:47:28 -0000	1.239
 --- gcse.c	24 Mar 2003 18:12:32 -0000
 *************** local_cprop_pass (alter_jumps)
 *** 4437,4442 ****
 --- 4437,4443 ----
         if (INSN_P (insn))
   	{
   	  rtx note = find_reg_note (insn, REG_LIBCALL, NULL_RTX);
 + 	  rtx set;
   
   	  if (note)
   	    {
 *************** local_cprop_pass (alter_jumps)
 *** 4448,4453 ****
 --- 4449,4499 ----
   	  if (note)
   	    libcall_sp++;
   	  note = find_reg_equal_equiv_note (insn);
 + 
 + 	  if (noop_move_p (insn)
 + 	      && !find_reg_note (insn, REG_LIBCALL, NULL_RTX))
 + 	    {
 + 	      delete_insn (insn);
 + 	      continue;
 + 	    }
 + 	  
 + 	  set = single_set (insn);
 + 
 + 	  /* Don't do copy propagation on noop moves as it only
 + 	     results in these being in the insn chain longer  */
 + 	  if (set && set_noop_p (set))
 + 	    continue;
 + 	  /* Attempt to turn (set A B) where A B are known to be equivalent
 + 	     into (set A A) or (set B B) so the instruction is removed later.
 + 	     */
 + 	  if (set && cselib_set_noop_p (set)
 + 	      && (!SMALL_REGISTER_CLASSES
 + 		  || ((!REG_P (SET_DEST (set))
 + 		       || REGNO (SET_DEST (set)) >= FIRST_PSEUDO_REGISTER)
 + 		      && (GET_CODE (SET_DEST (set)) != SUBREG
 + 		          || (REGNO (SUBREG_REG (SET_DEST (set)))
 + 			      >= FIRST_PSEUDO_REGISTER)))))
 + 	    {
 + 	      bool converted = false;
 + 
 + 	      if (validate_change (insn, &SET_DEST (set),
 + 				   copy_rtx (SET_SRC (set)), 0))
 + 		converted = true;
 + 	      else if (validate_change (insn, &SET_SRC (set),
 + 					copy_rtx (SET_DEST (set)), 0))
 + 		converted = true;
 + 	      if (converted)
 + 		{
 + 		  if (gcse_file)
 + 		    fprintf (gcse_file,
 + 			     "LOCAL COPY-PROP: Set in insn %d turned into noop",
 + 			     INSN_UID (insn));
 + 		  if (noop_move_p (insn)
 + 		      && !find_reg_note (insn, REG_LIBCALL, NULL_RTX))
 + 		    delete_insn (insn);
 + 		  continue;
 + 		}
 + 	    }
   	  do
   	    {
   	      reg_use_count = 0;
 Index: cselib.c
 ===================================================================
 RCS file: /cvs/gcc/gcc/gcc/cselib.c,v
 retrieving revision 1.26
 diff -c -3 -p -r1.26 cselib.c
 *** cselib.c	14 Mar 2003 20:15:11 -0000	1.26
 --- cselib.c	24 Mar 2003 18:12:33 -0000
 *************** rtx_equal_for_cselib_p (x, y)
 *** 541,546 ****
 --- 541,570 ----
     return 1;
   }
   
 + /* Return nonzero if the destination of SET equals the source
 +    and there are no side effects.  Use cselib infromation to recognize
 +    nontrivial cases.  */
 + 
 + bool
 + cselib_set_noop_p (set)
 +      rtx set;
 + {
 +   rtx src = SET_SRC (set);
 +   rtx dst = SET_DEST (set);
 + 
 +   if (GET_CODE (dst) == STRICT_LOW_PART)
 +     dst = XEXP (dst, 0);
 +   if ((GET_CODE (dst) == SIGN_EXTRACT
 +        || GET_CODE (dst) == ZERO_EXTRACT)
 +       && ! BYTES_BIG_ENDIAN && XEXP (dst, 2) == const0_rtx)
 +     dst = XEXP (dst, 0);
 + 
 +   if (rtx_equal_for_cselib_p (src, dst))
 +     return !side_effects_p (src) && !side_effects_p (dst);
 + 
 +   return false;
 + }
 + 
   /* We need to pass down the mode of constants through the hash table
      functions.  For that purpose, wrap them in a CONST of the appropriate
      mode.  */
 Index: cselib.h
 ===================================================================
 RCS file: /cvs/gcc/gcc/gcc/cselib.h,v
 retrieving revision 1.7
 diff -c -3 -p -r1.7 cselib.h
 *** cselib.h	11 Mar 2003 21:52:41 -0000	1.7
 --- cselib.h	24 Mar 2003 18:12:33 -0000
 *************** extern void cselib_update_varray_sizes	P
 *** 67,72 ****
 --- 67,73 ----
   extern void cselib_init			PARAMS ((void));
   extern void cselib_finish		PARAMS ((void));
   extern void cselib_process_insn		PARAMS ((rtx));
 + extern bool cselib_set_noop_p		PARAMS ((rtx));
   extern int rtx_equal_for_cselib_p	PARAMS ((rtx, rtx));
   extern int references_value_p		PARAMS ((rtx, int));
   extern rtx cselib_subst_to_values	PARAMS ((rtx));