Patchwork [reload] Fix bug pr52804, RELOAD pass reloads wrong register on ARM for cortex-m0

login
register
mail settings
Submitter Bin Cheng
Date April 20, 2012, 5:50 a.m.
Message ID <016101cd1eb9$8f4f14c0$aded3e40$@cheng@arm.com>
Download mbox | patch
Permalink /patch/153937/
State New
Headers show

Comments

Bin Cheng - April 20, 2012, 5:50 a.m.
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.
Bin Cheng - May 2, 2012, 7:12 a.m.
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.
Bin Cheng - May 3, 2012, 2:31 a.m.
> -----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.
Bin Cheng - May 4, 2012, 3 a.m.
> -----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
>

Patch

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: