Avoid invalid RTL sharing

Jan Hubicka jh@suse.cz
Thu Nov 9 22:35:00 GMT 2006


Hi,
with this patch, I can bootstrap i686 with verify_rtl_sharing run after each but
the pre-unshare_insns pass.  This is something I planned since 2003, but the
old loop optimizer was real offender and at that time I decided it is better
to wait for Zdenek to finish the replacement.  Now it is there.
Certainly there are more sharing bugs in the backend, but I would like to get
bootstrapping the basic platforms and get this enabled by default so we gradually
hammer them down.

THe problems here are all manifested in building libgcc, no more problems are shown
later, so I don't think testcases are neccesary. cse_process_notes problem is related
to complex CONST expressions that are not allowed to be shared.  THe copy_rtx should
do work only on those so it should be safe.

Similar case is seen in try_replace_reg (that does replace regs only by constants or
regs, so the complex CONST is about the worst evil one can get).

local-alloc.c introduce invalid RTL sharing for every MEM expression it biuld REG_EQUIV
note for.  After reload it is valid to share memory so this is probably quite harmless,
but still I think it is better for strict validity to SPECs than the alternative of
relaxing checker here.

i386 has few problems with optimize_mode_switching and function.c fails to copy
duplicated arguments creating invalid sharing in e.g. DIVMOD pattern on i386.

Bootstrapped/regtested i686-linux, OK?

:ADDPATCH middle-end:

Honza

	* cse.c (cse_process_notes): Copy the propagated value.
	* local-alloc.c (update_equiv_regs): Copy the memory RTX to be used
	in REG_EQUIV notes.
	* gcse.c (try_replace_reg): Copy the replacement.
	* i386.c (emit_i387_cw_initialization): Copy stored_mode
	(assign_386_stack_local): Always return copied memory expression
	* function.c (instantiate_virtual_regs_in_insn): Copy the operand
	duplicates.

Index: cse.c
===================================================================
*** cse.c	(revision 118584)
--- cse.c	(working copy)
*************** cse_process_notes (rtx x, rtx object)
*** 5817,5823 ****
  	    {
  	      rtx new = gen_lowpart (GET_MODE (x), ent->const_rtx);
  	      if (new)
! 		return new;
  	    }
  	}
  
--- 5817,5823 ----
  	    {
  	      rtx new = gen_lowpart (GET_MODE (x), ent->const_rtx);
  	      if (new)
! 		return copy_rtx (new);
  	    }
  	}
  
Index: local-alloc.c
===================================================================
*** local-alloc.c	(revision 118584)
--- local-alloc.c	(working copy)
*************** update_equiv_regs (void)
*** 907,913 ****
  	     REG_EQUAL note on the insn.  Since this note would be redundant,
  	     there's no point creating it earlier than here.  */
  	  if (! note && ! rtx_varies_p (src, 0))
! 	    note = set_unique_reg_note (insn, REG_EQUAL, src);
  
  	  /* Don't bother considering a REG_EQUAL note containing an EXPR_LIST
  	     since it represents a function call */
--- 907,913 ----
  	     REG_EQUAL note on the insn.  Since this note would be redundant,
  	     there's no point creating it earlier than here.  */
  	  if (! note && ! rtx_varies_p (src, 0))
! 	    note = set_unique_reg_note (insn, REG_EQUAL, copy_rtx (src));
  
  	  /* Don't bother considering a REG_EQUAL note containing an EXPR_LIST
  	     since it represents a function call */
*************** update_equiv_regs (void)
*** 953,959 ****
  	  if (note == 0 && REG_BASIC_BLOCK (regno) >= 0
  	      && MEM_P (SET_SRC (set))
  	      && validate_equiv_mem (insn, dest, SET_SRC (set)))
! 	    REG_NOTES (insn) = note = gen_rtx_EXPR_LIST (REG_EQUIV, SET_SRC (set),
  							 REG_NOTES (insn));
  
  	  if (note)
--- 953,960 ----
  	  if (note == 0 && REG_BASIC_BLOCK (regno) >= 0
  	      && MEM_P (SET_SRC (set))
  	      && validate_equiv_mem (insn, dest, SET_SRC (set)))
! 	    REG_NOTES (insn) = note = gen_rtx_EXPR_LIST (REG_EQUIV,
! 			    				 copy_rtx (SET_SRC (set)),
  							 REG_NOTES (insn));
  
  	  if (note)
*************** update_equiv_regs (void)
*** 1061,1067 ****
  	      && ! memref_used_between_p (dest, init_insn, insn))
  	    {
  	      REG_NOTES (init_insn)
! 		= gen_rtx_EXPR_LIST (REG_EQUIV, dest,
  				     REG_NOTES (init_insn));
  	      /* This insn makes the equivalence, not the one initializing
  		 the register.  */
--- 1062,1068 ----
  	      && ! memref_used_between_p (dest, init_insn, insn))
  	    {
  	      REG_NOTES (init_insn)
! 		= gen_rtx_EXPR_LIST (REG_EQUIV, copy_rtx (dest),
  				     REG_NOTES (init_insn));
  	      /* This insn makes the equivalence, not the one initializing
  		 the register.  */
Index: gcse.c
===================================================================
*** gcse.c	(revision 118584)
--- gcse.c	(working copy)
*************** try_replace_reg (rtx from, rtx to, rtx i
*** 2647,2652 ****
--- 2647,2657 ----
    int success = 0;
    rtx set = single_set (insn);
  
+   /* Usually we substitute easy stuff, so we won't copy everything.
+      We however need to take care to not duplicate non-trivial CONST
+      expressions.  */
+   to = copy_rtx (to);
+ 
    validate_replace_src_group (from, to, insn);
    if (num_changes_pending () && apply_change_group ())
      success = 1;
Index: config/i386/i386.c
===================================================================
*** config/i386/i386.c	(revision 118584)
--- config/i386/i386.c	(working copy)
*************** emit_i387_cw_initialization (int mode)
*** 8589,8595 ****
    rtx reg = gen_reg_rtx (HImode);
  
    emit_insn (gen_x86_fnstcw_1 (stored_mode));
!   emit_move_insn (reg, stored_mode);
  
    if (TARGET_64BIT || TARGET_PARTIAL_REG_STALL || optimize_size)
      {
--- 8589,8595 ----
    rtx reg = gen_reg_rtx (HImode);
  
    emit_insn (gen_x86_fnstcw_1 (stored_mode));
!   emit_move_insn (reg, copy_rtx (stored_mode));
  
    if (TARGET_64BIT || TARGET_PARTIAL_REG_STALL || optimize_size)
      {
*************** assign_386_stack_local (enum machine_mod
*** 13520,13526 ****
  
    for (s = ix86_stack_locals; s; s = s->next)
      if (s->mode == mode && s->n == n)
!       return s->rtl;
  
    s = (struct stack_local_entry *)
      ggc_alloc (sizeof (struct stack_local_entry));
--- 13520,13526 ----
  
    for (s = ix86_stack_locals; s; s = s->next)
      if (s->mode == mode && s->n == n)
!       return copy_rtx (s->rtl);
  
    s = (struct stack_local_entry *)
      ggc_alloc (sizeof (struct stack_local_entry));
Index: function.c
===================================================================
*** function.c	(revision 118584)
--- function.c	(working copy)
*************** instantiate_virtual_regs_in_insn (rtx in
*** 1540,1546 ****
        /* Propagate operand changes into the duplicates.  */
        for (i = 0; i < recog_data.n_dups; ++i)
  	*recog_data.dup_loc[i]
! 	  = recog_data.operand[(unsigned)recog_data.dup_num[i]];
  
        /* Force re-recognition of the instruction for validation.  */
        INSN_CODE (insn) = -1;
--- 1540,1546 ----
        /* Propagate operand changes into the duplicates.  */
        for (i = 0; i < recog_data.n_dups; ++i)
  	*recog_data.dup_loc[i]
! 	  = copy_rtx (recog_data.operand[(unsigned)recog_data.dup_num[i]]);
  
        /* Force re-recognition of the instruction for validation.  */
        INSN_CODE (insn) = -1;



More information about the Gcc-patches mailing list