| Message ID | 20120503.011527.1210866546187565505.davem@davemloft.net |
|---|---|
| State | New |
| Headers | show |
David Miller <davem@davemloft.net> writes: > From: Richard Sandiford <rdsandiford@googlemail.com> > Date: Wed, 02 May 2012 20:37:58 +0100 > >> I think the DSE assuption is fair though. If you reuse MEMs like >> this, then they're no longer just serving the purpose described by >> MEM_EXPR. > > The following seems to work, and matches what calls.c does when it > passes a value by reference. Is this what you had in mind? Yeah, looks good to me. Richard
On Thu, May 3, 2012 at 10:31 AM, Richard Sandiford <rdsandiford@googlemail.com> wrote: > David Miller <davem@davemloft.net> writes: >> From: Richard Sandiford <rdsandiford@googlemail.com> >> Date: Wed, 02 May 2012 20:37:58 +0100 >> >>> I think the DSE assuption is fair though. If you reuse MEMs like >>> this, then they're no longer just serving the purpose described by >>> MEM_EXPR. >> >> The following seems to work, and matches what calls.c does when it >> passes a value by reference. Is this what you had in mind? > > Yeah, looks good to me. I don't think that will work reliably (well, maybe now by luck, so better than nothing). You'd at least need to adjust the ESCAPED points-to set of the function, too (yes, DSE does very very conservative use analysis right now). Why not simply clear MEM_EXPR for the MEM? Richard. > Richard
Richard Guenther <richard.guenther@gmail.com> writes: > On Thu, May 3, 2012 at 10:31 AM, Richard Sandiford > <rdsandiford@googlemail.com> wrote: >> David Miller <davem@davemloft.net> writes: >>> From: Richard Sandiford <rdsandiford@googlemail.com> >>> Date: Wed, 02 May 2012 20:37:58 +0100 >>> >>>> I think the DSE assuption is fair though. If you reuse MEMs like >>>> this, then they're no longer just serving the purpose described by >>>> MEM_EXPR. >>> >>> The following seems to work, and matches what calls.c does when it >>> passes a value by reference. Is this what you had in mind? >> >> Yeah, looks good to me. > > I don't think that will work reliably (well, maybe now by luck, so better than > nothing). You'd at least need to adjust the ESCAPED points-to set of the > function, too (yes, DSE does very very conservative use analysis right now). Ah. > Why not simply clear MEM_EXPR for the MEM? The problem is that MEM rtxes aren't shared. AIUI, the store will already have been emitted by this point, using a distinct (but equivalent) MEM rtx. Richard
From: Richard Guenther <richard.guenther@gmail.com> Date: Thu, 3 May 2012 10:42:30 +0200 > On Thu, May 3, 2012 at 10:31 AM, Richard Sandiford > <rdsandiford@googlemail.com> wrote: >> David Miller <davem@davemloft.net> writes: >>> From: Richard Sandiford <rdsandiford@googlemail.com> >>> Date: Wed, 02 May 2012 20:37:58 +0100 >>> >>>> I think the DSE assuption is fair though. If you reuse MEMs like >>>> this, then they're no longer just serving the purpose described by >>>> MEM_EXPR. >>> >>> The following seems to work, and matches what calls.c does when it >>> passes a value by reference. Is this what you had in mind? >> >> Yeah, looks good to me. > > I don't think that will work reliably (well, maybe now by luck, so better than > nothing). You'd at least need to adjust the ESCAPED points-to set of the > function, too (yes, DSE does very very conservative use analysis right now). > Why not simply clear MEM_EXPR for the MEM? Then why does calls.c not have to do this?
From: Richard Sandiford <rdsandiford@googlemail.com> Date: Thu, 03 May 2012 10:17:44 +0100 > Richard Guenther <richard.guenther@gmail.com> writes: >> On Thu, May 3, 2012 at 10:31 AM, Richard Sandiford >> <rdsandiford@googlemail.com> wrote: >>> David Miller <davem@davemloft.net> writes: >>>> From: Richard Sandiford <rdsandiford@googlemail.com> >>>> Date: Wed, 02 May 2012 20:37:58 +0100 >>>> >>>>> I think the DSE assuption is fair though. If you reuse MEMs like >>>>> this, then they're no longer just serving the purpose described by >>>>> MEM_EXPR. >>>> >>>> The following seems to work, and matches what calls.c does when it >>>> passes a value by reference. Is this what you had in mind? >>> >>> Yeah, looks good to me. >> >> I don't think that will work reliably (well, maybe now by luck, so better than >> nothing). You'd at least need to adjust the ESCAPED points-to set of the >> function, too (yes, DSE does very very conservative use analysis right now). > > Ah. > >> Why not simply clear MEM_EXPR for the MEM? > > The problem is that MEM rtxes aren't shared. AIUI, the store will already > have been emitted by this point, using a distinct (but equivalent) MEM rtx. Again, why doesn't calls.c have to do anything about this when it passes arguments by reference? It does exactly what I'm changing the sparc libcall code to do.
On Thu, May 3, 2012 at 11:38 AM, David Miller <davem@davemloft.net> wrote: > From: Richard Sandiford <rdsandiford@googlemail.com> > Date: Thu, 03 May 2012 10:17:44 +0100 > >> Richard Guenther <richard.guenther@gmail.com> writes: >>> On Thu, May 3, 2012 at 10:31 AM, Richard Sandiford >>> <rdsandiford@googlemail.com> wrote: >>>> David Miller <davem@davemloft.net> writes: >>>>> From: Richard Sandiford <rdsandiford@googlemail.com> >>>>> Date: Wed, 02 May 2012 20:37:58 +0100 >>>>> >>>>>> I think the DSE assuption is fair though. If you reuse MEMs like >>>>>> this, then they're no longer just serving the purpose described by >>>>>> MEM_EXPR. >>>>> >>>>> The following seems to work, and matches what calls.c does when it >>>>> passes a value by reference. Is this what you had in mind? >>>> >>>> Yeah, looks good to me. >>> >>> I don't think that will work reliably (well, maybe now by luck, so better than >>> nothing). You'd at least need to adjust the ESCAPED points-to set of the >>> function, too (yes, DSE does very very conservative use analysis right now). >> >> Ah. >> >>> Why not simply clear MEM_EXPR for the MEM? >> >> The problem is that MEM rtxes aren't shared. AIUI, the store will already >> have been emitted by this point, using a distinct (but equivalent) MEM rtx. > > Again, why doesn't calls.c have to do anything about this when it > passes arguments by reference? It does exactly what I'm changing the > sparc libcall code to do. calls.c is unsafe, too. Which is probably why making DSE stronger for calls using the usual alias predicates did not work. Richard.
From: Richard Guenther <richard.guenther@gmail.com> Date: Thu, 3 May 2012 11:48:03 +0200 > calls.c is unsafe, too. Which is probably why making DSE stronger for > calls using the usual alias predicates did not work. Well, when calls.c does something more sophisticated we can do similarly for cases like sparc's libcalls.
On Thu, May 3, 2012 at 11:52 AM, David Miller <davem@davemloft.net> wrote: > From: Richard Guenther <richard.guenther@gmail.com> > Date: Thu, 3 May 2012 11:48:03 +0200 > >> calls.c is unsafe, too. Which is probably why making DSE stronger for >> calls using the usual alias predicates did not work. > > Well, when calls.c does something more sophisticated we can do similarly > for cases like sparc's libcalls. Sure.
From: Richard Guenther <richard.guenther@gmail.com> Date: Thu, 3 May 2012 12:25:11 +0200 > On Thu, May 3, 2012 at 11:52 AM, David Miller <davem@davemloft.net> wrote: >> From: Richard Guenther <richard.guenther@gmail.com> >> Date: Thu, 3 May 2012 11:48:03 +0200 >> >>> calls.c is unsafe, too. Which is probably why making DSE stronger for >>> calls using the usual alias predicates did not work. >> >> Well, when calls.c does something more sophisticated we can do similarly >> for cases like sparc's libcalls. > > Sure. Ok, so I plan to push this sparc fix into mainline and the 4.7 branch after my testing is done. Eric, any objections?
> Ok, so I plan to push this sparc fix into mainline and the 4.7 branch after > my testing is done. > > Eric, any objections? For the record, none.
diff --git a/gcc/config/sparc/sparc.c b/gcc/config/sparc/sparc.c index b987648..7434c0f 100644 --- a/gcc/config/sparc/sparc.c +++ b/gcc/config/sparc/sparc.c @@ -2698,7 +2698,12 @@ emit_soft_tfmode_libcall (const char *func_name, int nargs, rtx *operands) if (GET_CODE (this_arg) == MEM && ! force_stack_temp) - this_arg = XEXP (this_arg, 0); + { + tree expr = MEM_EXPR (this_arg); + if (expr) + mark_addressable (expr); + this_arg = XEXP (this_arg, 0); + } else if (CONSTANT_P (this_arg) && ! force_stack_temp) { @@ -7387,7 +7392,12 @@ sparc_emit_float_lib_cmp (rtx x, rtx y, enum rtx_code comparison) if (TARGET_ARCH64) { if (MEM_P (x)) - slot0 = x; + { + tree expr = MEM_EXPR (x); + if (expr) + mark_addressable (expr); + slot0 = x; + } else { slot0 = assign_stack_temp (TFmode, GET_MODE_SIZE(TFmode), 0); @@ -7395,7 +7405,12 @@ sparc_emit_float_lib_cmp (rtx x, rtx y, enum rtx_code comparison) } if (MEM_P (y)) - slot1 = y; + { + tree expr = MEM_EXPR (y); + if (expr) + mark_addressable (expr); + slot1 = y; + } else { slot1 = assign_stack_temp (TFmode, GET_MODE_SIZE(TFmode), 0);