Patchwork [GCC/pr56124] Don't prefer memory if the source of load operation has side effect

login
register
mail settings
Submitter Bin Cheng
Date March 25, 2013, 7:15 a.m.
Message ID <000901ce2928$7d570a50$78051ef0$@cheng@arm.com>
Download mbox | patch
Permalink /patch/230575/
State New
Headers show

Comments

Bin Cheng - March 25, 2013, 7:15 a.m.
Sorry for the wrong list.

-----Original Message-----
From: Bin Cheng [mailto:bin.cheng@arm.com] 
Sent: Monday, March 25, 2013 3:00 PM
To: gcc@gcc.gnu.org
Subject: [PATCH GCC/pr56124] Don't prefer memory if the source of load
operation has side effect

Hi,
As reported in PR56124, IRA causes redundant reload by preferring to put
pseudo which is target of loading in memory. Generally this is good but the
case in which the src of loading has side effect.
This patch fixes this issue by checking whether source of loading has side
effect.

I tested the patch on x86/thumb2. Is it OK? Thanks.

2013-03-25  Bin Cheng  <bin.cheng@arm.com>

	PR target/56124
	* ira-costs.c (scan_one_insn): Check whether the source rtx of
	loading has side effect.
Bin Cheng - April 7, 2013, 3:16 a.m.
> -----Original Message-----
> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-owner@gcc.gnu.org]
On
> Behalf Of Bin Cheng
> Sent: Monday, March 25, 2013 3:15 PM
> To: gcc-patches@gcc.gnu.org
> Subject: FW: [PATCH GCC/pr56124] Don't prefer memory if the source of load
> operation has side effect
> 
> Sorry for the wrong list.
> 
> -----Original Message-----
> From: Bin Cheng [mailto:bin.cheng@arm.com]
> Sent: Monday, March 25, 2013 3:00 PM
> To: gcc@gcc.gnu.org
> Subject: [PATCH GCC/pr56124] Don't prefer memory if the source of load
> operation has side effect
> 
> Hi,
> As reported in PR56124, IRA causes redundant reload by preferring to put
> pseudo which is target of loading in memory. Generally this is good but
the
> case in which the src of loading has side effect.
> This patch fixes this issue by checking whether source of loading has side
> effect.
> 
> I tested the patch on x86/thumb2. Is it OK? Thanks.
> 
> 2013-03-25  Bin Cheng  <bin.cheng@arm.com>
> 
> 	PR target/56124
> 	* ira-costs.c (scan_one_insn): Check whether the source rtx of
> 	loading has side effect.

Ping.
Vladimir Makarov - April 10, 2013, 11:19 p.m.
On 13-04-06 11:16 PM, Bin Cheng wrote:
>
>> -----Original Message-----
>> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-owner@gcc.gnu.org]
> On
>> Behalf Of Bin Cheng
>> Sent: Monday, March 25, 2013 3:15 PM
>> To: gcc-patches@gcc.gnu.org
>> Subject: FW: [PATCH GCC/pr56124] Don't prefer memory if the source of load
>> operation has side effect
>>
>> Sorry for the wrong list.
>>
>> -----Original Message-----
>> From: Bin Cheng [mailto:bin.cheng@arm.com]
>> Sent: Monday, March 25, 2013 3:00 PM
>> To: gcc@gcc.gnu.org
>> Subject: [PATCH GCC/pr56124] Don't prefer memory if the source of load
>> operation has side effect
>>
>> Hi,
>> As reported in PR56124, IRA causes redundant reload by preferring to put
>> pseudo which is target of loading in memory. Generally this is good but
> the
>> case in which the src of loading has side effect.
>> This patch fixes this issue by checking whether source of loading has side
>> effect.
>>
>> I tested the patch on x86/thumb2. Is it OK? Thanks.
>>
>> 2013-03-25  Bin Cheng  <bin.cheng@arm.com>
>>
>> 	PR target/56124
>> 	* ira-costs.c (scan_one_insn): Check whether the source rtx of
>> 	loading has side effect.
> Ping.
This patch is ok for trunk.  Thanks.  And sorry, for the delay with the 
answer.
Bin Cheng - April 11, 2013, 5:11 a.m.
> -----Original Message-----
> From: Vladimir Makarov [mailto:vmakarov@redhat.com]
> Sent: Thursday, April 11, 2013 7:20 AM
> To: Bin Cheng
> Cc: gcc-patches@gcc.gnu.org
> Subject: Re: [PATCH GCC/pr56124] Don't prefer memory if the source of load
> operation has side effect
> 
> On 13-04-06 11:16 PM, Bin Cheng wrote:
> >
> >> -----Original Message-----
> >> From: gcc-patches-owner@gcc.gnu.org
> >> [mailto:gcc-patches-owner@gcc.gnu.org]
> > On
> >> Behalf Of Bin Cheng
> >> Sent: Monday, March 25, 2013 3:15 PM
> >> To: gcc-patches@gcc.gnu.org
> >> Subject: FW: [PATCH GCC/pr56124] Don't prefer memory if the source of
> >> load operation has side effect
> >>
> >> Sorry for the wrong list.
> >>
> >> -----Original Message-----
> >> From: Bin Cheng [mailto:bin.cheng@arm.com]
> >> Sent: Monday, March 25, 2013 3:00 PM
> >> To: gcc@gcc.gnu.org
> >> Subject: [PATCH GCC/pr56124] Don't prefer memory if the source of
> >> load operation has side effect
> >>
> >> Hi,
> >> As reported in PR56124, IRA causes redundant reload by preferring to
> >> put pseudo which is target of loading in memory. Generally this is
> >> good but
> > the
> >> case in which the src of loading has side effect.
> >> This patch fixes this issue by checking whether source of loading has
> >> side effect.
> >>
> >> I tested the patch on x86/thumb2. Is it OK? Thanks.
> >>
> >> 2013-03-25  Bin Cheng  <bin.cheng@arm.com>
> >>
> >> 	PR target/56124
> >> 	* ira-costs.c (scan_one_insn): Check whether the source rtx of
> >> 	loading has side effect.
> > Ping.
> This patch is ok for trunk.  Thanks.  And sorry, for the delay with the
answer.
> 
Committed as r197691.

Thanks.

Patch

Index: gcc/ira-costs.c
===================================================================
--- gcc/ira-costs.c	(revision 197029)
+++ gcc/ira-costs.c	(working copy)
@@ -1293,10 +1293,13 @@  scan_one_insn (rtx insn)
      a memory requiring special instructions to load it, decreasing
      mem_cost might result in it being loaded using the specialized
      instruction into a register, then stored into stack and loaded
-     again from the stack.  See PR52208.  */
+     again from the stack.  See PR52208.
+     
+     Don't do this if SET_SRC (set) has side effect.  See PR56124.  */
   if (set != 0 && REG_P (SET_DEST (set)) && MEM_P (SET_SRC (set))
       && (note = find_reg_note (insn, REG_EQUIV, NULL_RTX)) != NULL_RTX
-      && ((MEM_P (XEXP (note, 0)))
+      && ((MEM_P (XEXP (note, 0))
+	   && !side_effects_p (SET_SRC (set)))
 	  || (CONSTANT_P (XEXP (note, 0))
 	      && targetm.legitimate_constant_p (GET_MODE (SET_DEST (set)),
 						XEXP (note, 0))