bitint, v2: Fix up adjustment of large/huge _BitInt arguments of returns_twice calls [PR113466]
Checks
Context |
Check |
Description |
linaro-tcwg-bot/tcwg_gcc_build--master-arm |
success
|
Testing passed
|
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 |
success
|
Testing passed
|
linaro-tcwg-bot/tcwg_gcc_check--master-arm |
success
|
Testing passed
|
linaro-tcwg-bot/tcwg_gcc_check--master-aarch64 |
success
|
Testing passed
|
Commit Message
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.
Jakub
Comments
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
>
>
@@ -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;
}
@@ -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);
+}