* Remove unused arguments of bulitin_unreachable
@ 2014-12-11 17:07 Jan Hubicka
2014-12-11 17:13 ` Jakub Jelinek
2014-12-22 18:14 ` Jeff Law
0 siblings, 2 replies; 6+ messages in thread
From: Jan Hubicka @ 2014-12-11 17:07 UTC (permalink / raw)
To: gcc-patches, rguenther
Hi,
in firefox .optimized dumps one can see few places where __builtin_unreachable
is called (as a result of devirtualization code proving the code path to be
undefined). There is usually some argument setup for the parameters of
__builtin_unreachable that are dead. This patch makes it somewhat better
so now we get:
<bb 30>:
# prephitmp_222 = PHI <_52(27), pretmp_245(29)>
_57 = prephitmp_222 + 2;
pool_40(D)->ptr = _57;
__builtin_unreachable ();
Why DSE does not eliminate the stores prior noreturn const function?
Bootstrapped/regtested x86_64-linux, OK?
Honza
* tree-ssa-dce.c (mark_stmt_if_obviously_necessary): Remove dead parameters
of BUILT_IN_UNREACHABLE
Index: tree-ssa-dce.c
===================================================================
--- tree-ssa-dce.c (revision 218610)
+++ tree-ssa-dce.c (working copy)
@@ -250,6 +250,15 @@ mark_stmt_if_obviously_necessary (gimple
case BUILT_IN_ALLOCA:
case BUILT_IN_ALLOCA_WITH_ALIGN:
return;
+ case BUILT_IN_UNREACHABLE:
+ /* All parameters of BUILT_IN_UNREACHABLE are dead. Remove them
+ from the stmt, so we can remove their definitions. */
+ if (gimple_call_num_args (stmt))
+ {
+ gimple_set_num_ops (stmt, 3);
+ update_stmt (stmt);
+ }
+ break;
default:;
}
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: Remove unused arguments of bulitin_unreachable
2014-12-11 17:07 Remove unused arguments of bulitin_unreachable Jan Hubicka
@ 2014-12-11 17:13 ` Jakub Jelinek
2014-12-11 18:16 ` Jan Hubicka
2014-12-22 18:14 ` Jeff Law
1 sibling, 1 reply; 6+ messages in thread
From: Jakub Jelinek @ 2014-12-11 17:13 UTC (permalink / raw)
To: Jan Hubicka; +Cc: gcc-patches, rguenther
On Thu, Dec 11, 2014 at 06:06:55PM +0100, Jan Hubicka wrote:
> Hi,
> in firefox .optimized dumps one can see few places where __builtin_unreachable
> is called (as a result of devirtualization code proving the code path to be
> undefined). There is usually some argument setup for the parameters of
> __builtin_unreachable that are dead. This patch makes it somewhat better
> so now we get:
> <bb 30>:
> # prephitmp_222 = PHI <_52(27), pretmp_245(29)>
> _57 = prephitmp_222 + 2;
> pool_40(D)->ptr = _57;
> __builtin_unreachable ();
>
> Why DSE does not eliminate the stores prior noreturn const function?
>
> Bootstrapped/regtested x86_64-linux, OK?
>
> Honza
> * tree-ssa-dce.c (mark_stmt_if_obviously_necessary): Remove dead parameters
> of BUILT_IN_UNREACHABLE
Shouldn't this be done when you actually change the call to
__builtin_unreachable ()? I mean, __builtin_unreachable () has no
arguments, so leaving any arguments there is broken IL, even if you clean it
up during the next DCE.
> --- tree-ssa-dce.c (revision 218610)
> +++ tree-ssa-dce.c (working copy)
> @@ -250,6 +250,15 @@ mark_stmt_if_obviously_necessary (gimple
> case BUILT_IN_ALLOCA:
> case BUILT_IN_ALLOCA_WITH_ALIGN:
> return;
> + case BUILT_IN_UNREACHABLE:
> + /* All parameters of BUILT_IN_UNREACHABLE are dead. Remove them
> + from the stmt, so we can remove their definitions. */
> + if (gimple_call_num_args (stmt))
> + {
> + gimple_set_num_ops (stmt, 3);
> + update_stmt (stmt);
> + }
> + break;
>
> default:;
> }
Jakub
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: Remove unused arguments of bulitin_unreachable
2014-12-11 17:13 ` Jakub Jelinek
@ 2014-12-11 18:16 ` Jan Hubicka
2014-12-11 21:05 ` Martin Jambor
2014-12-12 9:19 ` Richard Biener
0 siblings, 2 replies; 6+ messages in thread
From: Jan Hubicka @ 2014-12-11 18:16 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Jan Hubicka, gcc-patches, rguenther
> On Thu, Dec 11, 2014 at 06:06:55PM +0100, Jan Hubicka wrote:
> > Hi,
> > in firefox .optimized dumps one can see few places where __builtin_unreachable
> > is called (as a result of devirtualization code proving the code path to be
> > undefined). There is usually some argument setup for the parameters of
> > __builtin_unreachable that are dead. This patch makes it somewhat better
> > so now we get:
> > <bb 30>:
> > # prephitmp_222 = PHI <_52(27), pretmp_245(29)>
> > _57 = prephitmp_222 + 2;
> > pool_40(D)->ptr = _57;
> > __builtin_unreachable ();
> >
> > Why DSE does not eliminate the stores prior noreturn const function?
> >
> > Bootstrapped/regtested x86_64-linux, OK?
> >
> > Honza
> > * tree-ssa-dce.c (mark_stmt_if_obviously_necessary): Remove dead parameters
> > of BUILT_IN_UNREACHABLE
>
> Shouldn't this be done when you actually change the call to
> __builtin_unreachable ()? I mean, __builtin_unreachable () has no
> arguments, so leaving any arguments there is broken IL, even if you clean it
> up during the next DCE.
Hmm, I tought there was some reason to not do so becuase of inplace folding and memory-SSA.
I can give a try to update all the places we can put builtin_unreachable into IL.
(I wonder if that also include standard constant propagation)
Honza
>
> > --- tree-ssa-dce.c (revision 218610)
> > +++ tree-ssa-dce.c (working copy)
> > @@ -250,6 +250,15 @@ mark_stmt_if_obviously_necessary (gimple
> > case BUILT_IN_ALLOCA:
> > case BUILT_IN_ALLOCA_WITH_ALIGN:
> > return;
> > + case BUILT_IN_UNREACHABLE:
> > + /* All parameters of BUILT_IN_UNREACHABLE are dead. Remove them
> > + from the stmt, so we can remove their definitions. */
> > + if (gimple_call_num_args (stmt))
> > + {
> > + gimple_set_num_ops (stmt, 3);
> > + update_stmt (stmt);
> > + }
> > + break;
> >
> > default:;
> > }
>
> Jakub
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: Remove unused arguments of bulitin_unreachable
2014-12-11 18:16 ` Jan Hubicka
@ 2014-12-11 21:05 ` Martin Jambor
2014-12-12 9:19 ` Richard Biener
1 sibling, 0 replies; 6+ messages in thread
From: Martin Jambor @ 2014-12-11 21:05 UTC (permalink / raw)
To: Jan Hubicka; +Cc: Jakub Jelinek, gcc-patches, rguenther
Hi,
On Thu, Dec 11, 2014 at 07:16:43PM +0100, Jan Hubicka wrote:
> > On Thu, Dec 11, 2014 at 06:06:55PM +0100, Jan Hubicka wrote:
> > > Hi,
> > > in firefox .optimized dumps one can see few places where __builtin_unreachable
> > > is called (as a result of devirtualization code proving the code path to be
> > > undefined). There is usually some argument setup for the parameters of
> > > __builtin_unreachable that are dead. This patch makes it somewhat better
> > > so now we get:
> > > <bb 30>:
> > > # prephitmp_222 = PHI <_52(27), pretmp_245(29)>
> > > _57 = prephitmp_222 + 2;
> > > pool_40(D)->ptr = _57;
> > > __builtin_unreachable ();
> > >
> > > Why DSE does not eliminate the stores prior noreturn const function?
> > >
> > > Bootstrapped/regtested x86_64-linux, OK?
> > >
> > > Honza
> > > * tree-ssa-dce.c (mark_stmt_if_obviously_necessary): Remove dead parameters
> > > of BUILT_IN_UNREACHABLE
> >
> > Shouldn't this be done when you actually change the call to
> > __builtin_unreachable ()? I mean, __builtin_unreachable () has no
> > arguments, so leaving any arguments there is broken IL, even if you clean it
> > up during the next DCE.
>
> Hmm, I tought there was some reason to not do so becuase of inplace folding and memory-SSA.
> I can give a try to update all the places we can put builtin_unreachable into IL.
> (I wonder if that also include standard constant propagation)
I think that's what we ought to do, see also PR 61591.
Martin
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Remove unused arguments of bulitin_unreachable
2014-12-11 18:16 ` Jan Hubicka
2014-12-11 21:05 ` Martin Jambor
@ 2014-12-12 9:19 ` Richard Biener
1 sibling, 0 replies; 6+ messages in thread
From: Richard Biener @ 2014-12-12 9:19 UTC (permalink / raw)
To: Jan Hubicka; +Cc: Jakub Jelinek, GCC Patches
On Thu, Dec 11, 2014 at 7:16 PM, Jan Hubicka <hubicka@ucw.cz> wrote:
>> On Thu, Dec 11, 2014 at 06:06:55PM +0100, Jan Hubicka wrote:
>> > Hi,
>> > in firefox .optimized dumps one can see few places where __builtin_unreachable
>> > is called (as a result of devirtualization code proving the code path to be
>> > undefined). There is usually some argument setup for the parameters of
>> > __builtin_unreachable that are dead. This patch makes it somewhat better
>> > so now we get:
>> > <bb 30>:
>> > # prephitmp_222 = PHI <_52(27), pretmp_245(29)>
>> > _57 = prephitmp_222 + 2;
>> > pool_40(D)->ptr = _57;
>> > __builtin_unreachable ();
>> >
>> > Why DSE does not eliminate the stores prior noreturn const function?
Probably because it has a very special special-casing of "function exit" and
because pool_40(D)->ptr is a global store which cannot be eliminated
(it doesn't special case noreturn const function "exits").
>> > Bootstrapped/regtested x86_64-linux, OK?
>> >
>> > Honza
>> > * tree-ssa-dce.c (mark_stmt_if_obviously_necessary): Remove dead parameters
>> > of BUILT_IN_UNREACHABLE
>>
>> Shouldn't this be done when you actually change the call to
>> __builtin_unreachable ()? I mean, __builtin_unreachable () has no
>> arguments, so leaving any arguments there is broken IL, even if you clean it
>> up during the next DCE.
>
> Hmm, I tought there was some reason to not do so becuase of inplace folding and memory-SSA.
Well, reducing the number of used ops is fine for in-place folding. memory-SSA
shouldn't be an issue.
Richard.
> I can give a try to update all the places we can put builtin_unreachable into IL.
> (I wonder if that also include standard constant propagation)
>
> Honza
>>
>> > --- tree-ssa-dce.c (revision 218610)
>> > +++ tree-ssa-dce.c (working copy)
>> > @@ -250,6 +250,15 @@ mark_stmt_if_obviously_necessary (gimple
>> > case BUILT_IN_ALLOCA:
>> > case BUILT_IN_ALLOCA_WITH_ALIGN:
>> > return;
>> > + case BUILT_IN_UNREACHABLE:
>> > + /* All parameters of BUILT_IN_UNREACHABLE are dead. Remove them
>> > + from the stmt, so we can remove their definitions. */
>> > + if (gimple_call_num_args (stmt))
>> > + {
>> > + gimple_set_num_ops (stmt, 3);
>> > + update_stmt (stmt);
>> > + }
>> > + break;
>> >
>> > default:;
>> > }
>>
>> Jakub
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Remove unused arguments of bulitin_unreachable
2014-12-11 17:07 Remove unused arguments of bulitin_unreachable Jan Hubicka
2014-12-11 17:13 ` Jakub Jelinek
@ 2014-12-22 18:14 ` Jeff Law
1 sibling, 0 replies; 6+ messages in thread
From: Jeff Law @ 2014-12-22 18:14 UTC (permalink / raw)
To: Jan Hubicka, gcc-patches, rguenther
On 12/11/14 10:06, Jan Hubicka wrote:
> Hi,
> in firefox .optimized dumps one can see few places where __builtin_unreachable
> is called (as a result of devirtualization code proving the code path to be
> undefined). There is usually some argument setup for the parameters of
> __builtin_unreachable that are dead. This patch makes it somewhat better
> so now we get:
> <bb 30>:
> # prephitmp_222 = PHI <_52(27), pretmp_245(29)>
> _57 = prephitmp_222 + 2;
> pool_40(D)->ptr = _57;
> __builtin_unreachable ();
>
> Why DSE does not eliminate the stores prior noreturn const function?
>
> Bootstrapped/regtested x86_64-linux, OK?
>
> Honza
> * tree-ssa-dce.c (mark_stmt_if_obviously_necessary): Remove dead parameters
> of BUILT_IN_UNREACHABLE
ISTM that we shouldn't need any special handling in DCE for this. When
something is transformed into a builtin_unreachable(), arguments which
are SSA_NAMEs which are no longer referenced ought to just go away via
the normal DCE mechanisms.
A memory store has side effects that might be observable prior to
reaching the builtin_unreachable, thus it ought not be deleted. At
least that's my recollection of how we wanted this to work for other
transformations which turned turned statements with undefined behaviour
into a __builtin_unreachable.
jeff
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-12-22 18:12 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-12-11 17:07 Remove unused arguments of bulitin_unreachable Jan Hubicka
2014-12-11 17:13 ` Jakub Jelinek
2014-12-11 18:16 ` Jan Hubicka
2014-12-11 21:05 ` Martin Jambor
2014-12-12 9:19 ` Richard Biener
2014-12-22 18:14 ` 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).