Quoting Eric Botcazou <ebotca...@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.
2013-05-22  Joern Rennecke <joern.renne...@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

Reply via email to