This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Use subreg_regno instead of subreg_regno_offset
- From: Bernd Schmidt <bschmidt at redhat dot com>
- To: Anatoliy Sokolov <aesok at post dot ru>, gcc-patches <gcc-patches at gcc dot gnu dot org>
- Date: Tue, 27 Oct 2015 11:50:37 +0100
- Subject: Re: [PATCH] Use subreg_regno instead of subreg_regno_offset
- Authentication-results: sourceware.org; auth=none
- References: <562EAD37 dot 4040402 at post dot ru>
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