* RFA: fix rtl-optimization/56833
@ 2013-05-15 15:00 Joern Rennecke
2013-05-22 9:03 ` Eric Botcazou
0 siblings, 1 reply; 11+ messages in thread
From: Joern Rennecke @ 2013-05-15 15:00 UTC (permalink / raw)
To: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 468 bytes --]
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
[-- Attachment #2: pr56833-patch --]
[-- Type: text/plain, Size: 3002 bytes --]
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.
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: RFA: fix rtl-optimization/56833
2013-05-15 15:00 RFA: fix rtl-optimization/56833 Joern Rennecke
@ 2013-05-22 9:03 ` Eric Botcazou
2013-05-22 10:45 ` Joern Rennecke
0 siblings, 1 reply; 11+ messages in thread
From: Eric Botcazou @ 2013-05-22 9:03 UTC (permalink / raw)
To: Joern Rennecke; +Cc: gcc-patches
> 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?
--
Eric Botcazou
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: RFA: fix rtl-optimization/56833
2013-05-22 9:03 ` Eric Botcazou
@ 2013-05-22 10:45 ` Joern Rennecke
2013-05-23 15:00 ` Eric Botcazou
0 siblings, 1 reply; 11+ messages in thread
From: Joern Rennecke @ 2013-05-22 10:45 UTC (permalink / raw)
To: Eric Botcazou; +Cc: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 1810 bytes --]
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.
[-- Attachment #2: pr56833-patch-2 --]
[-- Type: text/plain, Size: 3131 bytes --]
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: RFA: fix rtl-optimization/56833
2013-05-22 10:45 ` Joern Rennecke
@ 2013-05-23 15:00 ` Eric Botcazou
2013-05-23 20:33 ` Joern Rennecke
0 siblings, 1 reply; 11+ messages in thread
From: Eric Botcazou @ 2013-05-23 15:00 UTC (permalink / raw)
To: Joern Rennecke; +Cc: gcc-patches
> 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.
Agreed.
> 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.
The patch is OK on principle, but can you factor out the common code? The
endings of move2add_use_add2_insn and move2add_use_add3_insn are identical so
it would be nice to have e.g. a record_reg_value helper function and call it
from there. Similarly, the 3 new checks look strictly identical.
--
Eric Botcazou
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: RFA: fix rtl-optimization/56833
2013-05-23 15:00 ` Eric Botcazou
@ 2013-05-23 20:33 ` Joern Rennecke
2013-05-24 0:43 ` SUBREGs in move2add_note_store (Was: Re: RFA: fix rtl-optimization/56833) Joern Rennecke
0 siblings, 1 reply; 11+ messages in thread
From: Joern Rennecke @ 2013-05-23 20:33 UTC (permalink / raw)
To: Eric Botcazou; +Cc: gcc-patches
Quoting Eric Botcazou <ebotcazou@adacore.com>:
> The patch is OK on principle, but can you factor out the common code? The
> endings of move2add_use_add2_insn and move2add_use_add3_insn are identical so
> it would be nice to have e.g. a record_reg_value helper function and call it
> from there. Similarly, the 3 new checks look strictly identical.
Looking into sharing the code with sites that perform essentially the same
function but look somewhat different, I see there's a problem with using
only reg_set_luid to indicate the consistency of a multi-hard-reg-value
in these other contexts.
For values that are use a base register, the reg_set_luid is the same as
for the base register; for constants, it is the same for all constants
set since the last label.
Say we have reg size 8 bit, base r0, and then
(set reg:HI 2 (plus:SI (reg:HI 0) (const_int 500))
...
(set reg:HI 3 (plus:SI (reg:HI 0) (const_int 500))
Now how do we tell that the value in r2 is no longer valid?
As the example shows, trying to replicate the recorded value across all hard
regs is pointless, as we still need to make sure that we have a still-valid
start register.
OTOH, this ties in nicely with setting the mode of subsequent registers
to VOIDmode. We can verify the mode to make sure there was no more recent
set of any constituent register. The check of the extra luids thus becomes
superflous, as becomes the set.
This logic relies on multi-hard register regs to be allocated contigously...
But if we'd want to change that, there'd be a lot more code that would
need changing.
^ permalink raw reply [flat|nested] 11+ messages in thread
* SUBREGs in move2add_note_store (Was: Re: RFA: fix rtl-optimization/56833)
2013-05-23 20:33 ` Joern Rennecke
@ 2013-05-24 0:43 ` Joern Rennecke
2013-05-24 7:57 ` Eric Botcazou
0 siblings, 1 reply; 11+ messages in thread
From: Joern Rennecke @ 2013-05-24 0:43 UTC (permalink / raw)
To: Alexandre Oliva; +Cc: Eric Botcazou, gcc-patches, Kaz Kojima
[-- Attachment #1: Type: text/plain, Size: 2160 bytes --]
Quoting Joern Rennecke <joern.rennecke@embecosm.com>:
> Looking into sharing the code with sites that perform essentially the same
> function but look somewhat different, I see there's a problem with using
> only reg_set_luid to indicate the consistency of a multi-hard-reg-value
> in these other contexts.
> For values that are use a base register, the reg_set_luid is the same as
> for the base register; for constants, it is the same for all constants
> set since the last label.
>
> Say we have reg size 8 bit, base r0, and then
> (set reg:HI 2 (plus:SI (reg:HI 0) (const_int 500))
> ...
> (set reg:HI 3 (plus:SI (reg:HI 0) (const_int 500))
>
> Now how do we tell that the value in r2 is no longer valid?
>
> As the example shows, trying to replicate the recorded value across all hard
> regs is pointless, as we still need to make sure that we have a still-valid
> start register.
> OTOH, this ties in nicely with setting the mode of subsequent registers
> to VOIDmode. We can verify the mode to make sure there was no more recent
> set of any constituent register. The check of the extra luids thus becomes
> superfluous, as becomes the set.
I've found it is good to have also one mode to invalidate a register for
all uses; it seems natural to use VOIDmode for that, and then we can use
BLKmode for all but the first hard register of a multi-hard-reg register.
However, digging into the code that can use some factoring out of mode setting
and validity testing, I stumbled over the destination handling in
move2add_note_store in the case of SUBREGs.
The original code was just coded or minimal effort to find the actual regno
of a REG or SUBREG (this only made sense with the old word-based SUBREG
scheme).
Alexandre, AFAICT, you have misunderstood this bit to make the function
pretend that the inner part of the subreg is the actual destination.
Or is there some obscure reason to have the mode checks worry about the
inside of the SUBREG?
I have attached the patch that makes the reload_cse_move2add sub-pass work
the way it think it should be; it'll take some time to test this properly,
though.
[-- Attachment #2: pr56833-patch-3 --]
[-- Type: text/plain, Size: 13680 bytes --]
2013-05-24 Joern Rennecke <joern.rennecke@embecosm.com>
* postreload.c (move2add_record_mode): New function.
(move2add_record_sym_value, move2add_valid_value_p): Likewise.
(move2add_use_add2_insn): Use move2add_record_sym_value.
(move2add_use_add3_insn): Likewise.
(reload_cse_move2add): Use move2add_valid_value_p and
move2add_record_mode. Invalidate call-clobbered regs
by setting reg_mode to VOIDmode.
(move2add_note_store): Don't pretend the inside of a SUBREG is
the actual destination. Invalidate PRE_INC / POST_INC
registers my setting reg_mode to VOIDmode.
Use move2add_record_sym_value, move2add_valid_value_p and
move2add_record_mode.
Index: postreload.c
===================================================================
--- postreload.c (revision 199190)
+++ postreload.c (working copy)
@@ -1645,14 +1645,22 @@ reload_combine_note_use (rtx *xp, rtx in
later disable any optimization that would cross it.
reg_offset[n] / reg_base_reg[n] / reg_symbol_ref[n] / reg_mode[n]
are only valid if reg_set_luid[n] is greater than
- move2add_last_label_luid. */
+ move2add_last_label_luid.
+ For a set that established a new (potential) base register with
+ non-constant value, we use move2add_luid from the place where the
+ setting insn is encountered; registers based off that base then
+ get the same reg_set_luid. Constants all get
+ move2add_last_label_luid + 1 as their reg_set_luid. */
static int reg_set_luid[FIRST_PSEUDO_REGISTER];
/* If reg_base_reg[n] is negative, register n has been set to
reg_offset[n] or reg_symbol_ref[n] + reg_offset[n] in mode reg_mode[n].
If reg_base_reg[n] is non-negative, register n has been set to the
sum of reg_offset[n] and the value of register reg_base_reg[n]
- before reg_set_luid[n], calculated in mode reg_mode[n] . */
+ before reg_set_luid[n], calculated in mode reg_mode[n] .
+ For multi-hard-register registers, all but the first one are
+ recorded as BLKmode in reg_mode. Setting reg_mode to VOIDmode
+ marks it as invalid. */
static HOST_WIDE_INT reg_offset[FIRST_PSEUDO_REGISTER];
static int reg_base_reg[FIRST_PSEUDO_REGISTER];
static rtx reg_symbol_ref[FIRST_PSEUDO_REGISTER];
@@ -1674,6 +1682,63 @@ #define MODES_OK_FOR_MOVE2ADD(OUTMODE, I
|| (GET_MODE_SIZE (OUTMODE) <= GET_MODE_SIZE (INMODE) \
&& TRULY_NOOP_TRUNCATION_MODES_P (OUTMODE, INMODE)))
+/* Record that REG is being set to a value with the mode of REG. */
+
+static void
+move2add_record_mode (rtx reg)
+{
+ int regno, nregs;
+ enum machine_mode mode = GET_MODE (reg);
+ int i;
+
+ if (GET_CODE (reg) == SUBREG)
+ {
+ regno = subreg_regno (reg);
+ nregs = subreg_nregs (reg);
+ }
+ else if (REG_P (reg))
+ {
+ regno = REGNO (reg);
+ nregs = hard_regno_nregs[regno][mode];
+ }
+ else
+ gcc_unreachable ();
+ for (i = nregs; --i > 0; )
+ reg_mode[regno + i] = BLKmode;
+ reg_mode[regno] = mode;
+}
+
+/* Record that REG is being set to the sum of SYM and OFF. */
+
+static void
+move2add_record_sym_value (rtx reg, rtx sym, rtx off)
+{
+ int regno = REGNO (reg);
+
+ move2add_record_mode (reg);
+ reg_set_luid[regno] = move2add_luid;
+ reg_base_reg[regno] = -1;
+ reg_symbol_ref[regno] = sym;
+ reg_offset[regno] = INTVAL (off);
+}
+
+/* Check if REGNO contains a valid value in MODE. */
+
+static bool
+move2add_valid_value_p (int regno, enum machine_mode mode)
+{
+ int i;
+
+ if (reg_set_luid[regno] <= move2add_last_label_luid
+ || !MODES_OK_FOR_MOVE2ADD (mode, reg_mode[regno]))
+ return false;
+
+ for (i = hard_regno_nregs[regno][mode] - 1; i > 0; i--)
+ if (reg_mode[i] != BLKmode)
+ return false;
+ return true;
+}
+
/* This function is called with INSN that sets REG to (SYM + OFF),
while REG is known to already have value (SYM + offset).
This function tries to change INSN into an add instruction
@@ -1749,11 +1814,7 @@ move2add_use_add2_insn (rtx reg, rtx sym
}
}
}
- reg_set_luid[regno] = move2add_luid;
- reg_base_reg[regno] = -1;
- reg_mode[regno] = GET_MODE (reg);
- reg_symbol_ref[regno] = sym;
- reg_offset[regno] = INTVAL (off);
+ move2add_record_sym_value (reg, sym, off);
return changed;
}
@@ -1787,8 +1848,7 @@ move2add_use_add3_insn (rtx reg, rtx sym
SET_SRC (pat) = plus_expr;
for (i = 0; i < FIRST_PSEUDO_REGISTER; i++)
- if (reg_set_luid[i] > move2add_last_label_luid
- && reg_mode[i] == GET_MODE (reg)
+ if (move2add_valid_value_p (i, GET_MODE (reg))
&& reg_base_reg[i] < 0
&& reg_symbol_ref[i] != NULL_RTX
&& rtx_equal_p (sym, reg_symbol_ref[i]))
@@ -1836,10 +1896,7 @@ move2add_use_add3_insn (rtx reg, rtx sym
changed = true;
}
reg_set_luid[regno] = move2add_luid;
- reg_base_reg[regno] = -1;
- reg_mode[regno] = GET_MODE (reg);
- reg_symbol_ref[regno] = sym;
- reg_offset[regno] = INTVAL (off);
+ move2add_record_sym_value (reg, sym, off);
return changed;
}
@@ -1890,8 +1947,7 @@ 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
- && MODES_OK_FOR_MOVE2ADD (GET_MODE (reg), reg_mode[regno])
+ if (move2add_valid_value_p (regno, GET_MODE (reg))
&& dbg_cnt (cse2_move2add))
{
/* Try to transform (set (REGX) (CONST_INT A))
@@ -1928,8 +1984,7 @@ reload_cse_move2add (rtx first)
else if (REG_P (src)
&& reg_set_luid[regno] == reg_set_luid[REGNO (src)]
&& reg_base_reg[regno] == reg_base_reg[REGNO (src)]
- && MODES_OK_FOR_MOVE2ADD (GET_MODE (reg),
- reg_mode[REGNO (src)]))
+ && move2add_valid_value_p (REGNO (src), GET_MODE (reg)))
{
rtx next = next_nonnote_nondebug_insn (insn);
rtx set = NULL_RTX;
@@ -1982,10 +2037,10 @@ reload_cse_move2add (rtx first)
delete_insn (insn);
changed |= success;
insn = next;
- reg_mode[regno] = GET_MODE (reg);
- reg_offset[regno] =
- trunc_int_for_mode (added_offset + base_offset,
- GET_MODE (reg));
+ move2add_record_mode (reg);
+ reg_offset[regno]
+ = trunc_int_for_mode (added_offset + base_offset,
+ GET_MODE (reg));
continue;
}
}
@@ -2021,8 +2076,7 @@ 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
- && MODES_OK_FOR_MOVE2ADD (GET_MODE (reg), reg_mode[regno])
+ if (move2add_valid_value_p (regno, GET_MODE (reg))
&& reg_base_reg[regno] < 0
&& reg_symbol_ref[regno] != NULL_RTX
&& rtx_equal_p (sym, reg_symbol_ref[regno]))
@@ -2045,7 +2099,10 @@ reload_cse_move2add (rtx first)
/* Reset the information about this register. */
int regno = REGNO (XEXP (note, 0));
if (regno < FIRST_PSEUDO_REGISTER)
- reg_set_luid[regno] = 0;
+ {
+ move2add_record_mode (XEXP (note, 0));
+ reg_set_luid[regno] = 0;
+ }
}
}
note_stores (PATTERN (insn), move2add_note_store, insn);
@@ -2082,7 +2139,7 @@ reload_cse_move2add (rtx first)
{
if (call_used_regs[i])
/* Reset the information about this register. */
- reg_set_luid[i] = 0;
+ reg_mode[i] = VOIDmode;
}
}
}
@@ -2099,20 +2156,8 @@ move2add_note_store (rtx dst, const_rtx
{
rtx insn = (rtx) data;
unsigned int regno = 0;
- unsigned int nregs = 0;
- unsigned int i;
enum machine_mode mode = GET_MODE (dst);
- if (GET_CODE (dst) == SUBREG)
- {
- regno = subreg_regno_offset (REGNO (SUBREG_REG (dst)),
- GET_MODE (SUBREG_REG (dst)),
- SUBREG_BYTE (dst),
- GET_MODE (dst));
- nregs = subreg_nregs (dst);
- dst = SUBREG_REG (dst);
- }
-
/* Some targets do argument pushes without adding REG_INC notes. */
if (MEM_P (dst))
@@ -2120,27 +2165,28 @@ move2add_note_store (rtx dst, const_rtx
dst = XEXP (dst, 0);
if (GET_CODE (dst) == PRE_INC || GET_CODE (dst) == POST_INC
|| GET_CODE (dst) == PRE_DEC || GET_CODE (dst) == POST_DEC)
- reg_set_luid[REGNO (XEXP (dst, 0))] = 0;
+ reg_mode[REGNO (XEXP (dst, 0))] = VOIDmode;
return;
}
- if (!REG_P (dst))
- return;
- regno += REGNO (dst);
- if (!nregs)
- nregs = hard_regno_nregs[regno][mode];
+ if (GET_CODE (dst) == SUBREG)
+ regno = subreg_regno (dst);
+ else if (REG_P (dst))
+ regno = REGNO (dst);
+ else
+ return;
- if (SCALAR_INT_MODE_P (GET_MODE (dst))
- && nregs == 1 && GET_CODE (set) == SET)
+ if (SCALAR_INT_MODE_P (mode)
+ && GET_CODE (set) == SET)
{
rtx note, sym = NULL_RTX;
- HOST_WIDE_INT off;
+ rtx off;
note = find_reg_equal_equiv_note (insn);
if (note && GET_CODE (XEXP (note, 0)) == SYMBOL_REF)
{
sym = XEXP (note, 0);
- off = 0;
+ off = const0_rtx;
}
else if (note && GET_CODE (XEXP (note, 0)) == CONST
&& GET_CODE (XEXP (XEXP (note, 0), 0)) == PLUS
@@ -2148,22 +2194,18 @@ move2add_note_store (rtx dst, const_rtx
&& CONST_INT_P (XEXP (XEXP (XEXP (note, 0), 0), 1)))
{
sym = XEXP (XEXP (XEXP (note, 0), 0), 0);
- off = INTVAL (XEXP (XEXP (XEXP (note, 0), 0), 1));
+ off = XEXP (XEXP (XEXP (note, 0), 0), 1);
}
if (sym != NULL_RTX)
{
- reg_base_reg[regno] = -1;
- reg_symbol_ref[regno] = sym;
- reg_offset[regno] = off;
- reg_mode[regno] = mode;
- reg_set_luid[regno] = move2add_luid;
+ move2add_record_sym_value (dst, sym, off);
return;
}
}
- if (SCALAR_INT_MODE_P (GET_MODE (dst))
- && nregs == 1 && GET_CODE (set) == SET
+ if (SCALAR_INT_MODE_P (mode)
+ && GET_CODE (set) == SET
&& GET_CODE (SET_DEST (set)) != ZERO_EXTRACT
&& GET_CODE (SET_DEST (set)) != STRICT_LOW_PART)
{
@@ -2171,9 +2213,6 @@ move2add_note_store (rtx dst, const_rtx
rtx base_reg;
HOST_WIDE_INT offset;
int base_regno;
- /* This may be different from mode, if SET_DEST (set) is a
- SUBREG. */
- enum machine_mode dst_mode = GET_MODE (dst);
switch (GET_CODE (src))
{
@@ -2185,20 +2224,14 @@ move2add_note_store (rtx dst, const_rtx
if (CONST_INT_P (XEXP (src, 1)))
offset = INTVAL (XEXP (src, 1));
else if (REG_P (XEXP (src, 1))
- && (reg_set_luid[REGNO (XEXP (src, 1))]
- > move2add_last_label_luid)
- && (MODES_OK_FOR_MOVE2ADD
- (dst_mode, reg_mode[REGNO (XEXP (src, 1))])))
+ && move2add_valid_value_p (REGNO (XEXP (src, 1)), mode))
{
if (reg_base_reg[REGNO (XEXP (src, 1))] < 0
&& reg_symbol_ref[REGNO (XEXP (src, 1))] == NULL_RTX)
offset = reg_offset[REGNO (XEXP (src, 1))];
/* Maybe the first register is known to be a
constant. */
- else if (reg_set_luid[REGNO (base_reg)]
- > move2add_last_label_luid
- && (MODES_OK_FOR_MOVE2ADD
- (dst_mode, reg_mode[REGNO (base_reg)]))
+ else if (move2add_valid_value_p (REGNO (base_reg), mode)
&& reg_base_reg[REGNO (base_reg)] < 0
&& reg_symbol_ref[REGNO (base_reg)] == NULL_RTX)
{
@@ -2228,33 +2261,26 @@ move2add_note_store (rtx dst, const_rtx
reg_offset[regno] = INTVAL (SET_SRC (set));
/* We assign the same luid to all registers set to constants. */
reg_set_luid[regno] = move2add_last_label_luid + 1;
- reg_mode[regno] = mode;
+ move2add_record_mode (dst);
return;
default:
- invalidate:
- /* Invalidate the contents of the register. */
- reg_set_luid[regno] = 0;
- return;
+ goto invalidate;
}
base_regno = REGNO (base_reg);
/* If information about the base register is not valid, set it
up as a new base register, pretending its value is known
starting from the current insn. */
- if (reg_set_luid[base_regno] <= move2add_last_label_luid)
+ if (!move2add_valid_value_p (base_regno, mode))
{
reg_base_reg[base_regno] = base_regno;
reg_symbol_ref[base_regno] = NULL_RTX;
reg_offset[base_regno] = 0;
reg_set_luid[base_regno] = move2add_luid;
- reg_mode[base_regno] = mode;
+ gcc_assert (GET_MODE (base_reg) == mode);
+ move2add_record_mode (base_reg);
}
- else if (! MODES_OK_FOR_MOVE2ADD (dst_mode,
- reg_mode[base_regno]))
- goto invalidate;
-
- reg_mode[regno] = mode;
/* Copy base information from our base register. */
reg_set_luid[regno] = reg_set_luid[base_regno];
@@ -2262,17 +2288,17 @@ move2add_note_store (rtx dst, const_rtx
reg_symbol_ref[regno] = reg_symbol_ref[base_regno];
/* Compute the sum of the offsets or constants. */
- reg_offset[regno] = trunc_int_for_mode (offset
- + reg_offset[base_regno],
- dst_mode);
+ reg_offset[regno]
+ = trunc_int_for_mode (offset + reg_offset[base_regno], mode);
+
+ move2add_record_mode (dst);
}
else
{
- unsigned int endregno = regno + nregs;
-
- for (i = regno; i < endregno; i++)
- /* Reset the information about this register. */
- reg_set_luid[i] = 0;
+ invalidate:
+ /* Invalidate the contents of the register. */
+ reg_set_luid[regno] = 0;
+ move2add_record_mode (dst);
}
}
\f
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: SUBREGs in move2add_note_store (Was: Re: RFA: fix rtl-optimization/56833)
2013-05-24 0:43 ` SUBREGs in move2add_note_store (Was: Re: RFA: fix rtl-optimization/56833) Joern Rennecke
@ 2013-05-24 7:57 ` Eric Botcazou
2013-05-24 8:42 ` Joern Rennecke
2013-05-24 16:33 ` RFA: fix rtl-optimization/56833 Joern Rennecke
0 siblings, 2 replies; 11+ messages in thread
From: Eric Botcazou @ 2013-05-24 7:57 UTC (permalink / raw)
To: Joern Rennecke; +Cc: gcc-patches, Alexandre Oliva, Kaz Kojima
> I've found it is good to have also one mode to invalidate a register for
> all uses; it seems natural to use VOIDmode for that, and then we can use
> BLKmode for all but the first hard register of a multi-hard-reg register.
OK, that sounds sensible.
> I have attached the patch that makes the reload_cse_move2add sub-pass work
> the way it think it should be; it'll take some time to test this properly,
> though.
Thanks for your efforts. Here's a preliminary review:
+/* Record that REG is being set to a value with the mode of REG. */
+
+static void
+move2add_record_mode (rtx reg)
[...]
+ for (i = nregs; --i > 0; )
+ reg_mode[regno + i] = BLKmode;
+/* Check if REGNO contains a valid value in MODE. */
+
+static bool
+move2add_valid_value_p (int regno, enum machine_mode mode)
[...]
+ for (i = hard_regno_nregs[regno][mode] - 1; i > 0; i--)
+ if (reg_mode[i] != BLKmode)
+ return false;
I think that a 'regno' is missing in the second hunk. And can you use the
same idiom for the loop in both cases (same initial value and iteration)?
@@ -1787,8 +1848,7 @@ move2add_use_add3_insn (rtx reg, rtx sym
SET_SRC (pat) = plus_expr;
for (i = 0; i < FIRST_PSEUDO_REGISTER; i++)
- if (reg_set_luid[i] > move2add_last_label_luid
- && reg_mode[i] == GET_MODE (reg)
+ if (move2add_valid_value_p (i, GET_MODE (reg))
&& reg_base_reg[i] < 0
&& reg_symbol_ref[i] != NULL_RTX
&& rtx_equal_p (sym, reg_symbol_ref[i]))
Note that you're weakening the mode equality test here.
@@ -2045,7 +2099,10 @@ reload_cse_move2add (rtx first)
/* Reset the information about this register. */
int regno = REGNO (XEXP (note, 0));
if (regno < FIRST_PSEUDO_REGISTER)
- reg_set_luid[regno] = 0;
+ {
+ move2add_record_mode (XEXP (note, 0));
+ reg_set_luid[regno] = 0;
+ }
}
}
note_stores (PATTERN (insn), move2add_note_store, insn);
@@ -2082,7 +2139,7 @@ reload_cse_move2add (rtx first)
{
if (call_used_regs[i])
/* Reset the information about this register. */
- reg_set_luid[i] = 0;
+ reg_mode[i] = VOIDmode;
}
}
@@ -2262,17 +2288,17 @@ move2add_note_store (rtx dst, const_rtx
[...]
+ invalidate:
+ /* Invalidate the contents of the register. */
+ reg_set_luid[regno] = 0;
+ move2add_record_mode (dst);
The intent is the same, why don't we have the same code?
--
Eric Botcazou
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: SUBREGs in move2add_note_store (Was: Re: RFA: fix rtl-optimization/56833)
2013-05-24 7:57 ` Eric Botcazou
@ 2013-05-24 8:42 ` Joern Rennecke
2013-05-24 16:33 ` RFA: fix rtl-optimization/56833 Joern Rennecke
1 sibling, 0 replies; 11+ messages in thread
From: Joern Rennecke @ 2013-05-24 8:42 UTC (permalink / raw)
To: Eric Botcazou; +Cc: gcc-patches, Alexandre Oliva, Kaz Kojima
Quoting Eric Botcazou <ebotcazou@adacore.com>:
> +static bool
> +move2add_valid_value_p (int regno, enum machine_mode mode)
> [...]
> + for (i = hard_regno_nregs[regno][mode] - 1; i > 0; i--)
> + if (reg_mode[i] != BLKmode)
> + return false;
>
> I think that a 'regno' is missing in the second hunk.
Huh? Could you be more specific?
If you mean I should check reg_mode[regno], that is tested a few lines above.
> And can you use the
> same idiom for the loop in both cases (same initial value and iteration)?
Sure. I rembered that Jeff prefered .. - 1; i > 0; i--), but forgot I had
a second instance of ; --i > 0; ).
> @@ -1787,8 +1848,7 @@ move2add_use_add3_insn (rtx reg, rtx sym
> SET_SRC (pat) = plus_expr;
>
> for (i = 0; i < FIRST_PSEUDO_REGISTER; i++)
> - if (reg_set_luid[i] > move2add_last_label_luid
> - && reg_mode[i] == GET_MODE (reg)
> + if (move2add_valid_value_p (i, GET_MODE (reg))
> && reg_base_reg[i] < 0
> && reg_symbol_ref[i] != NULL_RTX
> && rtx_equal_p (sym, reg_symbol_ref[i]))
>
> Note that you're weakening the mode equality test here.
Weakening inasmuch as it allows noop truncation, strengthening inasmuch
as it checks that all part of a multi-hard-reg reg are up to date
(which is necessary because I dropped some checks to only allow single
regs). Overall, it makes the logic more consistent.
> @@ -2045,7 +2099,10 @@ reload_cse_move2add (rtx first)
> /* Reset the information about this register. */
> int regno = REGNO (XEXP (note, 0));
> if (regno < FIRST_PSEUDO_REGISTER)
> - reg_set_luid[regno] = 0;
> + {
> + move2add_record_mode (XEXP (note, 0));
> + reg_set_luid[regno] = 0;
> + }
> }
> }
> note_stores (PATTERN (insn), move2add_note_store, insn);
See comment on third hunk of this set below
> @@ -2082,7 +2139,7 @@ reload_cse_move2add (rtx first)
> {
> if (call_used_regs[i])
> /* Reset the information about this register. */
> - reg_set_luid[i] = 0;
> + reg_mode[i] = VOIDmode;
This is for a single hard register.
Setting the mode to VOIDmode invalidates all uses.
> @@ -2262,17 +2288,17 @@ move2add_note_store (rtx dst, const_rtx
> [...]
>
> + invalidate:
> + /* Invalidate the contents of the register. */
> + reg_set_luid[regno] = 0;
> + move2add_record_mode (dst);
>
> The intent is the same, why don't we have the same code?
move2add_record_mode (dst) invalidates all but the first reg.
I left the invalidation of the first reg set reg_set_luid on the theory
that this causes less churn (albeit the code is moved, too).
Now, on second thought, I should really use reg_mode[regno] VOIDmode
for the first reg here instead, to make sure this reg ceases to be
tracked as part of a multi-hard-reg register starting at an earlier regnum.###
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: RFA: fix rtl-optimization/56833
2013-05-24 7:57 ` Eric Botcazou
2013-05-24 8:42 ` Joern Rennecke
@ 2013-05-24 16:33 ` Joern Rennecke
2013-05-27 7:30 ` Eric Botcazou
2013-05-28 9:35 ` Andreas Schwab
1 sibling, 2 replies; 11+ messages in thread
From: Joern Rennecke @ 2013-05-24 16:33 UTC (permalink / raw)
To: Eric Botcazou; +Cc: gcc-patches, Alexandre Oliva, Kaz Kojima
[-- Attachment #1: Type: text/plain, Size: 218 bytes --]
Please find attached the updated patch.
bootstrapped / regtested for i686-pc-linux-gnu
regtested for i686-pc-linux-gnu X sh-elf
regtested in gcc 4.8 branch for i686-pc-linux-gnu X avr
(--target-board atmega128-sim)
[-- Attachment #2: pr56833-patch-4 --]
[-- Type: text/plain, Size: 13736 bytes --]
2013-05-24 Joern Rennecke <joern.rennecke@embecosm.com>
PR rtl-optimization/56833
* postreload.c (move2add_record_mode): New function.
(move2add_record_sym_value, move2add_valid_value_p): Likewise.
(move2add_use_add2_insn): Use move2add_record_sym_value.
(move2add_use_add3_insn): Likewise.
(reload_cse_move2add): Use move2add_valid_value_p and
move2add_record_mode. Invalidate call-clobbered and REG_INC
affected regs by setting reg_mode to VOIDmode.
(move2add_note_store): Don't pretend the inside of a SUBREG is
the actual destination. Invalidate single/leading registers by
setting reg_mode to VOIDmode.
Use move2add_record_sym_value, move2add_valid_value_p and
move2add_record_mode.
Index: postreload.c
===================================================================
--- postreload.c (revision 199190)
+++ postreload.c (working copy)
@@ -1645,14 +1645,22 @@ reload_combine_note_use (rtx *xp, rtx in
later disable any optimization that would cross it.
reg_offset[n] / reg_base_reg[n] / reg_symbol_ref[n] / reg_mode[n]
are only valid if reg_set_luid[n] is greater than
- move2add_last_label_luid. */
+ move2add_last_label_luid.
+ For a set that established a new (potential) base register with
+ non-constant value, we use move2add_luid from the place where the
+ setting insn is encountered; registers based off that base then
+ get the same reg_set_luid. Constants all get
+ move2add_last_label_luid + 1 as their reg_set_luid. */
static int reg_set_luid[FIRST_PSEUDO_REGISTER];
/* If reg_base_reg[n] is negative, register n has been set to
reg_offset[n] or reg_symbol_ref[n] + reg_offset[n] in mode reg_mode[n].
If reg_base_reg[n] is non-negative, register n has been set to the
sum of reg_offset[n] and the value of register reg_base_reg[n]
- before reg_set_luid[n], calculated in mode reg_mode[n] . */
+ before reg_set_luid[n], calculated in mode reg_mode[n] .
+ For multi-hard-register registers, all but the first one are
+ recorded as BLKmode in reg_mode. Setting reg_mode to VOIDmode
+ marks it as invalid. */
static HOST_WIDE_INT reg_offset[FIRST_PSEUDO_REGISTER];
static int reg_base_reg[FIRST_PSEUDO_REGISTER];
static rtx reg_symbol_ref[FIRST_PSEUDO_REGISTER];
@@ -1674,6 +1682,63 @@ #define MODES_OK_FOR_MOVE2ADD(OUTMODE, I
|| (GET_MODE_SIZE (OUTMODE) <= GET_MODE_SIZE (INMODE) \
&& TRULY_NOOP_TRUNCATION_MODES_P (OUTMODE, INMODE)))
+/* Record that REG is being set to a value with the mode of REG. */
+
+static void
+move2add_record_mode (rtx reg)
+{
+ int regno, nregs;
+ enum machine_mode mode = GET_MODE (reg);
+ int i;
+
+ if (GET_CODE (reg) == SUBREG)
+ {
+ regno = subreg_regno (reg);
+ nregs = subreg_nregs (reg);
+ }
+ else if (REG_P (reg))
+ {
+ regno = REGNO (reg);
+ nregs = hard_regno_nregs[regno][mode];
+ }
+ else
+ gcc_unreachable ();
+ for (i = nregs - 1; i > 0; i--)
+ reg_mode[regno + i] = BLKmode;
+ reg_mode[regno] = mode;
+}
+
+/* Record that REG is being set to the sum of SYM and OFF. */
+
+static void
+move2add_record_sym_value (rtx reg, rtx sym, rtx off)
+{
+ int regno = REGNO (reg);
+
+ move2add_record_mode (reg);
+ reg_set_luid[regno] = move2add_luid;
+ reg_base_reg[regno] = -1;
+ reg_symbol_ref[regno] = sym;
+ reg_offset[regno] = INTVAL (off);
+}
+
+/* Check if REGNO contains a valid value in MODE. */
+
+static bool
+move2add_valid_value_p (int regno, enum machine_mode mode)
+{
+ int i;
+
+ if (reg_set_luid[regno] <= move2add_last_label_luid
+ || !MODES_OK_FOR_MOVE2ADD (mode, reg_mode[regno]))
+ return false;
+
+ for (i = hard_regno_nregs[regno][mode] - 1; i > 0; i--)
+ if (reg_mode[i] != BLKmode)
+ return false;
+ return true;
+}
+
/* This function is called with INSN that sets REG to (SYM + OFF),
while REG is known to already have value (SYM + offset).
This function tries to change INSN into an add instruction
@@ -1749,11 +1814,7 @@ move2add_use_add2_insn (rtx reg, rtx sym
}
}
}
- reg_set_luid[regno] = move2add_luid;
- reg_base_reg[regno] = -1;
- reg_mode[regno] = GET_MODE (reg);
- reg_symbol_ref[regno] = sym;
- reg_offset[regno] = INTVAL (off);
+ move2add_record_sym_value (reg, sym, off);
return changed;
}
@@ -1787,8 +1848,7 @@ move2add_use_add3_insn (rtx reg, rtx sym
SET_SRC (pat) = plus_expr;
for (i = 0; i < FIRST_PSEUDO_REGISTER; i++)
- if (reg_set_luid[i] > move2add_last_label_luid
- && reg_mode[i] == GET_MODE (reg)
+ if (move2add_valid_value_p (i, GET_MODE (reg))
&& reg_base_reg[i] < 0
&& reg_symbol_ref[i] != NULL_RTX
&& rtx_equal_p (sym, reg_symbol_ref[i]))
@@ -1836,10 +1896,7 @@ move2add_use_add3_insn (rtx reg, rtx sym
changed = true;
}
reg_set_luid[regno] = move2add_luid;
- reg_base_reg[regno] = -1;
- reg_mode[regno] = GET_MODE (reg);
- reg_symbol_ref[regno] = sym;
- reg_offset[regno] = INTVAL (off);
+ move2add_record_sym_value (reg, sym, off);
return changed;
}
@@ -1890,8 +1947,7 @@ 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
- && MODES_OK_FOR_MOVE2ADD (GET_MODE (reg), reg_mode[regno])
+ if (move2add_valid_value_p (regno, GET_MODE (reg))
&& dbg_cnt (cse2_move2add))
{
/* Try to transform (set (REGX) (CONST_INT A))
@@ -1928,8 +1984,7 @@ reload_cse_move2add (rtx first)
else if (REG_P (src)
&& reg_set_luid[regno] == reg_set_luid[REGNO (src)]
&& reg_base_reg[regno] == reg_base_reg[REGNO (src)]
- && MODES_OK_FOR_MOVE2ADD (GET_MODE (reg),
- reg_mode[REGNO (src)]))
+ && move2add_valid_value_p (REGNO (src), GET_MODE (reg)))
{
rtx next = next_nonnote_nondebug_insn (insn);
rtx set = NULL_RTX;
@@ -1982,10 +2037,10 @@ reload_cse_move2add (rtx first)
delete_insn (insn);
changed |= success;
insn = next;
- reg_mode[regno] = GET_MODE (reg);
- reg_offset[regno] =
- trunc_int_for_mode (added_offset + base_offset,
- GET_MODE (reg));
+ move2add_record_mode (reg);
+ reg_offset[regno]
+ = trunc_int_for_mode (added_offset + base_offset,
+ GET_MODE (reg));
continue;
}
}
@@ -2021,8 +2076,7 @@ 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
- && MODES_OK_FOR_MOVE2ADD (GET_MODE (reg), reg_mode[regno])
+ if (move2add_valid_value_p (regno, GET_MODE (reg))
&& reg_base_reg[regno] < 0
&& reg_symbol_ref[regno] != NULL_RTX
&& rtx_equal_p (sym, reg_symbol_ref[regno]))
@@ -2045,7 +2099,10 @@ reload_cse_move2add (rtx first)
/* Reset the information about this register. */
int regno = REGNO (XEXP (note, 0));
if (regno < FIRST_PSEUDO_REGISTER)
- reg_set_luid[regno] = 0;
+ {
+ move2add_record_mode (XEXP (note, 0));
+ reg_mode[regno] = VOIDmode;
+ }
}
}
note_stores (PATTERN (insn), move2add_note_store, insn);
@@ -2082,7 +2139,7 @@ reload_cse_move2add (rtx first)
{
if (call_used_regs[i])
/* Reset the information about this register. */
- reg_set_luid[i] = 0;
+ reg_mode[i] = VOIDmode;
}
}
}
@@ -2099,20 +2156,8 @@ move2add_note_store (rtx dst, const_rtx
{
rtx insn = (rtx) data;
unsigned int regno = 0;
- unsigned int nregs = 0;
- unsigned int i;
enum machine_mode mode = GET_MODE (dst);
- if (GET_CODE (dst) == SUBREG)
- {
- regno = subreg_regno_offset (REGNO (SUBREG_REG (dst)),
- GET_MODE (SUBREG_REG (dst)),
- SUBREG_BYTE (dst),
- GET_MODE (dst));
- nregs = subreg_nregs (dst);
- dst = SUBREG_REG (dst);
- }
-
/* Some targets do argument pushes without adding REG_INC notes. */
if (MEM_P (dst))
@@ -2120,27 +2165,28 @@ move2add_note_store (rtx dst, const_rtx
dst = XEXP (dst, 0);
if (GET_CODE (dst) == PRE_INC || GET_CODE (dst) == POST_INC
|| GET_CODE (dst) == PRE_DEC || GET_CODE (dst) == POST_DEC)
- reg_set_luid[REGNO (XEXP (dst, 0))] = 0;
+ reg_mode[REGNO (XEXP (dst, 0))] = VOIDmode;
return;
}
- if (!REG_P (dst))
- return;
- regno += REGNO (dst);
- if (!nregs)
- nregs = hard_regno_nregs[regno][mode];
+ if (GET_CODE (dst) == SUBREG)
+ regno = subreg_regno (dst);
+ else if (REG_P (dst))
+ regno = REGNO (dst);
+ else
+ return;
- if (SCALAR_INT_MODE_P (GET_MODE (dst))
- && nregs == 1 && GET_CODE (set) == SET)
+ if (SCALAR_INT_MODE_P (mode)
+ && GET_CODE (set) == SET)
{
rtx note, sym = NULL_RTX;
- HOST_WIDE_INT off;
+ rtx off;
note = find_reg_equal_equiv_note (insn);
if (note && GET_CODE (XEXP (note, 0)) == SYMBOL_REF)
{
sym = XEXP (note, 0);
- off = 0;
+ off = const0_rtx;
}
else if (note && GET_CODE (XEXP (note, 0)) == CONST
&& GET_CODE (XEXP (XEXP (note, 0), 0)) == PLUS
@@ -2148,22 +2194,18 @@ move2add_note_store (rtx dst, const_rtx
&& CONST_INT_P (XEXP (XEXP (XEXP (note, 0), 0), 1)))
{
sym = XEXP (XEXP (XEXP (note, 0), 0), 0);
- off = INTVAL (XEXP (XEXP (XEXP (note, 0), 0), 1));
+ off = XEXP (XEXP (XEXP (note, 0), 0), 1);
}
if (sym != NULL_RTX)
{
- reg_base_reg[regno] = -1;
- reg_symbol_ref[regno] = sym;
- reg_offset[regno] = off;
- reg_mode[regno] = mode;
- reg_set_luid[regno] = move2add_luid;
+ move2add_record_sym_value (dst, sym, off);
return;
}
}
- if (SCALAR_INT_MODE_P (GET_MODE (dst))
- && nregs == 1 && GET_CODE (set) == SET
+ if (SCALAR_INT_MODE_P (mode)
+ && GET_CODE (set) == SET
&& GET_CODE (SET_DEST (set)) != ZERO_EXTRACT
&& GET_CODE (SET_DEST (set)) != STRICT_LOW_PART)
{
@@ -2171,9 +2213,6 @@ move2add_note_store (rtx dst, const_rtx
rtx base_reg;
HOST_WIDE_INT offset;
int base_regno;
- /* This may be different from mode, if SET_DEST (set) is a
- SUBREG. */
- enum machine_mode dst_mode = GET_MODE (dst);
switch (GET_CODE (src))
{
@@ -2185,20 +2224,14 @@ move2add_note_store (rtx dst, const_rtx
if (CONST_INT_P (XEXP (src, 1)))
offset = INTVAL (XEXP (src, 1));
else if (REG_P (XEXP (src, 1))
- && (reg_set_luid[REGNO (XEXP (src, 1))]
- > move2add_last_label_luid)
- && (MODES_OK_FOR_MOVE2ADD
- (dst_mode, reg_mode[REGNO (XEXP (src, 1))])))
+ && move2add_valid_value_p (REGNO (XEXP (src, 1)), mode))
{
if (reg_base_reg[REGNO (XEXP (src, 1))] < 0
&& reg_symbol_ref[REGNO (XEXP (src, 1))] == NULL_RTX)
offset = reg_offset[REGNO (XEXP (src, 1))];
/* Maybe the first register is known to be a
constant. */
- else if (reg_set_luid[REGNO (base_reg)]
- > move2add_last_label_luid
- && (MODES_OK_FOR_MOVE2ADD
- (dst_mode, reg_mode[REGNO (base_reg)]))
+ else if (move2add_valid_value_p (REGNO (base_reg), mode)
&& reg_base_reg[REGNO (base_reg)] < 0
&& reg_symbol_ref[REGNO (base_reg)] == NULL_RTX)
{
@@ -2228,33 +2261,26 @@ move2add_note_store (rtx dst, const_rtx
reg_offset[regno] = INTVAL (SET_SRC (set));
/* We assign the same luid to all registers set to constants. */
reg_set_luid[regno] = move2add_last_label_luid + 1;
- reg_mode[regno] = mode;
+ move2add_record_mode (dst);
return;
default:
- invalidate:
- /* Invalidate the contents of the register. */
- reg_set_luid[regno] = 0;
- return;
+ goto invalidate;
}
base_regno = REGNO (base_reg);
/* If information about the base register is not valid, set it
up as a new base register, pretending its value is known
starting from the current insn. */
- if (reg_set_luid[base_regno] <= move2add_last_label_luid)
+ if (!move2add_valid_value_p (base_regno, mode))
{
reg_base_reg[base_regno] = base_regno;
reg_symbol_ref[base_regno] = NULL_RTX;
reg_offset[base_regno] = 0;
reg_set_luid[base_regno] = move2add_luid;
- reg_mode[base_regno] = mode;
+ gcc_assert (GET_MODE (base_reg) == mode);
+ move2add_record_mode (base_reg);
}
- else if (! MODES_OK_FOR_MOVE2ADD (dst_mode,
- reg_mode[base_regno]))
- goto invalidate;
-
- reg_mode[regno] = mode;
/* Copy base information from our base register. */
reg_set_luid[regno] = reg_set_luid[base_regno];
@@ -2262,17 +2288,17 @@ move2add_note_store (rtx dst, const_rtx
reg_symbol_ref[regno] = reg_symbol_ref[base_regno];
/* Compute the sum of the offsets or constants. */
- reg_offset[regno] = trunc_int_for_mode (offset
- + reg_offset[base_regno],
- dst_mode);
+ reg_offset[regno]
+ = trunc_int_for_mode (offset + reg_offset[base_regno], mode);
+
+ move2add_record_mode (dst);
}
else
{
- unsigned int endregno = regno + nregs;
-
- for (i = regno; i < endregno; i++)
- /* Reset the information about this register. */
- reg_set_luid[i] = 0;
+ invalidate:
+ /* Invalidate the contents of the register. */
+ move2add_record_mode (dst);
+ reg_mode[regno] = VOIDmode;
}
}
\f
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: RFA: fix rtl-optimization/56833
2013-05-24 16:33 ` RFA: fix rtl-optimization/56833 Joern Rennecke
@ 2013-05-27 7:30 ` Eric Botcazou
2013-05-28 9:35 ` Andreas Schwab
1 sibling, 0 replies; 11+ messages in thread
From: Eric Botcazou @ 2013-05-27 7:30 UTC (permalink / raw)
To: Joern Rennecke; +Cc: gcc-patches, Alexandre Oliva, Kaz Kojima
> Please find attached the updated patch.
Thanks. Still, although reg_mode[regno] is indeed tested above, in
+ for (i = hard_regno_nregs[regno][mode] - 1; i > 0; i--)
+ if (reg_mode[i] != BLKmode)
+ return false;
this should be reg_mode[regno + i] instead of reg_mode[i].
And one the nice benefits of the switch to C++ is that you can now declare the
iteration variable within the 'for' construct. So, if you aren't planning to
backport the patch to branches prior to 4.8, you can use the idiom in the
newmove2add_record_mode and move2add_valid_value_p functions.
The patch is OK with these modifications if it still passes testing.
--
Eric Botcazou
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: RFA: fix rtl-optimization/56833
2013-05-24 16:33 ` RFA: fix rtl-optimization/56833 Joern Rennecke
2013-05-27 7:30 ` Eric Botcazou
@ 2013-05-28 9:35 ` Andreas Schwab
1 sibling, 0 replies; 11+ messages in thread
From: Andreas Schwab @ 2013-05-28 9:35 UTC (permalink / raw)
To: Joern Rennecke; +Cc: Eric Botcazou, gcc-patches, Alexandre Oliva, Kaz Kojima
Joern Rennecke <joern.rennecke@embecosm.com> writes:
> 2013-05-24 Joern Rennecke <joern.rennecke@embecosm.com>
>
> PR rtl-optimization/56833
> * postreload.c (move2add_record_mode): New function.
> (move2add_record_sym_value, move2add_valid_value_p): Likewise.
> (move2add_use_add2_insn): Use move2add_record_sym_value.
> (move2add_use_add3_insn): Likewise.
> (reload_cse_move2add): Use move2add_valid_value_p and
> move2add_record_mode. Invalidate call-clobbered and REG_INC
> affected regs by setting reg_mode to VOIDmode.
> (move2add_note_store): Don't pretend the inside of a SUBREG is
> the actual destination. Invalidate single/leading registers by
> setting reg_mode to VOIDmode.
> Use move2add_record_sym_value, move2add_valid_value_p and
> move2add_record_mode.
This breaks m68k.
Executing on host: /daten/aranym/gcc/gcc-20130528/Build/gcc/xgcc -B/daten/aranym/gcc/gcc-20130528/Build/gcc/ /daten/aranym/gcc/gcc-20130528/gcc/testsuite/gcc.c-torture/execute/920501-6.c -fno-diagnostics-show-caret -fdiagnostics-color=never -w -O1 -lm -o /daten/aranym/gcc/gcc-20130528/Build/gcc/testsuite/gcc/920501-6.x1 (timeout = 300)
spawn /daten/aranym/gcc/gcc-20130528/Build/gcc/xgcc -B/daten/aranym/gcc/gcc-20130528/Build/gcc/ /daten/aranym/gcc/gcc-20130528/gcc/testsuite/gcc.c-torture/execute/920501-6.c -fno-diagnostics-show-caret -fdiagnostics-color=never -w -O1 -lm -o /daten/aranym/gcc/gcc-20130528/Build/gcc/testsuite/gcc/920501-6.x1.
PASS: gcc.c-torture/execute/920501-6.c compilation, -O1
Executing on aranym: LD_LIBRARY_PATH=:/daten/aranym/gcc/gcc-20130528/Build/gcc:/daten/aranym/gcc/gcc-20130528/Build/gcc timeout 600 /daten/aranym/gcc/gcc-20130528/Build/gcc/testsuite/gcc/920501-6.x1 (timeout = 300)
Executed /daten/aranym/gcc/gcc-20130528/Build/gcc/testsuite/gcc/920501-6.x1, status 1
Output: bash: line 1: 2912 Aborted LD_LIBRARY_PATH=:/daten/aranym/gcc/gcc-20130528/Build/gcc:/daten/aranym/gcc/gcc-20130528/Build/gcc timeout 600 /daten/aranym/gcc/gcc-20130528/Build/gcc/testsuite/gcc/920501-6.x1
child process exited abnormally
FAIL: gcc.c-torture/execute/920501-6.c execution, -O1
old:
1f0: 4282 clrl %d2
1f2: 263c 498f 0a91 movel #1234111121,%d3
new:
1f0: 143c ff91 moveb #-111,%d2
This is generating the constant 1234111121LL.
Andreas.
--
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2013-05-28 9:35 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-05-15 15:00 RFA: fix rtl-optimization/56833 Joern Rennecke
2013-05-22 9:03 ` Eric Botcazou
2013-05-22 10:45 ` Joern Rennecke
2013-05-23 15:00 ` Eric Botcazou
2013-05-23 20:33 ` Joern Rennecke
2013-05-24 0:43 ` SUBREGs in move2add_note_store (Was: Re: RFA: fix rtl-optimization/56833) Joern Rennecke
2013-05-24 7:57 ` Eric Botcazou
2013-05-24 8:42 ` Joern Rennecke
2013-05-24 16:33 ` RFA: fix rtl-optimization/56833 Joern Rennecke
2013-05-27 7:30 ` Eric Botcazou
2013-05-28 9:35 ` Andreas Schwab
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).