* [PATCH] Use subreg_regno instead of subreg_regno_offset
@ 2015-10-26 22:47 Anatoliy Sokolov
2015-10-27 10:54 ` Bernd Schmidt
0 siblings, 1 reply; 4+ messages in thread
From: Anatoliy Sokolov @ 2015-10-26 22:47 UTC (permalink / raw)
To: gcc-patches
Hello.
This patch change code 'REGNO (subreg) + subreg_regno_offset (...)' with
subreg_regno (subreg).
Bootstrapped and reg-tested on x86_64-unknown-linux-gnu.
OK for trunk?
2015-10-27 Anatoly Sokolov <aesok@post.ru>
* caller-save.c (add_stored_regs): Use subreg_regno instead of
subreg_regno_offset.
* df-scan.c (df_ref_record): Ditto.
* reg-stack.c (get_true_reg): Ditto.
* reload.c (operands_match_p, find_reloads_address_1,
reg_overlap_mentioned_for_reload_p): Ditto.
Index: gcc/caller-save.c
===================================================================
--- gcc/caller-save.c (revision 229083)
+++ 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 229083)
+++ gcc/df-scan.c (working copy)
@@ -2588,8 +2588,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 229083)
+++ 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 (subreg),
GET_MODE (subreg));
return pat;
}
Index: gcc/reload.c
===================================================================
--- gcc/reload.c (revision 229083)
+++ gcc/reload.c (working copy)
@@ -2256,10 +2256,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);
@@ -2269,10 +2266,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);
@@ -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));
}
if (GET_CODE (op1) == SUBREG)
@@ -5537,12 +5526,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.
@@ -6547,14 +6531,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);
}
Anatoliy.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Use subreg_regno instead of subreg_regno_offset
2015-10-26 22:47 [PATCH] Use subreg_regno instead of subreg_regno_offset Anatoliy Sokolov
@ 2015-10-27 10:54 ` Bernd Schmidt
2015-11-02 22:30 ` Anatoly Sokolov
0 siblings, 1 reply; 4+ messages in thread
From: Bernd Schmidt @ 2015-10-27 10:54 UTC (permalink / raw)
To: Anatoliy Sokolov, gcc-patches
On 10/26/2015 11:46 PM, Anatoliy Sokolov wrote:
> This patch change code 'REGNO (subreg) + subreg_regno_offset (...)'
> with subreg_regno (subreg).
The patch has whitespace damage that makes it difficult to apply. Please
use text/plain attachments.
> 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)))
> {
> - 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 (subreg),
> GET_MODE (subreg));
> return pat;
Isn't this wrong? subreg_regno wants to be called with a SUBREG, but
here we already had subreg = SUBREG_REG (*pat).
> @@ -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.
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Use subreg_regno instead of subreg_regno_offset
2015-10-27 10:54 ` Bernd Schmidt
@ 2015-11-02 22:30 ` Anatoly Sokolov
2015-11-05 10:33 ` Bernd Schmidt
0 siblings, 1 reply; 4+ messages in thread
From: Anatoly Sokolov @ 2015-11-02 22:30 UTC (permalink / raw)
To: gcc-patches, Bernd Schmidt
[-- Attachment #1: Type: text/plain, Size: 2020 bytes --]
Hello.
----- Original Message -----
From: "Bernd Schmidt" <bschmidt@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.
[-- Attachment #2: sr2.diff.txt --]
[-- Type: text/plain, Size: 5090 bytes --]
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);
}
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Use subreg_regno instead of subreg_regno_offset
2015-11-02 22:30 ` Anatoly Sokolov
@ 2015-11-05 10:33 ` Bernd Schmidt
0 siblings, 0 replies; 4+ messages in thread
From: Bernd Schmidt @ 2015-11-05 10:33 UTC (permalink / raw)
To: Anatoly Sokolov, gcc-patches
On 11/02/2015 11:29 PM, Anatoly Sokolov wrote:
>
>>> @@ -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.
Err, what? Before the quoted code we have
rtx op0 = orig_op0;
and then
op0 = SUBREG_REG (op0);
Are you overlooking this line?
> + else if (REG_P (reg) + && HARD_REGISTER_P (reg))
I don't see how this would even compile.
> - 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);
This looks like you edited the patch? The endregno assignment is on its
own line after this.
NAK, a final one as far as I'm concerned.
Bernd
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-11-05 10:33 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-10-26 22:47 [PATCH] Use subreg_regno instead of subreg_regno_offset Anatoliy Sokolov
2015-10-27 10:54 ` Bernd Schmidt
2015-11-02 22:30 ` Anatoly Sokolov
2015-11-05 10:33 ` Bernd Schmidt
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).