* [PATCH] internal-fn, tree-cfg: Fix .TRAP handling and another __builtin_trap vops issue [PR106099]
@ 2022-08-24 8:24 Jakub Jelinek
2022-08-24 10:20 ` Richard Biener
0 siblings, 1 reply; 4+ messages in thread
From: Jakub Jelinek @ 2022-08-24 8:24 UTC (permalink / raw)
To: Richard Biener; +Cc: gcc-patches
Hi!
This patch fixes 2 __builtin_unreachable/__builtin_trap related issues.
One (first hunk) is that CDDCE happily removes calls to .TRAP ()
internal-fn as useless. The problem is that the internal-fn is
ECF_CONST | ECF_NORETURN, doesn't have lhs and so DCE thinks it doesn't
have side-effects and removes it. __builtin_unreachable which has
the same ECF_* flags works fine, as since PR44485 we implicitly add
ECF_LOOPING_CONST_OR_PURE to ECF_CONST | ECF_NORETURN builtins, but
do it in flags_from_decl_or_type which isn't called for internal-fns.
As IFN_TRAP is the only ifn with such flags, it seems easier to
add it explicitly.
The other issue (which on the testcase can be seen only with the
first bug unfixed) is that execute_fixup_cfg can add a __builtin_trap
which needs vops, but nothing adds it and it can appear in many passes
which don't have corresponding TODO_update_ssa_only_virtuals etc.
Fixed similarly as last time but emitting ifn there instead.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2022-08-24 Jakub Jelinek <jakub@redhat.com>
PR tree-optimization/106099
* internal-fn.def (TRAP): Add ECF_LOOPING_CONST_OR_PURE flag.
* tree-cfg.cc (execute_fixup_cfg): Add IFN_TRAP instead of
__builtin_trap to avoid the need of vops.
* gcc.dg/pr106099.c: New test.
--- gcc/internal-fn.def.jj 2022-07-28 12:43:12.876295553 +0200
+++ gcc/internal-fn.def 2022-08-23 14:21:49.559364691 +0200
@@ -458,7 +458,8 @@ DEF_INTERNAL_FN (SPACESHIP, ECF_CONST |
/* __builtin_trap created from/for __builtin_unreachable. */
DEF_INTERNAL_FN (TRAP, ECF_CONST | ECF_LEAF | ECF_NORETURN
- | ECF_NOTHROW | ECF_COLD, NULL)
+ | ECF_NOTHROW | ECF_COLD | ECF_LOOPING_CONST_OR_PURE,
+ NULL)
#undef DEF_INTERNAL_INT_FN
#undef DEF_INTERNAL_FLT_FN
--- gcc/tree-cfg.cc.jj 2022-07-26 10:32:23.998267698 +0200
+++ gcc/tree-cfg.cc 2022-08-23 12:36:03.818856327 +0200
@@ -9879,10 +9879,16 @@ execute_fixup_cfg (void)
if (stmt && is_gimple_call (stmt))
gimple_call_set_ctrl_altering (stmt, false);
tree fndecl = builtin_decl_unreachable ();
- stmt = gimple_build_call (fndecl, 0);
+ if (DECL_FUNCTION_CODE (fndecl) != BUILT_IN_TRAP)
+ stmt = gimple_build_call (fndecl, 0);
+ else
+ /* Instead of __builtin_trap use .TRAP, so that it doesn't
+ need vops. */
+ stmt = gimple_build_call_internal (IFN_TRAP, 0);
gimple_stmt_iterator gsi = gsi_last_bb (bb);
gsi_insert_after (&gsi, stmt, GSI_NEW_STMT);
- if (!cfun->after_inlining)
+ if (!cfun->after_inlining
+ && DECL_FUNCTION_CODE (fndecl) != BUILT_IN_TRAP)
{
gcall *call_stmt = dyn_cast <gcall *> (stmt);
node->create_edge (cgraph_node::get_create (fndecl),
--- gcc/testsuite/gcc.dg/pr106099.c.jj 2022-08-23 14:30:51.992057144 +0200
+++ gcc/testsuite/gcc.dg/pr106099.c 2022-08-23 14:29:04.271508337 +0200
@@ -0,0 +1,10 @@
+/* PR tree-optimization/106099 */
+/* { dg-do compile } */
+/* { dg-options "-O -fharden-compares -fno-tree-forwprop -fno-tree-ch -fno-tree-dominator-opts -fno-tree-ccp -funreachable-traps --param=scev-max-expr-size=1" } */
+
+void
+foo (void)
+{
+ for (unsigned i = 0; i == 0; i++)
+ __builtin_printf ("%d", i);
+}
Jakub
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] internal-fn, tree-cfg: Fix .TRAP handling and another __builtin_trap vops issue [PR106099]
2022-08-24 8:24 [PATCH] internal-fn, tree-cfg: Fix .TRAP handling and another __builtin_trap vops issue [PR106099] Jakub Jelinek
@ 2022-08-24 10:20 ` Richard Biener
2022-08-25 7:48 ` [PATCH] internal-fn, tree-cfg, v2: " Jakub Jelinek
0 siblings, 1 reply; 4+ messages in thread
From: Richard Biener @ 2022-08-24 10:20 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: gcc-patches
On Wed, 24 Aug 2022, Jakub Jelinek wrote:
> Hi!
>
> This patch fixes 2 __builtin_unreachable/__builtin_trap related issues.
> One (first hunk) is that CDDCE happily removes calls to .TRAP ()
> internal-fn as useless. The problem is that the internal-fn is
> ECF_CONST | ECF_NORETURN, doesn't have lhs and so DCE thinks it doesn't
> have side-effects and removes it. __builtin_unreachable which has
> the same ECF_* flags works fine, as since PR44485 we implicitly add
> ECF_LOOPING_CONST_OR_PURE to ECF_CONST | ECF_NORETURN builtins, but
> do it in flags_from_decl_or_type which isn't called for internal-fns.
> As IFN_TRAP is the only ifn with such flags, it seems easier to
> add it explicitly.
>
> The other issue (which on the testcase can be seen only with the
> first bug unfixed) is that execute_fixup_cfg can add a __builtin_trap
> which needs vops, but nothing adds it and it can appear in many passes
> which don't have corresponding TODO_update_ssa_only_virtuals etc.
> Fixed similarly as last time but emitting ifn there instead.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
I still disagree with this in general - but well, seems the ship
has sailed.
Do we have to repeat this pattern elsewhere? It seems we have
build_builtin_unreachable (not using .IFN_TRAP) but no corresponding
GIMPLE variant? Maybe have builtin_cfn_unreachable () that returns
BUILT_IN_UNREACHABLE or IFN_TRAP and use gimple_build () with that
combined_fn argument? See below
> 2022-08-24 Jakub Jelinek <jakub@redhat.com>
>
> PR tree-optimization/106099
> * internal-fn.def (TRAP): Add ECF_LOOPING_CONST_OR_PURE flag.
> * tree-cfg.cc (execute_fixup_cfg): Add IFN_TRAP instead of
> __builtin_trap to avoid the need of vops.
>
> * gcc.dg/pr106099.c: New test.
>
> --- gcc/internal-fn.def.jj 2022-07-28 12:43:12.876295553 +0200
> +++ gcc/internal-fn.def 2022-08-23 14:21:49.559364691 +0200
> @@ -458,7 +458,8 @@ DEF_INTERNAL_FN (SPACESHIP, ECF_CONST |
>
> /* __builtin_trap created from/for __builtin_unreachable. */
> DEF_INTERNAL_FN (TRAP, ECF_CONST | ECF_LEAF | ECF_NORETURN
> - | ECF_NOTHROW | ECF_COLD, NULL)
> + | ECF_NOTHROW | ECF_COLD | ECF_LOOPING_CONST_OR_PURE,
> + NULL)
>
> #undef DEF_INTERNAL_INT_FN
> #undef DEF_INTERNAL_FLT_FN
> --- gcc/tree-cfg.cc.jj 2022-07-26 10:32:23.998267698 +0200
> +++ gcc/tree-cfg.cc 2022-08-23 12:36:03.818856327 +0200
> @@ -9879,10 +9879,16 @@ execute_fixup_cfg (void)
> if (stmt && is_gimple_call (stmt))
> gimple_call_set_ctrl_altering (stmt, false);
> tree fndecl = builtin_decl_unreachable ();
> - stmt = gimple_build_call (fndecl, 0);
> + if (DECL_FUNCTION_CODE (fndecl) != BUILT_IN_TRAP)
> + stmt = gimple_build_call (fndecl, 0);
> + else
> + /* Instead of __builtin_trap use .TRAP, so that it doesn't
> + need vops. */
> + stmt = gimple_build_call_internal (IFN_TRAP, 0);
> gimple_stmt_iterator gsi = gsi_last_bb (bb);
So
combined_fn cfn = builtin_cfn_unreachable ();
gimple_build (&gsi, false, GSI_NEW_STMT, cfn, void_type_node);
?
> gsi_insert_after (&gsi, stmt, GSI_NEW_STMT);
> - if (!cfun->after_inlining)
> + if (!cfun->after_inlining
> + && DECL_FUNCTION_CODE (fndecl) != BUILT_IN_TRAP)
> {
> gcall *call_stmt = dyn_cast <gcall *> (stmt);
> node->create_edge (cgraph_node::get_create (fndecl),
> --- gcc/testsuite/gcc.dg/pr106099.c.jj 2022-08-23 14:30:51.992057144 +0200
> +++ gcc/testsuite/gcc.dg/pr106099.c 2022-08-23 14:29:04.271508337 +0200
> @@ -0,0 +1,10 @@
> +/* PR tree-optimization/106099 */
> +/* { dg-do compile } */
> +/* { dg-options "-O -fharden-compares -fno-tree-forwprop -fno-tree-ch -fno-tree-dominator-opts -fno-tree-ccp -funreachable-traps --param=scev-max-expr-size=1" } */
> +
> +void
> +foo (void)
> +{
> + for (unsigned i = 0; i == 0; i++)
> + __builtin_printf ("%d", i);
> +}
>
> Jakub
>
>
--
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg,
Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman;
HRB 36809 (AG Nuernberg)
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] internal-fn, tree-cfg, v2: Fix .TRAP handling and another __builtin_trap vops issue [PR106099]
2022-08-24 10:20 ` Richard Biener
@ 2022-08-25 7:48 ` Jakub Jelinek
2022-08-25 8:29 ` Richard Biener
0 siblings, 1 reply; 4+ messages in thread
From: Jakub Jelinek @ 2022-08-25 7:48 UTC (permalink / raw)
To: Richard Biener; +Cc: gcc-patches
On Wed, Aug 24, 2022 at 10:20:45AM +0000, Richard Biener wrote:
> So
>
> combined_fn cfn = builtin_cfn_unreachable ();
> gimple_build (&gsi, false, GSI_NEW_STMT, cfn, void_type_node);
>
> ?
So what about just using existing call that creates the GIMPLE call,
whether it is builtin call or internal call?
I didn't use that initially because the code then wants to add a cgraph
edge for the fndecl, but in fact it is quite easy to gimple_call_fndecl
afterwards:
2022-08-25 Jakub Jelinek <jakub@redhat.com>
PR tree-optimization/106099
* internal-fn.def (TRAP): Add ECF_LOOPING_CONST_OR_PURE flag.
* tree-cfg.cc (execute_fixup_cfg): Add IFN_TRAP instead of
__builtin_trap to avoid the need of vops.
* gcc.dg/pr106099.c: New test.
--- gcc/internal-fn.def.jj 2022-07-28 12:43:12.876295553 +0200
+++ gcc/internal-fn.def 2022-08-23 14:21:49.559364691 +0200
@@ -458,7 +458,8 @@ DEF_INTERNAL_FN (SPACESHIP, ECF_CONST |
/* __builtin_trap created from/for __builtin_unreachable. */
DEF_INTERNAL_FN (TRAP, ECF_CONST | ECF_LEAF | ECF_NORETURN
- | ECF_NOTHROW | ECF_COLD, NULL)
+ | ECF_NOTHROW | ECF_COLD | ECF_LOOPING_CONST_OR_PURE,
+ NULL)
#undef DEF_INTERNAL_INT_FN
#undef DEF_INTERNAL_FLT_FN
--- gcc/tree-cfg.cc.jj 2022-07-26 10:32:23.998267698 +0200
+++ gcc/tree-cfg.cc 2022-08-25 09:42:19.013039869 +0200
@@ -9878,16 +9878,16 @@ execute_fixup_cfg (void)
{
if (stmt && is_gimple_call (stmt))
gimple_call_set_ctrl_altering (stmt, false);
- tree fndecl = builtin_decl_unreachable ();
- stmt = gimple_build_call (fndecl, 0);
+ stmt = gimple_build_builtin_unreachable (UNKNOWN_LOCATION);
gimple_stmt_iterator gsi = gsi_last_bb (bb);
gsi_insert_after (&gsi, stmt, GSI_NEW_STMT);
if (!cfun->after_inlining)
- {
- gcall *call_stmt = dyn_cast <gcall *> (stmt);
- node->create_edge (cgraph_node::get_create (fndecl),
- call_stmt, bb->count);
- }
+ if (tree fndecl = gimple_call_fndecl (stmt))
+ {
+ gcall *call_stmt = dyn_cast <gcall *> (stmt);
+ node->create_edge (cgraph_node::get_create (fndecl),
+ call_stmt, bb->count);
+ }
}
}
}
--- gcc/testsuite/gcc.dg/pr106099.c.jj 2022-08-23 14:30:51.992057144 +0200
+++ gcc/testsuite/gcc.dg/pr106099.c 2022-08-23 14:29:04.271508337 +0200
@@ -0,0 +1,10 @@
+/* PR tree-optimization/106099 */
+/* { dg-do compile } */
+/* { dg-options "-O -fharden-compares -fno-tree-forwprop -fno-tree-ch -fno-tree-dominator-opts -fno-tree-ccp -funreachable-traps --param=scev-max-expr-size=1" } */
+
+void
+foo (void)
+{
+ for (unsigned i = 0; i == 0; i++)
+ __builtin_printf ("%d", i);
+}
Jakub
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] internal-fn, tree-cfg, v2: Fix .TRAP handling and another __builtin_trap vops issue [PR106099]
2022-08-25 7:48 ` [PATCH] internal-fn, tree-cfg, v2: " Jakub Jelinek
@ 2022-08-25 8:29 ` Richard Biener
0 siblings, 0 replies; 4+ messages in thread
From: Richard Biener @ 2022-08-25 8:29 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: gcc-patches
On Thu, 25 Aug 2022, Jakub Jelinek wrote:
> On Wed, Aug 24, 2022 at 10:20:45AM +0000, Richard Biener wrote:
> > So
> >
> > combined_fn cfn = builtin_cfn_unreachable ();
> > gimple_build (&gsi, false, GSI_NEW_STMT, cfn, void_type_node);
> >
> > ?
>
> So what about just using existing call that creates the GIMPLE call,
> whether it is builtin call or internal call?
> I didn't use that initially because the code then wants to add a cgraph
> edge for the fndecl, but in fact it is quite easy to gimple_call_fndecl
> afterwards:
That works for me.
OK,
Thanks,
Richard.
> 2022-08-25 Jakub Jelinek <jakub@redhat.com>
>
> PR tree-optimization/106099
> * internal-fn.def (TRAP): Add ECF_LOOPING_CONST_OR_PURE flag.
> * tree-cfg.cc (execute_fixup_cfg): Add IFN_TRAP instead of
> __builtin_trap to avoid the need of vops.
>
> * gcc.dg/pr106099.c: New test.
>
> --- gcc/internal-fn.def.jj 2022-07-28 12:43:12.876295553 +0200
> +++ gcc/internal-fn.def 2022-08-23 14:21:49.559364691 +0200
> @@ -458,7 +458,8 @@ DEF_INTERNAL_FN (SPACESHIP, ECF_CONST |
>
> /* __builtin_trap created from/for __builtin_unreachable. */
> DEF_INTERNAL_FN (TRAP, ECF_CONST | ECF_LEAF | ECF_NORETURN
> - | ECF_NOTHROW | ECF_COLD, NULL)
> + | ECF_NOTHROW | ECF_COLD | ECF_LOOPING_CONST_OR_PURE,
> + NULL)
>
> #undef DEF_INTERNAL_INT_FN
> #undef DEF_INTERNAL_FLT_FN
> --- gcc/tree-cfg.cc.jj 2022-07-26 10:32:23.998267698 +0200
> +++ gcc/tree-cfg.cc 2022-08-25 09:42:19.013039869 +0200
> @@ -9878,16 +9878,16 @@ execute_fixup_cfg (void)
> {
> if (stmt && is_gimple_call (stmt))
> gimple_call_set_ctrl_altering (stmt, false);
> - tree fndecl = builtin_decl_unreachable ();
> - stmt = gimple_build_call (fndecl, 0);
> + stmt = gimple_build_builtin_unreachable (UNKNOWN_LOCATION);
> gimple_stmt_iterator gsi = gsi_last_bb (bb);
> gsi_insert_after (&gsi, stmt, GSI_NEW_STMT);
> if (!cfun->after_inlining)
> - {
> - gcall *call_stmt = dyn_cast <gcall *> (stmt);
> - node->create_edge (cgraph_node::get_create (fndecl),
> - call_stmt, bb->count);
> - }
> + if (tree fndecl = gimple_call_fndecl (stmt))
> + {
> + gcall *call_stmt = dyn_cast <gcall *> (stmt);
> + node->create_edge (cgraph_node::get_create (fndecl),
> + call_stmt, bb->count);
> + }
> }
> }
> }
> --- gcc/testsuite/gcc.dg/pr106099.c.jj 2022-08-23 14:30:51.992057144 +0200
> +++ gcc/testsuite/gcc.dg/pr106099.c 2022-08-23 14:29:04.271508337 +0200
> @@ -0,0 +1,10 @@
> +/* PR tree-optimization/106099 */
> +/* { dg-do compile } */
> +/* { dg-options "-O -fharden-compares -fno-tree-forwprop -fno-tree-ch -fno-tree-dominator-opts -fno-tree-ccp -funreachable-traps --param=scev-max-expr-size=1" } */
> +
> +void
> +foo (void)
> +{
> + for (unsigned i = 0; i == 0; i++)
> + __builtin_printf ("%d", i);
> +}
>
> Jakub
>
>
--
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg,
Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman;
HRB 36809 (AG Nuernberg)
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-08-25 8:29 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-24 8:24 [PATCH] internal-fn, tree-cfg: Fix .TRAP handling and another __builtin_trap vops issue [PR106099] Jakub Jelinek
2022-08-24 10:20 ` Richard Biener
2022-08-25 7:48 ` [PATCH] internal-fn, tree-cfg, v2: " Jakub Jelinek
2022-08-25 8:29 ` Richard Biener
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).