RFA: fix rtl-optimization/56833

Joern Rennecke joern.rennecke@embecosm.com
Wed May 22 10:45:00 GMT 2013


Quoting Eric Botcazou <ebotcazou@adacore.com>:

>> The problem was that we had some optimzations added to the
>> reload_cse_move2add pass that would attempt transformations with
>> multi-hard-register registers, without keeping track of the validity of the
>> values in all hard registers involved.
>
> That's not clear to me: for example in move2add_note_store, we reset the info
> about any multi hard-register; moveover 5 arrays are supposed to be   
> maintained
> for each hard-register:
>
>   for (i = FIRST_PSEUDO_REGISTER - 1; i >= 0; i--)
>     {
>       reg_set_luid[i] = 0;
>       reg_offset[i] = 0;
>       reg_base_reg[i] = 0;
>       reg_symbol_ref[i] = NULL_RTX;
>       reg_mode[i] = VOIDmode;
>     }
> so I'm not sure whether we really properly handle multi hard-registers.

Initializing all the values is nice to have defined contents, but
the values in reg_offset[i] / reg_base_reg[i] / reg_symbol_ref[i]
just tell what's in these registers if they are valid, not if they
are valid.
But I can see that there could be a problem with an earlier value
that used to be valid in a multi-hard-register sub register to be
considered to be still valid.
Setting the mode of every constituent register but the first one
(which has the new value recorded) to VOIDmode at the same time
as updating reg_set_luid should be sufficent to address this issue.

> So
> what about explicitly punting for multi hard-registers in   
> reload_cse_move2add?
> Do you think that this would pessimize too much in practice?

The pass was originally written with word_mode == Pmode targets like
the SH in mind, where multi-hard-register values are uninteresting.

But for targets like the avr, most or all of the interesting values
will be in multi-hard-register registers.
-------------- next part --------------
2013-05-22  Joern Rennecke <joern.rennecke@embecosm.com>

	PR rtl-optimization/56833
	* postreload.c (move2add_use_add2_insn): Update reg_set_luid
	and reg_mode for all affected hard registers.
	(move2add_use_add3_insn): Likewise.  Check that all source hard
	regs have been set by the same set.
	(reload_cse_move2add): Before concluding that we have a pre-existing
	value, check that all destination hard registers have been set by the
	same set.

Index: postreload.c
===================================================================
--- postreload.c	(revision 199190)
+++ postreload.c	(working copy)
@@ -1749,7 +1749,13 @@ move2add_use_add2_insn (rtx reg, rtx sym
 	    }
 	}
     }
-  reg_set_luid[regno] = move2add_luid;
+  int i = hard_regno_nregs[regno][GET_MODE (reg)] -1;
+  do
+    {
+      reg_set_luid[regno + i] = move2add_luid;
+      reg_mode[regno + i] = VOIDmode;
+    }
+  while (--i >= 0);
   reg_base_reg[regno] = -1;
   reg_mode[regno] = GET_MODE (reg);
   reg_symbol_ref[regno] = sym;
@@ -1793,6 +1799,14 @@ move2add_use_add3_insn (rtx reg, rtx sym
 	&& reg_symbol_ref[i] != NULL_RTX
 	&& rtx_equal_p (sym, reg_symbol_ref[i]))
       {
+	int j;
+
+	for (j = hard_regno_nregs[i][reg_mode[i]] - 1;
+	     j > 0 && reg_set_luid[i+j] == reg_set_luid[i];)
+	  j--;
+	if (j != 0)
+	  continue;
+
 	rtx new_src = gen_int_mode (INTVAL (off) - reg_offset[i],
 				    GET_MODE (reg));
 	/* (set (reg) (plus (reg) (const_int 0))) is not canonical;
@@ -1835,7 +1849,13 @@ move2add_use_add3_insn (rtx reg, rtx sym
       if (validate_change (insn, &SET_SRC (pat), tem, 0))
 	changed = true;
     }
-  reg_set_luid[regno] = move2add_luid;
+  i = hard_regno_nregs[regno][GET_MODE (reg)] -1;
+  do
+    {
+      reg_set_luid[regno + i] = move2add_luid;
+      reg_mode[regno + i] = VOIDmode;
+    }
+  while (--i >= 0);
   reg_base_reg[regno] = -1;
   reg_mode[regno] = GET_MODE (reg);
   reg_symbol_ref[regno] = sym;
@@ -1890,7 +1910,10 @@ reload_cse_move2add (rtx first)
 
 	  /* Check if we have valid information on the contents of this
 	     register in the mode of REG.  */
-	  if (reg_set_luid[regno] > move2add_last_label_luid
+	  for (i = hard_regno_nregs[regno][GET_MODE (reg)] - 1;
+	       i > 0 && reg_set_luid[regno + i] == reg_set_luid[regno];)
+	    i--;
+	  if (i == 0 && reg_set_luid[regno] > move2add_last_label_luid
 	      && MODES_OK_FOR_MOVE2ADD (GET_MODE (reg), reg_mode[regno])
               && dbg_cnt (cse2_move2add))
 	    {
@@ -2021,7 +2044,10 @@ reload_cse_move2add (rtx first)
 
 	      /* If the reg already contains the value which is sum of
 		 sym and some constant value, we can use an add2 insn.  */
-	      if (reg_set_luid[regno] > move2add_last_label_luid
+	      for (i = hard_regno_nregs[regno][GET_MODE (reg)] - 1;
+		   i > 0 && reg_set_luid[regno + i] == reg_set_luid[regno];)
+		i--;
+	      if (i == 0 && reg_set_luid[regno] > move2add_last_label_luid
 		  && MODES_OK_FOR_MOVE2ADD (GET_MODE (reg), reg_mode[regno])
 		  && reg_base_reg[regno] < 0
 		  && reg_symbol_ref[regno] != NULL_RTX


More information about the Gcc-patches mailing list