* [PATCH] bitint: Fix up adjustment of large/huge _BitInt arguments of returns_twice calls [PR113466]
@ 2024-03-14 8:28 Jakub Jelinek
2024-03-14 8:48 ` Richard Biener
0 siblings, 1 reply; 6+ messages in thread
From: Jakub Jelinek @ 2024-03-14 8:28 UTC (permalink / raw)
To: Richard Biener; +Cc: gcc-patches
Hi!
This patch (on top of the just posted gsi_safe_insert* fixes patch)
fixes the instrumentation of large/huge _BitInt SSA_NAME arguments of
returns_twice calls.
In this case it isn't just a matter of using gsi_safe_insert_before instead
of gsi_insert_before, we need to do more.
One thing is that unlike the asan/ubsan instrumentation which does just some
checking, here we want the statement before the call to load into a SSA_NAME
which is passed to the call. With another edge we need to add a PHI,
with one PHI argument the loaded SSA_NAME, another argument an uninitialized
warning free SSA_NAME and a result and arrange for all 3 SSA_NAMEs to be
preserved (i.e. stay as is, be no longer lowered afterwards).
Another problem is because edge_before_returns_twice_call may use
copy_ssa_name, we can end up with large/huge _BitInt SSA_NAMEs we don't
really track in the pass. We need an underlying variable for those, but
because of the way they are constructed we can find that easily, we can
use the same underlying variable for the PHI arg from non-EDGE_ABNORMAL
edge as we use for the corresponding PHI result. The ugliest part of
this is growing the partition if it needs to be growed (otherwise it is
just a partition_union).
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2024-03-14 Jakub Jelinek <jakub@redhat.com>
PR tree-optimization/113466
* gimple-lower-bitint.cc (bitint_large_huge::lower_call): Handle
ECF_RETURNS_TWICE call arguments correctly.
(gimple_lower_bitint): Ignore PHIs where the PHI result is
in m_preserved bitmap.
* gcc.dg/bitint-100.c: New test.
--- gcc/gimple-lower-bitint.cc.jj 2024-03-13 13:03:08.120117081 +0100
+++ gcc/gimple-lower-bitint.cc 2024-03-13 15:05:54.830303524 +0100
@@ -5248,6 +5248,7 @@ bitint_large_huge::lower_call (tree obj,
default:
break;
}
+ int returns_twice = (gimple_call_flags (stmt) & ECF_RETURNS_TWICE) != 0;
for (unsigned int i = 0; i < nargs; ++i)
{
tree arg = gimple_call_arg (stmt, i);
@@ -5255,6 +5256,8 @@ bitint_large_huge::lower_call (tree obj,
|| TREE_CODE (TREE_TYPE (arg)) != BITINT_TYPE
|| bitint_precision_kind (TREE_TYPE (arg)) <= bitint_prec_middle)
continue;
+ if (m_preserved == NULL)
+ m_preserved = BITMAP_ALLOC (NULL);
if (SSA_NAME_IS_DEFAULT_DEF (arg)
&& (!SSA_NAME_VAR (arg) || VAR_P (SSA_NAME_VAR (arg))))
{
@@ -5270,11 +5273,93 @@ bitint_large_huge::lower_call (tree obj,
v = build1 (VIEW_CONVERT_EXPR, TREE_TYPE (arg), v);
arg = make_ssa_name (TREE_TYPE (arg));
gimple *g = gimple_build_assign (arg, v);
- gsi_insert_before (&gsi, g, GSI_SAME_STMT);
+ gsi_safe_insert_before (&gsi, g);
+ if (returns_twice)
+ {
+ basic_block bb = gimple_bb (stmt);
+ gcc_checking_assert (EDGE_COUNT (bb->preds) == 2);
+ edge e = EDGE_PRED (bb, 0), ead = EDGE_PRED (bb, 1);
+ if ((ead->flags & EDGE_ABNORMAL) == 0)
+ std::swap (e, ead);
+ gcc_checking_assert ((e->flags & EDGE_ABNORMAL) == 0
+ && (ead->flags & EDGE_ABNORMAL));
+ if (returns_twice == 1)
+ {
+ /* edge_before_returns_twice_call can use copy_ssa_name
+ for some PHIs, but in that case we need to put it
+ into the same partition as the copied SSA_NAME. */
+ unsigned max_ver = 0;
+ for (gphi_iterator gsi = gsi_start_phis (bb);
+ !gsi_end_p (gsi); gsi_next (&gsi))
+ {
+ gphi *phi = gsi.phi ();
+ tree lhs = gimple_phi_result (phi);
+ tree arg = gimple_phi_arg_def_from_edge (phi, e);
+ if (m_names
+ && TREE_CODE (arg) == SSA_NAME
+ && TREE_CODE (TREE_TYPE (lhs)) == BITINT_TYPE
+ && (bitint_precision_kind (TREE_TYPE (lhs))
+ > bitint_prec_middle)
+ && bitmap_bit_p (m_names, SSA_NAME_VERSION (lhs))
+ && !bitmap_bit_p (m_names, SSA_NAME_VERSION (arg)))
+ max_ver = MAX (max_ver, SSA_NAME_VERSION (arg));
+ }
+ if (max_ver != 0)
+ {
+ if ((unsigned) m_map->var_partition->num_elements
+ <= max_ver)
+ {
+ partition p = partition_new (max_ver + 1);
+ partition o = m_map->var_partition;
+ for (int e = 0; e < o->num_elements; ++e)
+ {
+ p->elements[e].class_element
+ = o->elements[e].class_element;
+ p->elements[e].class_count
+ = o->elements[e].class_count;
+ p->elements[e].next
+ = &p->elements[0] + (o->elements[e].next
+ - &o->elements[0]);
+ }
+ m_map->var_partition = p;
+ partition_delete (o);
+ }
+ for (gphi_iterator gsi = gsi_start_phis (bb);
+ !gsi_end_p (gsi); gsi_next (&gsi))
+ {
+ gphi *phi = gsi.phi ();
+ tree lhs = gimple_phi_result (phi);
+ tree arg = gimple_phi_arg_def_from_edge (phi, e);
+ if (m_names
+ && TREE_CODE (arg) == SSA_NAME
+ && TREE_CODE (TREE_TYPE (lhs)) == BITINT_TYPE
+ && (bitint_precision_kind (TREE_TYPE (lhs))
+ > bitint_prec_middle)
+ && bitmap_bit_p (m_names, SSA_NAME_VERSION (lhs))
+ && bitmap_set_bit (m_names,
+ SSA_NAME_VERSION (arg)))
+ partition_union (m_map->var_partition,
+ SSA_NAME_VERSION (lhs),
+ SSA_NAME_VERSION (arg));
+ }
+ }
+ returns_twice = 2;
+ }
+ gphi *phi = create_phi_node (make_ssa_name (TREE_TYPE (arg)),
+ bb);
+ add_phi_arg (phi, arg, e, UNKNOWN_LOCATION);
+ bitmap_set_bit (m_preserved, SSA_NAME_VERSION (arg));
+ tree var = create_tmp_reg (TREE_TYPE (arg));
+ suppress_warning (var, OPT_Wuninitialized);
+ arg = get_or_create_ssa_default_def (cfun, var);
+ SSA_NAME_OCCURS_IN_ABNORMAL_PHI (arg) = 1;
+ add_phi_arg (phi, arg, ead, UNKNOWN_LOCATION);
+ bitmap_set_bit (m_preserved, SSA_NAME_VERSION (arg));
+ arg = gimple_phi_result (phi);
+ SSA_NAME_OCCURS_IN_ABNORMAL_PHI (arg) = 1;
+ }
}
gimple_call_set_arg (stmt, i, arg);
- if (m_preserved == NULL)
- m_preserved = BITMAP_ALLOC (NULL);
bitmap_set_bit (m_preserved, SSA_NAME_VERSION (arg));
}
tree lhs = gimple_call_lhs (stmt);
@@ -6870,6 +6955,9 @@ gimple_lower_bitint (void)
if (TREE_CODE (TREE_TYPE (lhs)) != BITINT_TYPE
|| bitint_precision_kind (TREE_TYPE (lhs)) < bitint_prec_large)
continue;
+ if (large_huge.m_preserved
+ && bitmap_bit_p (large_huge.m_preserved, SSA_NAME_VERSION (lhs)))
+ continue;
int p1 = var_to_partition (large_huge.m_map, lhs);
gcc_assert (large_huge.m_vars[p1] != NULL_TREE);
tree v1 = large_huge.m_vars[p1];
--- gcc/testsuite/gcc.dg/bitint-100.c.jj 2024-03-13 15:21:02.313768843 +0100
+++ gcc/testsuite/gcc.dg/bitint-100.c 2024-03-13 12:11:18.636036865 +0100
@@ -0,0 +1,61 @@
+/* PR tree-optimization/113466 */
+/* { dg-do compile { target bitint575 } } */
+/* { dg-options "-O2" } */
+
+int foo (int);
+
+__attribute__((returns_twice, noipa)) _BitInt(325)
+bar (_BitInt(575) x)
+{
+ (void) x;
+ return 0wb;
+}
+
+_BitInt(325)
+baz (_BitInt(575) y)
+{
+ foo (1);
+ return bar (y);
+}
+
+_BitInt(325)
+qux (int x, _BitInt(575) y)
+{
+ if (x == 25)
+ x = foo (2);
+ else if (x == 42)
+ x = foo (foo (3));
+ return bar (y);
+}
+
+void
+corge (int x, _BitInt(575) y, _BitInt(325) *z)
+{
+ void *q[] = { &&l1, &&l2, &&l3, &&l3 };
+ if (x == 25)
+ {
+ l1:
+ x = foo (2);
+ }
+ else if (x == 42)
+ {
+ l2:
+ x = foo (foo (3));
+ }
+l3:
+ *z = bar (y);
+ if (x < 4)
+ goto *q[x & 3];
+}
+
+_BitInt(325)
+freddy (int x, _BitInt(575) y)
+{
+ bar (y);
+ ++y;
+ if (x == 25)
+ x = foo (2);
+ else if (x == 42)
+ x = foo (foo (3));
+ return bar (y);
+}
Jakub
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] bitint: Fix up adjustment of large/huge _BitInt arguments of returns_twice calls [PR113466]
2024-03-14 8:28 [PATCH] bitint: Fix up adjustment of large/huge _BitInt arguments of returns_twice calls [PR113466] Jakub Jelinek
@ 2024-03-14 8:48 ` Richard Biener
2024-03-14 8:53 ` Jakub Jelinek
0 siblings, 1 reply; 6+ messages in thread
From: Richard Biener @ 2024-03-14 8:48 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: gcc-patches
On Thu, 14 Mar 2024, Jakub Jelinek wrote:
> Hi!
>
> This patch (on top of the just posted gsi_safe_insert* fixes patch)
> fixes the instrumentation of large/huge _BitInt SSA_NAME arguments of
> returns_twice calls.
>
> In this case it isn't just a matter of using gsi_safe_insert_before instead
> of gsi_insert_before, we need to do more.
>
> One thing is that unlike the asan/ubsan instrumentation which does just some
> checking, here we want the statement before the call to load into a SSA_NAME
> which is passed to the call. With another edge we need to add a PHI,
> with one PHI argument the loaded SSA_NAME, another argument an uninitialized
> warning free SSA_NAME and a result and arrange for all 3 SSA_NAMEs to be
> preserved (i.e. stay as is, be no longer lowered afterwards).
>
> Another problem is because edge_before_returns_twice_call may use
> copy_ssa_name, we can end up with large/huge _BitInt SSA_NAMEs we don't
> really track in the pass. We need an underlying variable for those, but
> because of the way they are constructed we can find that easily, we can
> use the same underlying variable for the PHI arg from non-EDGE_ABNORMAL
> edge as we use for the corresponding PHI result. The ugliest part of
> this is growing the partition if it needs to be growed (otherwise it is
> just a partition_union).
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
Ugh. OK, but I wonder whether we might want to simply delay
fixing the CFG for inserts before returns-twice? Would that make
things less ugly?
Thanks,
Richard.
> 2024-03-14 Jakub Jelinek <jakub@redhat.com>
>
> PR tree-optimization/113466
> * gimple-lower-bitint.cc (bitint_large_huge::lower_call): Handle
> ECF_RETURNS_TWICE call arguments correctly.
> (gimple_lower_bitint): Ignore PHIs where the PHI result is
> in m_preserved bitmap.
>
> * gcc.dg/bitint-100.c: New test.
>
> --- gcc/gimple-lower-bitint.cc.jj 2024-03-13 13:03:08.120117081 +0100
> +++ gcc/gimple-lower-bitint.cc 2024-03-13 15:05:54.830303524 +0100
> @@ -5248,6 +5248,7 @@ bitint_large_huge::lower_call (tree obj,
> default:
> break;
> }
> + int returns_twice = (gimple_call_flags (stmt) & ECF_RETURNS_TWICE) != 0;
> for (unsigned int i = 0; i < nargs; ++i)
> {
> tree arg = gimple_call_arg (stmt, i);
> @@ -5255,6 +5256,8 @@ bitint_large_huge::lower_call (tree obj,
> || TREE_CODE (TREE_TYPE (arg)) != BITINT_TYPE
> || bitint_precision_kind (TREE_TYPE (arg)) <= bitint_prec_middle)
> continue;
> + if (m_preserved == NULL)
> + m_preserved = BITMAP_ALLOC (NULL);
> if (SSA_NAME_IS_DEFAULT_DEF (arg)
> && (!SSA_NAME_VAR (arg) || VAR_P (SSA_NAME_VAR (arg))))
> {
> @@ -5270,11 +5273,93 @@ bitint_large_huge::lower_call (tree obj,
> v = build1 (VIEW_CONVERT_EXPR, TREE_TYPE (arg), v);
> arg = make_ssa_name (TREE_TYPE (arg));
> gimple *g = gimple_build_assign (arg, v);
> - gsi_insert_before (&gsi, g, GSI_SAME_STMT);
> + gsi_safe_insert_before (&gsi, g);
> + if (returns_twice)
> + {
> + basic_block bb = gimple_bb (stmt);
> + gcc_checking_assert (EDGE_COUNT (bb->preds) == 2);
> + edge e = EDGE_PRED (bb, 0), ead = EDGE_PRED (bb, 1);
> + if ((ead->flags & EDGE_ABNORMAL) == 0)
> + std::swap (e, ead);
> + gcc_checking_assert ((e->flags & EDGE_ABNORMAL) == 0
> + && (ead->flags & EDGE_ABNORMAL));
> + if (returns_twice == 1)
> + {
> + /* edge_before_returns_twice_call can use copy_ssa_name
> + for some PHIs, but in that case we need to put it
> + into the same partition as the copied SSA_NAME. */
> + unsigned max_ver = 0;
> + for (gphi_iterator gsi = gsi_start_phis (bb);
> + !gsi_end_p (gsi); gsi_next (&gsi))
> + {
> + gphi *phi = gsi.phi ();
> + tree lhs = gimple_phi_result (phi);
> + tree arg = gimple_phi_arg_def_from_edge (phi, e);
> + if (m_names
> + && TREE_CODE (arg) == SSA_NAME
> + && TREE_CODE (TREE_TYPE (lhs)) == BITINT_TYPE
> + && (bitint_precision_kind (TREE_TYPE (lhs))
> + > bitint_prec_middle)
> + && bitmap_bit_p (m_names, SSA_NAME_VERSION (lhs))
> + && !bitmap_bit_p (m_names, SSA_NAME_VERSION (arg)))
> + max_ver = MAX (max_ver, SSA_NAME_VERSION (arg));
> + }
> + if (max_ver != 0)
> + {
> + if ((unsigned) m_map->var_partition->num_elements
> + <= max_ver)
> + {
> + partition p = partition_new (max_ver + 1);
> + partition o = m_map->var_partition;
> + for (int e = 0; e < o->num_elements; ++e)
> + {
> + p->elements[e].class_element
> + = o->elements[e].class_element;
> + p->elements[e].class_count
> + = o->elements[e].class_count;
> + p->elements[e].next
> + = &p->elements[0] + (o->elements[e].next
> + - &o->elements[0]);
> + }
> + m_map->var_partition = p;
> + partition_delete (o);
> + }
> + for (gphi_iterator gsi = gsi_start_phis (bb);
> + !gsi_end_p (gsi); gsi_next (&gsi))
> + {
> + gphi *phi = gsi.phi ();
> + tree lhs = gimple_phi_result (phi);
> + tree arg = gimple_phi_arg_def_from_edge (phi, e);
> + if (m_names
> + && TREE_CODE (arg) == SSA_NAME
> + && TREE_CODE (TREE_TYPE (lhs)) == BITINT_TYPE
> + && (bitint_precision_kind (TREE_TYPE (lhs))
> + > bitint_prec_middle)
> + && bitmap_bit_p (m_names, SSA_NAME_VERSION (lhs))
> + && bitmap_set_bit (m_names,
> + SSA_NAME_VERSION (arg)))
> + partition_union (m_map->var_partition,
> + SSA_NAME_VERSION (lhs),
> + SSA_NAME_VERSION (arg));
> + }
> + }
> + returns_twice = 2;
> + }
> + gphi *phi = create_phi_node (make_ssa_name (TREE_TYPE (arg)),
> + bb);
> + add_phi_arg (phi, arg, e, UNKNOWN_LOCATION);
> + bitmap_set_bit (m_preserved, SSA_NAME_VERSION (arg));
> + tree var = create_tmp_reg (TREE_TYPE (arg));
> + suppress_warning (var, OPT_Wuninitialized);
> + arg = get_or_create_ssa_default_def (cfun, var);
> + SSA_NAME_OCCURS_IN_ABNORMAL_PHI (arg) = 1;
> + add_phi_arg (phi, arg, ead, UNKNOWN_LOCATION);
> + bitmap_set_bit (m_preserved, SSA_NAME_VERSION (arg));
> + arg = gimple_phi_result (phi);
> + SSA_NAME_OCCURS_IN_ABNORMAL_PHI (arg) = 1;
> + }
> }
> gimple_call_set_arg (stmt, i, arg);
> - if (m_preserved == NULL)
> - m_preserved = BITMAP_ALLOC (NULL);
> bitmap_set_bit (m_preserved, SSA_NAME_VERSION (arg));
> }
> tree lhs = gimple_call_lhs (stmt);
> @@ -6870,6 +6955,9 @@ gimple_lower_bitint (void)
> if (TREE_CODE (TREE_TYPE (lhs)) != BITINT_TYPE
> || bitint_precision_kind (TREE_TYPE (lhs)) < bitint_prec_large)
> continue;
> + if (large_huge.m_preserved
> + && bitmap_bit_p (large_huge.m_preserved, SSA_NAME_VERSION (lhs)))
> + continue;
> int p1 = var_to_partition (large_huge.m_map, lhs);
> gcc_assert (large_huge.m_vars[p1] != NULL_TREE);
> tree v1 = large_huge.m_vars[p1];
> --- gcc/testsuite/gcc.dg/bitint-100.c.jj 2024-03-13 15:21:02.313768843 +0100
> +++ gcc/testsuite/gcc.dg/bitint-100.c 2024-03-13 12:11:18.636036865 +0100
> @@ -0,0 +1,61 @@
> +/* PR tree-optimization/113466 */
> +/* { dg-do compile { target bitint575 } } */
> +/* { dg-options "-O2" } */
> +
> +int foo (int);
> +
> +__attribute__((returns_twice, noipa)) _BitInt(325)
> +bar (_BitInt(575) x)
> +{
> + (void) x;
> + return 0wb;
> +}
> +
> +_BitInt(325)
> +baz (_BitInt(575) y)
> +{
> + foo (1);
> + return bar (y);
> +}
> +
> +_BitInt(325)
> +qux (int x, _BitInt(575) y)
> +{
> + if (x == 25)
> + x = foo (2);
> + else if (x == 42)
> + x = foo (foo (3));
> + return bar (y);
> +}
> +
> +void
> +corge (int x, _BitInt(575) y, _BitInt(325) *z)
> +{
> + void *q[] = { &&l1, &&l2, &&l3, &&l3 };
> + if (x == 25)
> + {
> + l1:
> + x = foo (2);
> + }
> + else if (x == 42)
> + {
> + l2:
> + x = foo (foo (3));
> + }
> +l3:
> + *z = bar (y);
> + if (x < 4)
> + goto *q[x & 3];
> +}
> +
> +_BitInt(325)
> +freddy (int x, _BitInt(575) y)
> +{
> + bar (y);
> + ++y;
> + if (x == 25)
> + x = foo (2);
> + else if (x == 42)
> + x = foo (foo (3));
> + return bar (y);
> +}
>
> Jakub
>
>
--
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] bitint: Fix up adjustment of large/huge _BitInt arguments of returns_twice calls [PR113466]
2024-03-14 8:48 ` Richard Biener
@ 2024-03-14 8:53 ` Jakub Jelinek
2024-03-14 8:59 ` Richard Biener
0 siblings, 1 reply; 6+ messages in thread
From: Jakub Jelinek @ 2024-03-14 8:53 UTC (permalink / raw)
To: Richard Biener; +Cc: gcc-patches
On Thu, Mar 14, 2024 at 09:48:45AM +0100, Richard Biener wrote:
> Ugh. OK, but I wonder whether we might want to simply delay
> fixing the CFG for inserts before returns-twice? Would that make
> things less ugly?
You mean in lower_call just remember if we added anything before
ECF_RETURNS_TWICE call (or say add such stmt into a vector) and
then fix it up before the end of the pass?
I can certainly try that and see what is shorter/more maintainable.
Jakub
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] bitint: Fix up adjustment of large/huge _BitInt arguments of returns_twice calls [PR113466]
2024-03-14 8:53 ` Jakub Jelinek
@ 2024-03-14 8:59 ` Richard Biener
2024-03-14 10:42 ` [PATCH] bitint, v2: " Jakub Jelinek
0 siblings, 1 reply; 6+ messages in thread
From: Richard Biener @ 2024-03-14 8:59 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: gcc-patches
On Thu, 14 Mar 2024, Jakub Jelinek wrote:
> On Thu, Mar 14, 2024 at 09:48:45AM +0100, Richard Biener wrote:
> > Ugh. OK, but I wonder whether we might want to simply delay
> > fixing the CFG for inserts before returns-twice? Would that make
> > things less ugly?
>
> You mean in lower_call just remember if we added anything before
> ECF_RETURNS_TWICE call (or say add such stmt into a vector) and
> then fix it up before the end of the pass?
Yeah, or just walk BBs with abnormal preds, whatever tracking is
easier.
> I can certainly try that and see what is shorter/more maintainable.
>
> Jakub
>
>
--
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] bitint, v2: Fix up adjustment of large/huge _BitInt arguments of returns_twice calls [PR113466]
2024-03-14 8:59 ` Richard Biener
@ 2024-03-14 10:42 ` Jakub Jelinek
2024-03-14 11:52 ` Richard Biener
0 siblings, 1 reply; 6+ messages in thread
From: Jakub Jelinek @ 2024-03-14 10:42 UTC (permalink / raw)
To: Richard Biener; +Cc: gcc-patches
On Thu, Mar 14, 2024 at 09:59:12AM +0100, Richard Biener wrote:
> On Thu, 14 Mar 2024, Jakub Jelinek wrote:
>
> > On Thu, Mar 14, 2024 at 09:48:45AM +0100, Richard Biener wrote:
> > > Ugh. OK, but I wonder whether we might want to simply delay
> > > fixing the CFG for inserts before returns-twice? Would that make
> > > things less ugly?
> >
> > You mean in lower_call just remember if we added anything before
> > ECF_RETURNS_TWICE call (or say add such stmt into a vector) and
> > then fix it up before the end of the pass?
>
> Yeah, or just walk BBs with abnormal preds, whatever tracking is
> easier.
Walking all the bbs with abnormal preds would mean I'd have to walk their
whole bodies, because the ECF_RETURNS_TWICE call is no longer at the start.
The following patch seems to work, ok for trunk if it passes full
bootstrap/regtest?
2024-03-14 Jakub Jelinek <jakub@redhat.com>
PR tree-optimization/113466
* gimple-lower-bitint.cc (bitint_large_huge): Add m_returns_twice_calls
member.
(bitint_large_huge::bitint_large_huge): Initialize it.
(bitint_large_huge::~bitint_large_huge): Release it.
(bitint_large_huge::lower_call): Remember ECF_RETURNS_TWICE call stmts
before which at least one statement has been inserted.
(gimple_lower_bitint): Move argument loads before ECF_RETURNS_TWICE
calls to a different block and add corresponding PHIs.
* gcc.dg/bitint-100.c: New test.
--- gcc/gimple-lower-bitint.cc.jj 2024-03-13 15:34:29.969725763 +0100
+++ gcc/gimple-lower-bitint.cc 2024-03-14 11:25:07.763169074 +0100
@@ -419,7 +419,8 @@ struct bitint_large_huge
bitint_large_huge ()
: m_names (NULL), m_loads (NULL), m_preserved (NULL),
m_single_use_names (NULL), m_map (NULL), m_vars (NULL),
- m_limb_type (NULL_TREE), m_data (vNULL) {}
+ m_limb_type (NULL_TREE), m_data (vNULL),
+ m_returns_twice_calls (vNULL) {}
~bitint_large_huge ();
@@ -553,6 +554,7 @@ struct bitint_large_huge
unsigned m_bitfld_load;
vec<tree> m_data;
unsigned int m_data_cnt;
+ vec<gimple *> m_returns_twice_calls;
};
bitint_large_huge::~bitint_large_huge ()
@@ -565,6 +567,7 @@ bitint_large_huge::~bitint_large_huge ()
delete_var_map (m_map);
XDELETEVEC (m_vars);
m_data.release ();
+ m_returns_twice_calls.release ();
}
/* Insert gimple statement G before current location
@@ -5248,6 +5251,7 @@ bitint_large_huge::lower_call (tree obj,
default:
break;
}
+ bool returns_twice = (gimple_call_flags (stmt) & ECF_RETURNS_TWICE) != 0;
for (unsigned int i = 0; i < nargs; ++i)
{
tree arg = gimple_call_arg (stmt, i);
@@ -5271,6 +5275,11 @@ bitint_large_huge::lower_call (tree obj,
arg = make_ssa_name (TREE_TYPE (arg));
gimple *g = gimple_build_assign (arg, v);
gsi_insert_before (&gsi, g, GSI_SAME_STMT);
+ if (returns_twice)
+ {
+ m_returns_twice_calls.safe_push (stmt);
+ returns_twice = false;
+ }
}
gimple_call_set_arg (stmt, i, arg);
if (m_preserved == NULL)
@@ -7091,6 +7100,66 @@ gimple_lower_bitint (void)
if (edge_insertions)
gsi_commit_edge_inserts ();
+ /* Fix up arguments of ECF_RETURNS_TWICE calls. Those were temporarily
+ inserted before the call, but that is invalid IL, so move them to the
+ right place and add corresponding PHIs. */
+ if (!large_huge.m_returns_twice_calls.is_empty ())
+ {
+ auto_vec<gimple *, 16> arg_stmts;
+ while (!large_huge.m_returns_twice_calls.is_empty ())
+ {
+ gimple *stmt = large_huge.m_returns_twice_calls.pop ();
+ gimple_stmt_iterator gsi = gsi_after_labels (gimple_bb (stmt));
+ while (gsi_stmt (gsi) != stmt)
+ {
+ arg_stmts.safe_push (gsi_stmt (gsi));
+ gsi_remove (&gsi, false);
+ }
+ gimple *g;
+ basic_block bb = NULL;
+ edge e = NULL, ead = NULL;
+ FOR_EACH_VEC_ELT (arg_stmts, i, g)
+ {
+ gsi_safe_insert_before (&gsi, g);
+ if (i == 0)
+ {
+ bb = gimple_bb (stmt);
+ gcc_checking_assert (EDGE_COUNT (bb->preds) == 2);
+ e = EDGE_PRED (bb, 0);
+ ead = EDGE_PRED (bb, 1);
+ if ((ead->flags & EDGE_ABNORMAL) == 0)
+ std::swap (e, ead);
+ gcc_checking_assert ((e->flags & EDGE_ABNORMAL) == 0
+ && (ead->flags & EDGE_ABNORMAL));
+ }
+ tree lhs = gimple_assign_lhs (g);
+ tree arg = lhs;
+ gphi *phi = create_phi_node (copy_ssa_name (arg), bb);
+ add_phi_arg (phi, arg, e, UNKNOWN_LOCATION);
+ tree var = create_tmp_reg (TREE_TYPE (arg));
+ suppress_warning (var, OPT_Wuninitialized);
+ arg = get_or_create_ssa_default_def (cfun, var);
+ SSA_NAME_OCCURS_IN_ABNORMAL_PHI (arg) = 1;
+ add_phi_arg (phi, arg, ead, UNKNOWN_LOCATION);
+ arg = gimple_phi_result (phi);
+ SSA_NAME_OCCURS_IN_ABNORMAL_PHI (arg) = 1;
+ imm_use_iterator iter;
+ gimple *use_stmt;
+ FOR_EACH_IMM_USE_STMT (use_stmt, iter, lhs)
+ {
+ if (use_stmt == phi)
+ continue;
+ gcc_checking_assert (use_stmt == stmt);
+ use_operand_p use_p;
+ FOR_EACH_IMM_USE_ON_STMT (use_p, iter)
+ SET_USE (use_p, arg);
+ }
+ }
+ update_stmt (stmt);
+ arg_stmts.truncate (0);
+ }
+ }
+
return ret;
}
--- gcc/testsuite/gcc.dg/bitint-100.c.jj 2024-03-14 10:32:02.432644685 +0100
+++ gcc/testsuite/gcc.dg/bitint-100.c 2024-03-14 11:21:16.444398599 +0100
@@ -0,0 +1,108 @@
+/* PR tree-optimization/113466 */
+/* { dg-do compile { target bitint575 } } */
+/* { dg-options "-O2" } */
+
+int foo (int);
+
+__attribute__((returns_twice, noipa)) _BitInt(325)
+bar (_BitInt(575) x)
+{
+ (void) x;
+ return 0wb;
+}
+
+__attribute__((returns_twice, noipa)) _BitInt(325)
+garply (_BitInt(575) x, _BitInt(575) y, _BitInt(575) z, int u, int v, _BitInt(575) w)
+{
+ (void) x;
+ (void) y;
+ (void) z;
+ (void) u;
+ (void) v;
+ (void) w;
+ return 0wb;
+}
+
+_BitInt(325)
+baz (_BitInt(575) y)
+{
+ foo (1);
+ return bar (y);
+}
+
+_BitInt(325)
+qux (int x, _BitInt(575) y)
+{
+ if (x == 25)
+ x = foo (2);
+ else if (x == 42)
+ x = foo (foo (3));
+ return bar (y);
+}
+
+void
+corge (int x, _BitInt(575) y, _BitInt(325) *z)
+{
+ void *q[] = { &&l1, &&l2, &&l3, &&l3 };
+ if (x == 25)
+ {
+ l1:
+ x = foo (2);
+ }
+ else if (x == 42)
+ {
+ l2:
+ x = foo (foo (3));
+ }
+l3:
+ *z = bar (y);
+ if (x < 4)
+ goto *q[x & 3];
+}
+
+_BitInt(325)
+freddy (int x, _BitInt(575) y)
+{
+ bar (y);
+ ++y;
+ if (x == 25)
+ x = foo (2);
+ else if (x == 42)
+ x = foo (foo (3));
+ return bar (y);
+}
+
+_BitInt(325)
+quux (_BitInt(575) x, _BitInt(575) y, _BitInt(575) z)
+{
+ _BitInt(575) w = x + y;
+ foo (1);
+ return garply (x, y, z, 42, 42, w);
+}
+
+_BitInt(325)
+grault (int x, _BitInt(575) y, _BitInt(575) z)
+{
+ _BitInt(575) v = x + y;
+ _BitInt(575) w = x - y;
+ if (x == 25)
+ x = foo (2);
+ else if (x == 42)
+ x = foo (foo (3));
+ return garply (y, z, v, 0, 0, w);
+}
+
+_BitInt(325)
+plugh (int x, _BitInt(575) y, _BitInt(575) z, _BitInt(575) v, _BitInt(575) w)
+{
+ garply (y, z, v, 1, 2, w);
+ ++y;
+ z += 2wb;
+ v <<= 3;
+ w *= 3wb;
+ if (x == 25)
+ x = foo (2);
+ else if (x == 42)
+ x = foo (foo (3));
+ return garply (y, z, v, 1, 2, w);
+}
Jakub
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] bitint, v2: Fix up adjustment of large/huge _BitInt arguments of returns_twice calls [PR113466]
2024-03-14 10:42 ` [PATCH] bitint, v2: " Jakub Jelinek
@ 2024-03-14 11:52 ` Richard Biener
0 siblings, 0 replies; 6+ messages in thread
From: Richard Biener @ 2024-03-14 11:52 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: gcc-patches
On Thu, 14 Mar 2024, Jakub Jelinek wrote:
> On Thu, Mar 14, 2024 at 09:59:12AM +0100, Richard Biener wrote:
> > On Thu, 14 Mar 2024, Jakub Jelinek wrote:
> >
> > > On Thu, Mar 14, 2024 at 09:48:45AM +0100, Richard Biener wrote:
> > > > Ugh. OK, but I wonder whether we might want to simply delay
> > > > fixing the CFG for inserts before returns-twice? Would that make
> > > > things less ugly?
> > >
> > > You mean in lower_call just remember if we added anything before
> > > ECF_RETURNS_TWICE call (or say add such stmt into a vector) and
> > > then fix it up before the end of the pass?
> >
> > Yeah, or just walk BBs with abnormal preds, whatever tracking is
> > easier.
>
> Walking all the bbs with abnormal preds would mean I'd have to walk their
> whole bodies, because the ECF_RETURNS_TWICE call is no longer at the start.
>
> The following patch seems to work, ok for trunk if it passes full
> bootstrap/regtest?
OK. I'll let you decide which variant is better maintainable
(I think this one).
Thanks,
Richard.
> 2024-03-14 Jakub Jelinek <jakub@redhat.com>
>
> PR tree-optimization/113466
> * gimple-lower-bitint.cc (bitint_large_huge): Add m_returns_twice_calls
> member.
> (bitint_large_huge::bitint_large_huge): Initialize it.
> (bitint_large_huge::~bitint_large_huge): Release it.
> (bitint_large_huge::lower_call): Remember ECF_RETURNS_TWICE call stmts
> before which at least one statement has been inserted.
> (gimple_lower_bitint): Move argument loads before ECF_RETURNS_TWICE
> calls to a different block and add corresponding PHIs.
>
> * gcc.dg/bitint-100.c: New test.
>
> --- gcc/gimple-lower-bitint.cc.jj 2024-03-13 15:34:29.969725763 +0100
> +++ gcc/gimple-lower-bitint.cc 2024-03-14 11:25:07.763169074 +0100
> @@ -419,7 +419,8 @@ struct bitint_large_huge
> bitint_large_huge ()
> : m_names (NULL), m_loads (NULL), m_preserved (NULL),
> m_single_use_names (NULL), m_map (NULL), m_vars (NULL),
> - m_limb_type (NULL_TREE), m_data (vNULL) {}
> + m_limb_type (NULL_TREE), m_data (vNULL),
> + m_returns_twice_calls (vNULL) {}
>
> ~bitint_large_huge ();
>
> @@ -553,6 +554,7 @@ struct bitint_large_huge
> unsigned m_bitfld_load;
> vec<tree> m_data;
> unsigned int m_data_cnt;
> + vec<gimple *> m_returns_twice_calls;
> };
>
> bitint_large_huge::~bitint_large_huge ()
> @@ -565,6 +567,7 @@ bitint_large_huge::~bitint_large_huge ()
> delete_var_map (m_map);
> XDELETEVEC (m_vars);
> m_data.release ();
> + m_returns_twice_calls.release ();
> }
>
> /* Insert gimple statement G before current location
> @@ -5248,6 +5251,7 @@ bitint_large_huge::lower_call (tree obj,
> default:
> break;
> }
> + bool returns_twice = (gimple_call_flags (stmt) & ECF_RETURNS_TWICE) != 0;
> for (unsigned int i = 0; i < nargs; ++i)
> {
> tree arg = gimple_call_arg (stmt, i);
> @@ -5271,6 +5275,11 @@ bitint_large_huge::lower_call (tree obj,
> arg = make_ssa_name (TREE_TYPE (arg));
> gimple *g = gimple_build_assign (arg, v);
> gsi_insert_before (&gsi, g, GSI_SAME_STMT);
> + if (returns_twice)
> + {
> + m_returns_twice_calls.safe_push (stmt);
> + returns_twice = false;
> + }
> }
> gimple_call_set_arg (stmt, i, arg);
> if (m_preserved == NULL)
> @@ -7091,6 +7100,66 @@ gimple_lower_bitint (void)
> if (edge_insertions)
> gsi_commit_edge_inserts ();
>
> + /* Fix up arguments of ECF_RETURNS_TWICE calls. Those were temporarily
> + inserted before the call, but that is invalid IL, so move them to the
> + right place and add corresponding PHIs. */
> + if (!large_huge.m_returns_twice_calls.is_empty ())
> + {
> + auto_vec<gimple *, 16> arg_stmts;
> + while (!large_huge.m_returns_twice_calls.is_empty ())
> + {
> + gimple *stmt = large_huge.m_returns_twice_calls.pop ();
> + gimple_stmt_iterator gsi = gsi_after_labels (gimple_bb (stmt));
> + while (gsi_stmt (gsi) != stmt)
> + {
> + arg_stmts.safe_push (gsi_stmt (gsi));
> + gsi_remove (&gsi, false);
> + }
> + gimple *g;
> + basic_block bb = NULL;
> + edge e = NULL, ead = NULL;
> + FOR_EACH_VEC_ELT (arg_stmts, i, g)
> + {
> + gsi_safe_insert_before (&gsi, g);
> + if (i == 0)
> + {
> + bb = gimple_bb (stmt);
> + gcc_checking_assert (EDGE_COUNT (bb->preds) == 2);
> + e = EDGE_PRED (bb, 0);
> + ead = EDGE_PRED (bb, 1);
> + if ((ead->flags & EDGE_ABNORMAL) == 0)
> + std::swap (e, ead);
> + gcc_checking_assert ((e->flags & EDGE_ABNORMAL) == 0
> + && (ead->flags & EDGE_ABNORMAL));
> + }
> + tree lhs = gimple_assign_lhs (g);
> + tree arg = lhs;
> + gphi *phi = create_phi_node (copy_ssa_name (arg), bb);
> + add_phi_arg (phi, arg, e, UNKNOWN_LOCATION);
> + tree var = create_tmp_reg (TREE_TYPE (arg));
> + suppress_warning (var, OPT_Wuninitialized);
> + arg = get_or_create_ssa_default_def (cfun, var);
> + SSA_NAME_OCCURS_IN_ABNORMAL_PHI (arg) = 1;
> + add_phi_arg (phi, arg, ead, UNKNOWN_LOCATION);
> + arg = gimple_phi_result (phi);
> + SSA_NAME_OCCURS_IN_ABNORMAL_PHI (arg) = 1;
> + imm_use_iterator iter;
> + gimple *use_stmt;
> + FOR_EACH_IMM_USE_STMT (use_stmt, iter, lhs)
> + {
> + if (use_stmt == phi)
> + continue;
> + gcc_checking_assert (use_stmt == stmt);
> + use_operand_p use_p;
> + FOR_EACH_IMM_USE_ON_STMT (use_p, iter)
> + SET_USE (use_p, arg);
> + }
> + }
> + update_stmt (stmt);
> + arg_stmts.truncate (0);
> + }
> + }
> +
> return ret;
> }
>
> --- gcc/testsuite/gcc.dg/bitint-100.c.jj 2024-03-14 10:32:02.432644685 +0100
> +++ gcc/testsuite/gcc.dg/bitint-100.c 2024-03-14 11:21:16.444398599 +0100
> @@ -0,0 +1,108 @@
> +/* PR tree-optimization/113466 */
> +/* { dg-do compile { target bitint575 } } */
> +/* { dg-options "-O2" } */
> +
> +int foo (int);
> +
> +__attribute__((returns_twice, noipa)) _BitInt(325)
> +bar (_BitInt(575) x)
> +{
> + (void) x;
> + return 0wb;
> +}
> +
> +__attribute__((returns_twice, noipa)) _BitInt(325)
> +garply (_BitInt(575) x, _BitInt(575) y, _BitInt(575) z, int u, int v, _BitInt(575) w)
> +{
> + (void) x;
> + (void) y;
> + (void) z;
> + (void) u;
> + (void) v;
> + (void) w;
> + return 0wb;
> +}
> +
> +_BitInt(325)
> +baz (_BitInt(575) y)
> +{
> + foo (1);
> + return bar (y);
> +}
> +
> +_BitInt(325)
> +qux (int x, _BitInt(575) y)
> +{
> + if (x == 25)
> + x = foo (2);
> + else if (x == 42)
> + x = foo (foo (3));
> + return bar (y);
> +}
> +
> +void
> +corge (int x, _BitInt(575) y, _BitInt(325) *z)
> +{
> + void *q[] = { &&l1, &&l2, &&l3, &&l3 };
> + if (x == 25)
> + {
> + l1:
> + x = foo (2);
> + }
> + else if (x == 42)
> + {
> + l2:
> + x = foo (foo (3));
> + }
> +l3:
> + *z = bar (y);
> + if (x < 4)
> + goto *q[x & 3];
> +}
> +
> +_BitInt(325)
> +freddy (int x, _BitInt(575) y)
> +{
> + bar (y);
> + ++y;
> + if (x == 25)
> + x = foo (2);
> + else if (x == 42)
> + x = foo (foo (3));
> + return bar (y);
> +}
> +
> +_BitInt(325)
> +quux (_BitInt(575) x, _BitInt(575) y, _BitInt(575) z)
> +{
> + _BitInt(575) w = x + y;
> + foo (1);
> + return garply (x, y, z, 42, 42, w);
> +}
> +
> +_BitInt(325)
> +grault (int x, _BitInt(575) y, _BitInt(575) z)
> +{
> + _BitInt(575) v = x + y;
> + _BitInt(575) w = x - y;
> + if (x == 25)
> + x = foo (2);
> + else if (x == 42)
> + x = foo (foo (3));
> + return garply (y, z, v, 0, 0, w);
> +}
> +
> +_BitInt(325)
> +plugh (int x, _BitInt(575) y, _BitInt(575) z, _BitInt(575) v, _BitInt(575) w)
> +{
> + garply (y, z, v, 1, 2, w);
> + ++y;
> + z += 2wb;
> + v <<= 3;
> + w *= 3wb;
> + if (x == 25)
> + x = foo (2);
> + else if (x == 42)
> + x = foo (foo (3));
> + return garply (y, z, v, 1, 2, w);
> +}
>
>
> Jakub
>
>
--
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-03-14 11:52 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-03-14 8:28 [PATCH] bitint: Fix up adjustment of large/huge _BitInt arguments of returns_twice calls [PR113466] Jakub Jelinek
2024-03-14 8:48 ` Richard Biener
2024-03-14 8:53 ` Jakub Jelinek
2024-03-14 8:59 ` Richard Biener
2024-03-14 10:42 ` [PATCH] bitint, v2: " Jakub Jelinek
2024-03-14 11:52 ` 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).