On Wed, 13 Dec 2023, Jakub Jelinek wrote:
> Hi!
>
> The following testcase ICEs, because a PHI argument from latch edge
> uses a SSA_NAME set only in a conditionally executed block inside of the
> loop.
> This happens when we have some outer cast which lowers its operand several
> times, under some condition with variable index, under different condition
> with some constant index, otherwise something else, and then there is
> an inner cast from non-_BitInt integer (or small/middle one). Such cast
> in certain conditions is emitted by initializing some SSA_NAMEs in the
> initialization statements before loops (say for casts from <= limb size
> precision by computing a SSA_NAME for the first limb and then extension
> of it for the later limbs) and uses the prepare_data_in_out function
> to create a PHI node. Such function is passed the value (constant or
> SSA_NAME) to use in the PHI argument from the pre-header edge, but for
> the latch edge it always created a new SSA_NAME and then caller emitted
> in the following 3 spots an extra assignment to set that SSA_NAME to
> whatever value we want from the latch edge. In all these 3 cases
> the argument from the latch edge is known already before the loop though,
> either constant or SSA_NAME computed in pre-header as well.
> But the need to emit an assignment combined with the handle_operand done
> in a conditional basic block results in the SSA verification failure.
>
> The following patch fixes it by extending the prpare_data_in_out method,
> so that when the latch edge argument is known before (constant or computed
> in pre-header), we can just use it directly and avoid the extra assignment
> that would normally be hopefully optimized away later to what we now emit
> directly.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
OK.
Richard.
> 2023-12-13 Jakub Jelinek <ja...@redhat.com>
>
> PR tree-optimization/112940
> * gimple-lower-bitint.cc (struct bitint_large_huge): Add another
> argument to prepare_data_in_out method defaulted to NULL_TREE.
> (bitint_large_huge::handle_operand): Pass another argument to
> prepare_data_in_out instead of emitting an assignment to set it.
> (bitint_large_huge::prepare_data_in_out): Add VAL_OUT argument.
> If non-NULL, use it as PHI argument instead of creating a new
> SSA_NAME.
> (bitint_large_huge::handle_cast): Pass rext as another argument
> to 2 prepare_data_in_out calls instead of emitting assignments
> to set them.
>
> * gcc.dg/bitint-53.c: New test.
>
> --- gcc/gimple-lower-bitint.cc.jj 2023-12-08 09:03:06.644475539 +0100
> +++ gcc/gimple-lower-bitint.cc 2023-12-12 18:31:24.440394239 +0100
> @@ -405,7 +405,7 @@ struct bitint_large_huge
> profile_probability, profile_probability,
> edge &, edge &, edge &);
> tree handle_operand (tree, tree);
> - tree prepare_data_in_out (tree, tree, tree *);
> + tree prepare_data_in_out (tree, tree, tree *, tree = NULL_TREE);
> tree add_cast (tree, tree);
> tree handle_plus_minus (tree_code, tree, tree, tree);
> tree handle_lshift (tree, tree, tree);
> @@ -872,11 +872,8 @@ bitint_large_huge::handle_operand (tree
> gcc_assert (m_first);
> m_data.pop ();
> m_data.pop ();
> - prepare_data_in_out (fold_convert (m_limb_type, op), idx, &out);
> - g = gimple_build_assign (m_data[m_data_cnt + 1],
> - build_int_cst (m_limb_type, ext));
> - insert_before (g);
> - m_data[m_data_cnt + 1] = gimple_assign_rhs1 (g);
> + prepare_data_in_out (fold_convert (m_limb_type, op), idx, &out,
> + build_int_cst (m_limb_type, ext));
> }
> else if (min_prec > prec - rem - 2 * limb_prec)
> {
> @@ -1008,10 +1005,13 @@ bitint_large_huge::handle_operand (tree
>
> /* Helper method, add a PHI node with VAL from preheader edge if
> inside of a loop and m_first. Keep state in a pair of m_data
> - elements. */
> + elements. If VAL_OUT is non-NULL, use that as PHI argument from
> + the latch edge, otherwise create a new SSA_NAME for it and let
> + caller initialize it. */
>
> tree
> -bitint_large_huge::prepare_data_in_out (tree val, tree idx, tree *data_out)
> +bitint_large_huge::prepare_data_in_out (tree val, tree idx, tree *data_out,
> + tree val_out)
> {
> if (!m_first)
> {
> @@ -1034,7 +1034,7 @@ bitint_large_huge::prepare_data_in_out (
> if (e1 == e2)
> e2 = EDGE_PRED (m_bb, 1);
> add_phi_arg (phi, val, e1, UNKNOWN_LOCATION);
> - tree out = make_ssa_name (TREE_TYPE (val));
> + tree out = val_out ? val_out : make_ssa_name (TREE_TYPE (val));
> add_phi_arg (phi, out, e2, UNKNOWN_LOCATION);
> m_data.safe_push (in);
> m_data.safe_push (out);
> @@ -1541,14 +1541,10 @@ bitint_large_huge::handle_cast (tree lhs
> if (m_first)
> {
> tree out1, out2;
> - prepare_data_in_out (r1, idx, &out1);
> - g = gimple_build_assign (m_data[m_data_cnt + 1], rext);
> - insert_before (g);
> + prepare_data_in_out (r1, idx, &out1, rext);
> if (TYPE_PRECISION (rhs_type) > limb_prec)
> {
> - prepare_data_in_out (r2, idx, &out2);
> - g = gimple_build_assign (m_data[m_data_cnt + 3], rext);
> - insert_before (g);
> + prepare_data_in_out (r2, idx, &out2, rext);
> m_data.pop ();
> t = m_data.pop ();
> m_data[m_data_cnt + 1] = t;
> --- gcc/testsuite/gcc.dg/bitint-53.c.jj 2023-12-12 18:28:16.203949817
> +0100
> +++ gcc/testsuite/gcc.dg/bitint-53.c 2023-12-12 18:27:47.307342133 +0100
> @@ -0,0 +1,17 @@
> +/* PR tree-optimization/112940 */
> +/* { dg-do compile { target bitint } } */
> +/* { dg-options "-std=c23 -O2" } */
> +
> +#if __BITINT_MAXWIDTH__ >= 1025
> +_BitInt (1025) b;
> +#endif
> +
> +void
> +foo (long x)
> +{
> +#if __BITINT_MAXWIDTH__ >= 1025
> + b += (unsigned _BitInt (255)) x;
> +#else
> + (void) x;
> +#endif
> +}
>
> Jakub
>
>
--
Richard Biener <rguent...@suse.de>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)
