* [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).