| Message ID | 016101cd1eb9$8f4f14c0$aded3e40$@cheng@arm.com |
|---|---|
| State | New |
| Headers | show |
Ping. Hi, could anyone help me with this bug please, if you have time? Any comments will be appreciated. Thanks very much. > -----Original Message----- > From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-owner@gcc.gnu.org] On > Behalf Of Bin Cheng > Sent: Friday, April 20, 2012 1:51 PM > To: gcc-patches@gcc.gnu.org > Subject: [PATCH, reload] Fix bug pr52804, RELOAD pass reloads wrong register > on ARM for cortex-m0 > > Hi, > Previously I reported pr52804 in bugzilla about reload pass reloads wrong > register. > After investigation I believe it is a bug in reload pass and here comes the > patch. > You can refer to http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52804 for details. > > In short, I think the confliction of reloads with type > RELOAD_FOR_INPADDR_ADDRESS and type RELOAD_FOR_INPUT_ADDRESS should be handled > in "reload_reg_reaches_end_p". > Also I think RELOAD_FOR_OUTPUT_ADDRESS/RELOAD_FOR_OUTADDR_ADDRESS have the > issue symmetrically, though I have no test case for it. > > I have tested the patch for x86 and arm. Is it OK? > > I think it is a bug of reload, and I understand reload pass should be rarely > touched, so any comments are highly appreciated on this topic. > > Thanks very much. > > > 2012-04-20 Bin Cheng <bin.cheng@arm.com> > > PR target/52804 > * reload1.c (reload_reg_reaches_end_p): Check whether successor reload > with > type RELOAD_FOR_INPUT_ADDRESS kills reload register of current one with > type > RELOAD_FOR_INPADDR_ADDRESS.
> -----Original Message----- > From: Ulrich Weigand [mailto:uweigand@de.ibm.com] > Sent: Thursday, May 03, 2012 12:05 AM > To: Bin Cheng > Cc: gcc-patches@gcc.gnu.org > Subject: Re: [PATCH, reload] Fix bug pr52804, RELOAD pass reloads wrong > register on ARM for cortex-m0 > > Bin Cheng wrote: > > > In short, I think the confliction of reloads with type > > RELOAD_FOR_INPADDR_ADDRESS and type RELOAD_FOR_INPUT_ADDRESS should be > > handled in "reload_reg_reaches_end_p". > > Also I think RELOAD_FOR_OUTPUT_ADDRESS/RELOAD_FOR_OUTADDR_ADDRESS have > > the issue symmetrically, though I have no test case for it. > > Yes, I agree with your reasoning here. This looks like an oversight. > > > PR target/52804 > > * reload1.c (reload_reg_reaches_end_p): Check whether successor > > reload with > > type RELOAD_FOR_INPUT_ADDRESS kills reload register of current one > > with type > > RELOAD_FOR_INPADDR_ADDRESS. > > This is OK. (You also ought to mention the RELOAD_FOR_OUTADDR_ADDRESS part of > the change in the ChangeLog.) Thanks for reviewing, I modified the ChangeLog. Is it ok for trunk and 4.7? I am not sure whether this can be treated as a regression in 4.7 branch since PR52804 has not been confirmed yet. 2012-05-03 Bin Cheng <bin.cheng@arm.com> PR target/52804 * reload1.c (reload_reg_reaches_end_p): Check whether successor reload with type RELOAD_FOR_INPUT_ADDRESS kills reload register of current one with type RELOAD_FOR_INPADDR_ADDRESS. Same stands for RELOAD_FOR_OUTPUT_ADDRESS and RELOAD_FOR_OUTADDR_ADDRESS.
> -----Original Message----- > From: Ulrich Weigand [mailto:uweigand@de.ibm.com] > Sent: Thursday, May 03, 2012 6:11 PM > To: Bin Cheng > Cc: gcc-patches@gcc.gnu.org > Subject: Re: [PATCH, reload] Fix bug pr52804, RELOAD pass reloads wrong > register on ARM for cortex-m0 > > Bin Cheng wrote: > > > Thanks for reviewing, I modified the ChangeLog. Is it ok for trunk and 4.7? > > Yes, this is OK for trunk. Committed to trunk. As in http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52804 Thanks. > > Once it has been in trunk for a week or so and nobody reported any regressions, > it would also be OK to backport to 4.7. > > Thanks, > Ulrich > > -- > Dr. Ulrich Weigand > GNU Toolchain for Linux on System z and Cell BE > Ulrich.Weigand@de.ibm.com >
Index: gcc/reload1.c =================================================================== --- gcc/reload1.c (revision 185992) +++ gcc/reload1.c (working copy) @@ -5429,6 +5429,13 @@ if (TEST_HARD_REG_BIT (reload_reg_used_in_input[i], regno)) return 0; + /* Reload register of reload with type RELOAD_FOR_INPADDR_ADDRESS + could be killed if the register is also used by reload with type + RELOAD_FOR_INPUT_ADDRESS, so check it. */ + if (type == RELOAD_FOR_INPADDR_ADDRESS + && TEST_HARD_REG_BIT (reload_reg_used_in_input_addr[opnum], regno)) + return 0; + for (i = opnum + 1; i < reload_n_operands; i++) if (TEST_HARD_REG_BIT (reload_reg_used_in_input_addr[i], regno) || TEST_HARD_REG_BIT (reload_reg_used_in_inpaddr_addr[i], regno)) @@ -5503,6 +5510,13 @@ || TEST_HARD_REG_BIT (reload_reg_used_in_outaddr_addr[i], regno)) return 0; + /* Reload register of reload with type RELOAD_FOR_OUTADDR_ADDRESS + could be killed if the register is also used by reload with type + RELOAD_FOR_OUTPUT_ADDRESS, so check it. */ + if (type == RELOAD_FOR_OUTADDR_ADDRESS + && TEST_HARD_REG_BIT (reload_reg_used_in_outaddr_addr[opnum], regno)) + return 0; + return 1; default: