RFA: Fix PR middle-end/40154
diff mbox

Message ID 20111207091055.n8fa76utk0g488ko-nzlynne@webmail.spamcop.net
State New
Headers show

Commit Message

Joern Rennecke Dec. 7, 2011, 2:10 p.m. UTC
Having a value in the wrong mode is not the only hazard that users of
set_unique_reg_note should anticipate; it could also be that the value
is computed as a constant by the compiler, and thus the last instruction
obtained with get_last_insn is completely (or somewhat) unrelated; or an
expander could decide to save / restore some hard register around the
actual operation.
So, in general, the test should be if the candidate insn actually sets the
expected target.  There are already a number of users of set_unique_reg_note
that do that.  Others assume that a move, add or other expander emits a
single insn, or a sequence of insns that ends with a single insn, which
sets the target.  Although that is often the case, this is not a safe
assumption to make.  Fixing all these places with inlined code would
mean a lot of cut & paste programming.  So instead, I've added a new
wrapper for set_unique_reg_note to be used in these circumstances, and
also used it to replace the existing inlined checks unless something special
was going on (e.g. testing two different target / value pairs).

Bootstrapped and regression tested on x86_64-unknown-linux-gnu.
2011-12-07  Joern Rennecke  <joern.rennecke@embecosm.com>

	PR middle-end/40154
	* emit-rtl.c (set_dst_reg_note): New function.
	* rtl.h (set_dst_reg_note): Declare.
	* optabs.c (expand_binop, expand_absneg_bit): Use set_dst_reg_note.
	(emit_libcall_block, expand_fix): Likewise.
	* function.c (assign_parm_setup_reg, expand_function_start): Likewise.
	* expmed.c (expand_mult_const, expand_divmod): Likewise.
	* reload1.c (gen_reload): Likewise.

Comments

Richard Henderson Dec. 8, 2011, 11:59 p.m. UTC | #1
On 12/07/2011 06:10 AM, Joern Rennecke wrote:
> 	PR middle-end/40154
> 	* emit-rtl.c (set_dst_reg_note): New function.
> 	* rtl.h (set_dst_reg_note): Declare.
> 	* optabs.c (expand_binop, expand_absneg_bit): Use set_dst_reg_note.
> 	(emit_libcall_block, expand_fix): Likewise.
> 	* function.c (assign_parm_setup_reg, expand_function_start): Likewise.
> 	* expmed.c (expand_mult_const, expand_divmod): Likewise.
> 	* reload1.c (gen_reload): Likewise.

Ok.



> +							 GEN_INT
> +							   (trunc_int_for_mode

While you're at it, or as a followup, this is combination is gen_int_mode.
I mis-read the patch the first time and thought you'd done this, but it's
a pre-existing condition.


r~

Patch
diff mbox

Index: optabs.c
===================================================================
--- optabs.c	(revision 182075)
+++ optabs.c	(working copy)
@@ -2052,11 +2052,11 @@  expand_binop (enum machine_mode mode, op
 	    {
 	      rtx temp = emit_move_insn (target, xtarget);
 
-	      set_unique_reg_note (temp,
-				   REG_EQUAL,
-				   gen_rtx_fmt_ee (binoptab->code, mode,
-						   copy_rtx (xop0),
-						   copy_rtx (xop1)));
+	      set_dst_reg_note (temp, REG_EQUAL,
+				gen_rtx_fmt_ee (binoptab->code, mode,
+						copy_rtx (xop0),
+						copy_rtx (xop1)),
+				target);
 	    }
 	  else
 	    target = xtarget;
@@ -2104,11 +2104,12 @@  expand_binop (enum machine_mode mode, op
 	  if (optab_handler (mov_optab, mode) != CODE_FOR_nothing)
 	    {
 	      temp = emit_move_insn (target ? target : product, product);
-	      set_unique_reg_note (temp,
-				   REG_EQUAL,
-				   gen_rtx_fmt_ee (MULT, mode,
-						   copy_rtx (op0),
-						   copy_rtx (op1)));
+	      set_dst_reg_note (temp,
+				REG_EQUAL,
+				gen_rtx_fmt_ee (MULT, mode,
+						copy_rtx (op0),
+						copy_rtx (op1)),
+				target ? target : product);
 	    }
 	  return product;
 	}
@@ -2966,8 +2967,9 @@  expand_absneg_bit (enum rtx_code code, e
 		           gen_lowpart (imode, target), 1, OPTAB_LIB_WIDEN);
       target = lowpart_subreg_maybe_copy (mode, temp, imode);
 
-      set_unique_reg_note (get_last_insn (), REG_EQUAL,
-			   gen_rtx_fmt_e (code, mode, copy_rtx (op0)));
+      set_dst_reg_note (get_last_insn (), REG_EQUAL,
+			gen_rtx_fmt_e (code, mode, copy_rtx (op0)),
+			target);
     }
 
   return target;
@@ -3899,8 +3901,7 @@  emit_libcall_block (rtx insns, rtx targe
     }
 
   last = emit_move_insn (target, result);
-  if (optab_handler (mov_optab, GET_MODE (target)) != CODE_FOR_nothing)
-    set_unique_reg_note (last, REG_EQUAL, copy_rtx (equiv));
+  set_dst_reg_note (last, REG_EQUAL, copy_rtx (equiv), target);
 
   if (final_dest != target)
     emit_move_insn (final_dest, target);
@@ -5213,11 +5214,10 @@  expand_fix (rtx to, rtx from, int unsign
 	    {
 	      /* Make a place for a REG_NOTE and add it.  */
 	      insn = emit_move_insn (to, to);
-	      set_unique_reg_note (insn,
-	                           REG_EQUAL,
-				   gen_rtx_fmt_e (UNSIGNED_FIX,
-						  GET_MODE (to),
-						  copy_rtx (from)));
+	      set_dst_reg_note (insn, REG_EQUAL,
+				gen_rtx_fmt_e (UNSIGNED_FIX, GET_MODE (to),
+					       copy_rtx (from)),
+				to);
 	    }
 
 	  return;
Index: function.c
===================================================================
--- function.c	(revision 182075)
+++ function.c	(working copy)
@@ -3140,9 +3140,8 @@  assign_parm_setup_reg (struct assign_par
 		set_unique_reg_note (sinsn, REG_EQUIV, stackr);
 	    }
 	}
-      else if ((set = single_set (linsn)) != 0
-	       && SET_DEST (set) == parmreg)
-	set_unique_reg_note (linsn, REG_EQUIV, equiv_stack_parm);
+      else 
+	set_dst_reg_note (linsn, REG_EQUIV, equiv_stack_parm, parmreg);
     }
 
   /* For pointer data type, suggest pointer register.  */
@@ -4757,7 +4756,7 @@  expand_function_start (tree subr)
       /* Mark the register as eliminable, similar to parameters.  */
       if (MEM_P (chain)
 	  && reg_mentioned_p (arg_pointer_rtx, XEXP (chain, 0)))
-	set_unique_reg_note (insn, REG_EQUIV, chain);
+	set_dst_reg_note (insn, REG_EQUIV, chain, local);
     }
 
   /* If the function receives a non-local goto, then store the
Index: expmed.c
===================================================================
--- expmed.c	(revision 182075)
+++ expmed.c	(working copy)
@@ -2939,6 +2939,7 @@  expand_mult_const (enum machine_mode mod
 	   && !optimize)
 	  ? target : 0;
       rtx accum_target = optimize ? 0 : accum;
+      rtx accum_inner;
 
       switch (alg->op[opno])
 	{
@@ -3004,16 +3005,18 @@  expand_mult_const (enum machine_mode mod
 	 that.  */
 
       tem = op0, nmode = mode;
+      accum_inner = accum;
       if (GET_CODE (accum) == SUBREG)
 	{
-	  nmode = GET_MODE (SUBREG_REG (accum));
+	  accum_inner = SUBREG_REG (accum);
+	  nmode = GET_MODE (accum_inner);
 	  tem = gen_lowpart (nmode, op0);
 	}
 
       insn = get_last_insn ();
-      set_unique_reg_note (insn, REG_EQUAL,
-			   gen_rtx_MULT (nmode, tem,
-					 GEN_INT (val_so_far)));
+      set_dst_reg_note (insn, REG_EQUAL,
+			gen_rtx_MULT (nmode, tem, GEN_INT (val_so_far)),
+			accum_inner);
     }
 
   if (variant == negate_variant)
@@ -3823,7 +3826,7 @@  expand_divmod (int rem_flag, enum tree_c
   rtx quotient = 0, remainder = 0;
   rtx last;
   int size;
-  rtx insn, set;
+  rtx insn;
   optab optab1, optab2;
   int op1_is_constant, op1_is_pow2 = 0;
   int max_cost, extra_cost;
@@ -4127,12 +4130,10 @@  expand_divmod (int rem_flag, enum tree_c
 		  break;
 
 		insn = get_last_insn ();
-		if (insn != last
-		    && (set = single_set (insn)) != 0
-		    && SET_DEST (set) == quotient)
-		  set_unique_reg_note (insn,
-				       REG_EQUAL,
-				       gen_rtx_UDIV (compute_mode, op0, op1));
+		if (insn != last)
+		  set_dst_reg_note (insn, REG_EQUAL,
+				    gen_rtx_UDIV (compute_mode, op0, op1),
+				    quotient);
 	      }
 	    else		/* TRUNC_DIV, signed */
 	      {
@@ -4211,18 +4212,15 @@  expand_divmod (int rem_flag, enum tree_c
 		      {
 			insn = get_last_insn ();
 			if (insn != last
-			    && (set = single_set (insn)) != 0
-			    && SET_DEST (set) == quotient
 			    && abs_d < ((unsigned HOST_WIDE_INT) 1
 					<< (HOST_BITS_PER_WIDE_INT - 1)))
-			  set_unique_reg_note (insn,
-					       REG_EQUAL,
-					       gen_rtx_DIV (compute_mode,
-							    op0,
-							    GEN_INT
-							    (trunc_int_for_mode
+			  set_dst_reg_note (insn, REG_EQUAL,
+					    gen_rtx_DIV (compute_mode, op0,
+							 GEN_INT
+							   (trunc_int_for_mode
 							     (abs_d,
-							      compute_mode))));
+							      compute_mode))),
+					    quotient);
 
 			quotient = expand_unop (compute_mode, neg_optab,
 						quotient, quotient, 0);
@@ -4309,12 +4307,10 @@  expand_divmod (int rem_flag, enum tree_c
 		  break;
 
 		insn = get_last_insn ();
-		if (insn != last
-		    && (set = single_set (insn)) != 0
-		    && SET_DEST (set) == quotient)
-		  set_unique_reg_note (insn,
-				       REG_EQUAL,
-				       gen_rtx_DIV (compute_mode, op0, op1));
+		if (insn != last)
+		  set_dst_reg_note (insn, REG_EQUAL,
+				    gen_rtx_DIV (compute_mode, op0, op1),
+				    quotient);
 	      }
 	    break;
 	  }
@@ -4732,11 +4728,10 @@  expand_divmod (int rem_flag, enum tree_c
 				    NULL_RTX, 1);
 
 	    insn = get_last_insn ();
-	    set_unique_reg_note (insn,
-				 REG_EQUAL,
-				 gen_rtx_fmt_ee (unsignedp ? UDIV : DIV,
-						 compute_mode,
-						 op0, op1));
+	    set_dst_reg_note (insn, REG_EQUAL,
+			      gen_rtx_fmt_ee (unsignedp ? UDIV : DIV,
+					      compute_mode, op0, op1),
+			      quotient);
 	  }
 	break;
 
Index: emit-rtl.c
===================================================================
--- emit-rtl.c	(revision 182075)
+++ emit-rtl.c	(working copy)
@@ -5004,6 +5004,17 @@  set_unique_reg_note (rtx insn, enum reg_
 
   return REG_NOTES (insn);
 }
+
+/* Like set_unique_reg_note, but don't do anything unless INSN sets DST.  */
+rtx
+set_dst_reg_note (rtx insn, enum reg_note kind, rtx datum, rtx dst)
+{
+  rtx set = single_set (insn);
+
+  if (set && SET_DEST (set) == dst)
+    return set_unique_reg_note (insn, kind, datum);
+  return NULL_RTX;
+}
 
 /* Return an indication of which type of insn should have X as a body.
    The value is CODE_LABEL, INSN, CALL_INSN or JUMP_INSN.  */
Index: rtl.h
===================================================================
--- rtl.h	(revision 182075)
+++ rtl.h	(working copy)
@@ -1893,6 +1893,7 @@  extern enum machine_mode choose_hard_reg
 
 /* In emit-rtl.c  */
 extern rtx set_unique_reg_note (rtx, enum reg_note, rtx);
+extern rtx set_dst_reg_note (rtx, enum reg_note, rtx, rtx);
 extern void set_insn_deleted (rtx);
 
 /* Functions in rtlanal.c */
Index: reload1.c
===================================================================
--- reload1.c	(revision 182075)
+++ reload1.c	(working copy)
@@ -8574,7 +8574,7 @@  gen_reload (rtx out, rtx in, int opnum,
       if (insn)
 	{
 	  /* Add a REG_EQUIV note so that find_equiv_reg can find it.  */
-	  set_unique_reg_note (insn, REG_EQUIV, in);
+	  set_dst_reg_note (insn, REG_EQUIV, in, out);
 	  return insn;
 	}
 
@@ -8584,7 +8584,7 @@  gen_reload (rtx out, rtx in, int opnum,
       gcc_assert (!reg_overlap_mentioned_p (out, op0));
       gen_reload (out, op1, opnum, type);
       insn = emit_insn (gen_add2_insn (out, op0));
-      set_unique_reg_note (insn, REG_EQUIV, in);
+      set_dst_reg_note (insn, REG_EQUIV, in, out);
     }
 
 #ifdef SECONDARY_MEMORY_NEEDED