Patchwork RFA: fix rtl-optimization/56833

login
register
mail settings
Submitter Joern Rennecke
Date May 15, 2013, 3 p.m.
Message ID <20130515110002.j5o6zwvv8c88c48g-nzlynne@webmail.spamcop.net>
Download mbox | patch
Permalink /patch/244105/
State New
Headers show

Comments

Joern Rennecke - May 15, 2013, 3 p.m.
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.

The attached patch fixes this by updating reg_set_luid for all hard regs
involved in a set, and requiring identical reg_set_luid values for all
hard regs involved when using a value.

bootstrapped/regtested on i686-pc-linux-gnu
2013-05-14  Joern Rennecke <joern.rennecke@embecosm.com>

	PR rtl-optimization/56833
	* postreload.c (move2add_use_add2_insn): Update reg_set_luid
	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.
Eric Botcazou - May 22, 2013, 9:03 a.m.
> 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.  So 
what about explicitly punting for multi hard-registers in reload_cse_move2add?
Do you think that this would pessimize too much in practice?

Patch

Index: postreload.c
===================================================================
--- postreload.c	(revision 198927)
+++ postreload.c	(working copy)
@@ -1749,7 +1749,10 @@  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;
+  while (--i >= 0);
   reg_base_reg[regno] = -1;
   reg_mode[regno] = GET_MODE (reg);
   reg_symbol_ref[regno] = sym;
@@ -1793,6 +1796,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 +1846,10 @@  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;
+  while (--i >= 0);
   reg_base_reg[regno] = -1;
   reg_mode[regno] = GET_MODE (reg);
   reg_symbol_ref[regno] = sym;
@@ -1890,7 +1904,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 +2038,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