* [PATCH] tree-optimization/110434 - avoid <retval> ={v} {CLOBBER} from NRV
@ 2023-06-28 10:21 Richard Biener
0 siblings, 0 replies; 5+ messages in thread
From: Richard Biener @ 2023-06-28 10:21 UTC (permalink / raw)
To: gcc-patches; +Cc: Jakub Jelinek
When NRV replaces a local variable with <retval> it also replaces
occurences in clobbers. This leads to <retval> being clobbered
before the return of it which is strictly invalid but harmless in
practice since there's no pass after NRV which would remove
earlier stores.
The following fixes this nevertheless.
Bootstrapped and tested on x86_64-unknown-linux-gnu, OK?
Thanks,
Richard.
PR tree-optimization/110434
* tree-nrv.cc (pass_nrv::execute): Remove CLOBBERs of
VAR we replace with <retval>.
---
gcc/tree-nrv.cc | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/gcc/tree-nrv.cc b/gcc/tree-nrv.cc
index ff47439647c..466b491e4e7 100644
--- a/gcc/tree-nrv.cc
+++ b/gcc/tree-nrv.cc
@@ -256,6 +256,14 @@ pass_nrv::execute (function *fun)
gsi_remove (&gsi, true);
release_defs (stmt);
}
+ /* If this is a CLOBBER of VAR, remove it. */
+ else if (gimple_clobber_p (stmt)
+ && gimple_assign_lhs (stmt) == found)
+ {
+ unlink_stmt_vdef (stmt);
+ gsi_remove (&gsi, true);
+ release_defs (stmt);
+ }
else
{
struct walk_stmt_info wi;
--
2.35.3
^ permalink raw reply [flat|nested] 5+ messages in thread
[parent not found: <20230628102150.620743857B8E@sourceware.org>]
* Re: [PATCH] tree-optimization/110434 - avoid <retval> ={v} {CLOBBER} from NRV
[not found] <20230628102150.620743857B8E@sourceware.org>
@ 2023-06-28 10:37 ` Jakub Jelinek
2023-06-28 12:32 ` Richard Biener
0 siblings, 1 reply; 5+ messages in thread
From: Jakub Jelinek @ 2023-06-28 10:37 UTC (permalink / raw)
To: Richard Biener; +Cc: gcc-patches
On Wed, Jun 28, 2023 at 10:21:45AM +0000, Richard Biener via Gcc-patches wrote:
> When NRV replaces a local variable with <retval> it also replaces
> occurences in clobbers. This leads to <retval> being clobbered
> before the return of it which is strictly invalid but harmless in
> practice since there's no pass after NRV which would remove
> earlier stores.
>
> The following fixes this nevertheless.
>
> Bootstrapped and tested on x86_64-unknown-linux-gnu, OK?
>
> Thanks,
> Richard.
>
> PR tree-optimization/110434
> * tree-nrv.cc (pass_nrv::execute): Remove CLOBBERs of
> VAR we replace with <retval>.
This is in a loop over all basic blocks in a function.
Do we want to kill all clobbers, or just the ones at the end of functions
(i.e. after the <result> = VAR; assignment that we also remove)?
Complication is that doesn't necessarily have to be just the rest of
a single basic block, but all basic blocks from that point until end of
function.
I mean, if we have
var = whatever;
use (var);
var = {CLOBBER};
...
var = whatever_else;
<result> = var;
var = {CLOBBER};
killing the first clobber might result in missed optimizations later on.
On the other side, could there be partial clobbers for the var -> <result>,
var.fld = {CLOBBER};
? Or even worse, indirect clobbers (MEM_REF with SSA_NAME pointing to
var or parts of it)?
Jakub
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] tree-optimization/110434 - avoid <retval> ={v} {CLOBBER} from NRV
2023-06-28 10:37 ` Jakub Jelinek
@ 2023-06-28 12:32 ` Richard Biener
2023-06-28 12:38 ` Jakub Jelinek
0 siblings, 1 reply; 5+ messages in thread
From: Richard Biener @ 2023-06-28 12:32 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: gcc-patches
On Wed, 28 Jun 2023, Jakub Jelinek wrote:
> On Wed, Jun 28, 2023 at 10:21:45AM +0000, Richard Biener via Gcc-patches wrote:
> > When NRV replaces a local variable with <retval> it also replaces
> > occurences in clobbers. This leads to <retval> being clobbered
> > before the return of it which is strictly invalid but harmless in
> > practice since there's no pass after NRV which would remove
> > earlier stores.
> >
> > The following fixes this nevertheless.
> >
> > Bootstrapped and tested on x86_64-unknown-linux-gnu, OK?
> >
> > Thanks,
> > Richard.
> >
> > PR tree-optimization/110434
> > * tree-nrv.cc (pass_nrv::execute): Remove CLOBBERs of
> > VAR we replace with <retval>.
>
> This is in a loop over all basic blocks in a function.
> Do we want to kill all clobbers, or just the ones at the end of functions
> (i.e. after the <result> = VAR; assignment that we also remove)?
> Complication is that doesn't necessarily have to be just the rest of
> a single basic block, but all basic blocks from that point until end of
> function.
> I mean, if we have
> var = whatever;
> use (var);
> var = {CLOBBER};
> ...
> var = whatever_else;
> <result> = var;
> var = {CLOBBER};
> killing the first clobber might result in missed optimizations later on.
As said there's nothing run after NRV.
>
> On the other side, could there be partial clobbers for the var -> <result>,
> var.fld = {CLOBBER};
> ? Or even worse, indirect clobbers (MEM_REF with SSA_NAME pointing to
> var or parts of it)?
We know that 'var' is not address taken, not sure about the partial
clobbers. We could deal with this in the walk_gimple_op case and
simply remove a clobber when data.modified.
I went at it under the presumption that <retval> never goes out of
scope so we shouldn't have any CLOBBER for it. You could also say
that NRV should operate flow-sensitive, going from returns backwards
and simply stop replacing when the var it substitutes for is clobbered.
I'll do the adjustment handling var.fld = {CLOBBER};
If we don't remove earlier clobbers shouldn't that prevent the NRV
then?
Richard.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] tree-optimization/110434 - avoid <retval> ={v} {CLOBBER} from NRV
2023-06-28 12:32 ` Richard Biener
@ 2023-06-28 12:38 ` Jakub Jelinek
0 siblings, 0 replies; 5+ messages in thread
From: Jakub Jelinek @ 2023-06-28 12:38 UTC (permalink / raw)
To: Richard Biener; +Cc: gcc-patches
On Wed, Jun 28, 2023 at 12:32:51PM +0000, Richard Biener wrote:
> As said there's nothing run after NRV.
There is expansion but in the <result> case I strongly doubt we are trying
to stack reuse it for other vars, so maybe it is ok.
> > On the other side, could there be partial clobbers for the var -> <result>,
> > var.fld = {CLOBBER};
> > ? Or even worse, indirect clobbers (MEM_REF with SSA_NAME pointing to
> > var or parts of it)?
>
> We know that 'var' is not address taken, not sure about the partial
> clobbers. We could deal with this in the walk_gimple_op case and
> simply remove a clobber when data.modified.
LGTM.
Jakub
^ permalink raw reply [flat|nested] 5+ messages in thread
[parent not found: <20230628102159.84C093858412@sourceware.org>]
* Re: [PATCH] tree-optimization/110434 - avoid <retval> ={v} {CLOBBER} from NRV
[not found] <20230628102159.84C093858412@sourceware.org>
@ 2023-06-28 14:19 ` Jeff Law
0 siblings, 0 replies; 5+ messages in thread
From: Jeff Law @ 2023-06-28 14:19 UTC (permalink / raw)
To: Richard Biener, gcc-patches; +Cc: Jakub Jelinek
On 6/28/23 04:21, Richard Biener via Gcc-patches wrote:
> When NRV replaces a local variable with <retval> it also replaces
> occurences in clobbers. This leads to <retval> being clobbered
> before the return of it which is strictly invalid but harmless in
> practice since there's no pass after NRV which would remove
> earlier stores.
>
> The following fixes this nevertheless.
>
> Bootstrapped and tested on x86_64-unknown-linux-gnu, OK?
>
> Thanks,
> Richard.
>
> PR tree-optimization/110434
> * tree-nrv.cc (pass_nrv::execute): Remove CLOBBERs of
> VAR we replace with <retval>.
OK.
jeff
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-06-28 14:19 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-28 10:21 [PATCH] tree-optimization/110434 - avoid <retval> ={v} {CLOBBER} from NRV Richard Biener
[not found] <20230628102150.620743857B8E@sourceware.org>
2023-06-28 10:37 ` Jakub Jelinek
2023-06-28 12:32 ` Richard Biener
2023-06-28 12:38 ` Jakub Jelinek
[not found] <20230628102159.84C093858412@sourceware.org>
2023-06-28 14:19 ` Jeff Law
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).