Hello.
----- Original Message -----
From: "Bernd Schmidt" <bschm...@redhat.com>
Sent: Tuesday, October 27, 2015 1:50 PM
On 10/26/2015 11:46 PM, Anatoliy Sokolov wrote:
This patch change code 'REGNO (subreg) + subreg_regno_offset (...)'
with subreg_regno (subreg).
Index: gcc/reg-stack.c
===================================================================
--- gcc/reg-stack.c (revision 229083)
+++ gcc/reg-stack.c (working copy)
@@ -416,11 +416,7 @@
rtx subreg;
if (STACK_REG_P (subreg = SUBREG_REG (*pat)))
{
Isn't this wrong? subreg_regno wants to be called with a SUBREG, but here
we already had subreg = SUBREG_REG (*pat).
Fixed.
@@ -5522,12 +5516,7 @@
op0 = SUBREG_REG (op0);
code0 = GET_CODE (op0);
if (code0 == REG && REGNO (op0) < FIRST_PSEUDO_REGISTER)
- op0 = gen_rtx_REG (word_mode,
- (REGNO (op0) +
- subreg_regno_offset (REGNO (SUBREG_REG (orig_op0)),
- GET_MODE (SUBREG_REG (orig_op0)),
- SUBREG_BYTE (orig_op0),
- GET_MODE (orig_op0))));
+ op0 = gen_rtx_REG (word_mode, subreg_regno (op0));
}
Same problem as in the reg-stack code? The existing code was using
orig_op0 to get the subreg, you've changed it to use op0 which is already
the SUBREG_REG.
No promblens here. At this point op0 is equivalent orig_op0. New value to
op0 can be assigned later.
With an x86 test you're not exercising reload, and even on other targets
this is not a frequently used path. I've stopped reviewing here, I think
this is a good example of the kind of cleanup patch we _shouldn't_ be
doing. We've proved it's risky, and unless these cleanup patches were a
preparation for functional changes, we should just leave the code alone
IMO.
Bernd
Ok. In any case, a revised patch.
Anatoly.
Index: gcc/caller-save.c
===================================================================
--- gcc/caller-save.c (revision 229560)
+++ gcc/caller-save.c (working copy)
@@ -991,31 +991,25 @@
add_stored_regs (rtx reg, const_rtx setter, void *data)
{
int regno, endregno, i;
- machine_mode mode = GET_MODE (reg);
- int offset = 0;
if (GET_CODE (setter) == CLOBBER)
return;
- if (GET_CODE (reg) == SUBREG
+ if (SUBREG_P (reg)
&& REG_P (SUBREG_REG (reg))
- && REGNO (SUBREG_REG (reg)) < FIRST_PSEUDO_REGISTER)
+ && HARD_REGISTER_P (SUBREG_REG (reg)))
{
- offset = subreg_regno_offset (REGNO (SUBREG_REG (reg)),
- GET_MODE (SUBREG_REG (reg)),
- SUBREG_BYTE (reg),
- GET_MODE (reg));
- regno = REGNO (SUBREG_REG (reg)) + offset;
+ regno = subreg_regno (reg);
endregno = regno + subreg_nregs (reg);
}
- else
+ else if (REG_P (reg)
+ && HARD_REGISTER_P (reg))
{
- if (!REG_P (reg) || REGNO (reg) >= FIRST_PSEUDO_REGISTER)
- return;
-
- regno = REGNO (reg) + offset;
- endregno = end_hard_regno (mode, regno);
+ regno = REGNO (reg);
+ endregno = end_hard_regno (GET_MODE (reg), regno);
}
+ else
+ return;
for (i = regno; i < endregno; i++)
SET_REGNO_REG_SET ((regset) data, i);
Index: gcc/df-scan.c
===================================================================
--- gcc/df-scan.c (revision 229560)
+++ gcc/df-scan.c (working copy)
@@ -2587,8 +2587,7 @@
if (GET_CODE (reg) == SUBREG)
{
- regno += subreg_regno_offset (regno, GET_MODE (SUBREG_REG (reg)),
- SUBREG_BYTE (reg), GET_MODE (reg));
+ regno = subreg_regno (reg);
endregno = regno + subreg_nregs (reg);
}
else
Index: gcc/reg-stack.c
===================================================================
--- gcc/reg-stack.c (revision 229560)
+++ gcc/reg-stack.c (working copy)
@@ -416,11 +416,7 @@
rtx subreg;
if (STACK_REG_P (subreg = SUBREG_REG (*pat)))
{
- int regno_off = subreg_regno_offset (REGNO (subreg),
- GET_MODE (subreg),
- SUBREG_BYTE (*pat),
- GET_MODE (*pat));
- *pat = FP_MODE_REG (REGNO (subreg) + regno_off,
+ *pat = FP_MODE_REG (subreg_regno (*pat),
GET_MODE (subreg));
return pat;
}
Index: gcc/reload.c
===================================================================
--- gcc/reload.c (revision 229560)
+++ gcc/reload.c (working copy)
@@ -2253,10 +2253,7 @@
i = REGNO (SUBREG_REG (x));
if (i >= FIRST_PSEUDO_REGISTER)
goto slow;
- i += subreg_regno_offset (REGNO (SUBREG_REG (x)),
- GET_MODE (SUBREG_REG (x)),
- SUBREG_BYTE (x),
- GET_MODE (x));
+ i = subreg_regno (x);
}
else
i = REGNO (x);
@@ -2266,10 +2263,7 @@
j = REGNO (SUBREG_REG (y));
if (j >= FIRST_PSEUDO_REGISTER)
goto slow;
- j += subreg_regno_offset (REGNO (SUBREG_REG (y)),
- GET_MODE (SUBREG_REG (y)),
- SUBREG_BYTE (y),
- GET_MODE (y));
+ j = subreg_regno (y);
}
else
j = REGNO (y);
@@ -5519,12 +5513,7 @@
op0 = SUBREG_REG (op0);
code0 = GET_CODE (op0);
if (code0 == REG && REGNO (op0) < FIRST_PSEUDO_REGISTER)
- op0 = gen_rtx_REG (word_mode,
- (REGNO (op0) +
- subreg_regno_offset (REGNO (SUBREG_REG
(orig_op0)),
- GET_MODE (SUBREG_REG
(orig_op0)),
- SUBREG_BYTE (orig_op0),
- GET_MODE (orig_op0))));
+ op0 = gen_rtx_REG (word_mode, subreg_regno (op0));
}
if (GET_CODE (op1) == SUBREG)
@@ -5534,12 +5523,7 @@
if (code1 == REG && REGNO (op1) < FIRST_PSEUDO_REGISTER)
/* ??? Why is this given op1's mode and above for
??? op0 SUBREGs we use word_mode? */
- op1 = gen_rtx_REG (GET_MODE (op1),
- (REGNO (op1) +
- subreg_regno_offset (REGNO (SUBREG_REG
(orig_op1)),
- GET_MODE (SUBREG_REG
(orig_op1)),
- SUBREG_BYTE (orig_op1),
- GET_MODE (orig_op1))));
+ op1 = gen_rtx_REG (GET_MODE (op1), subreg_regno (op1));
}
/* Plus in the index register may be created only as a result of
register rematerialization for expression like &localvar*4. Reload
it.
@@ -6544,14 +6528,16 @@
return refers_to_mem_for_reload_p (in);
else if (GET_CODE (x) == SUBREG)
{
- regno = REGNO (SUBREG_REG (x));
- if (regno < FIRST_PSEUDO_REGISTER)
- regno += subreg_regno_offset (REGNO (SUBREG_REG (x)),
- GET_MODE (SUBREG_REG (x)),
- SUBREG_BYTE (x),
- GET_MODE (x));
- endregno = regno + (regno < FIRST_PSEUDO_REGISTER
- ? subreg_nregs (x) : 1);
+ if (HARD_REGISTER_P (SUBREG_REG (x)))
+ {
+ regno = subreg_regno (x);
+ endregno = regno + subreg_nregs (x);
+ }
+ else
+ {
+ regno = REGNO (SUBREG_REG (x));
+ endregno = regno + 1;
+ }
return refers_to_regno_for_reload_p (regno, endregno, in, (rtx*) 0);
}
