On Thu, May 8, 2025 at 6:09 AM Andrew Pinski <quic_apin...@quicinc.com> wrote:
>
> Like the patch to phiopt (r15-4033-g1f619fe25925a5f7), this adds rewriting
> of VCE to gimple_with_undefined_signed_overflow/rewrite_to_defined_overflow.
> In the case of moving VCE of a bool from being conditional to unconditional,
> it needs to be rewritten to not to use VCE but a normal cast. pr120122-1.c is
> an example of where LIM needs this rewriting. The precision of the outer type
> needs to be less then the inner one.
>
> This also renames gimple_with_undefined_signed_overflow to 
> gimple_needing_rewrite_undefined
> and rewrite_to_defined_overflow to rewrite_to_defined_unconditional as they 
> will be doing
> more than just handling signed overflow.
>
> Changes since v1:
> * v2: rename the functions.
> * v3: Add check for precision to be smaller.
>
> Bootstrappd and tested on x86_64-linux-gnu.

OK.

>         PR tree-optimization/120122
>         PR tree-optimization/116939
>
> gcc/ChangeLog:
>
>         * gimple-fold.h (gimple_with_undefined_signed_overflow): Rename to ..
>         (rewrite_to_defined_overflow): This.
>         (gimple_needing_rewrite_undefined): Rename to ...
>         (rewrite_to_defined_unconditional): this.
>         * gimple-fold.cc (gimple_with_undefined_signed_overflow): Rename to 
> ...
>         (gimple_needing_rewrite_undefined): This. Return true for VCE with 
> integral
>         types of smaller precision.
>         (rewrite_to_defined_overflow): Rename to ...
>         (rewrite_to_defined_unconditional): This. Handle VCE rewriting to a 
> cast.
>         * tree-if-conv.cc: 
> s/gimple_with_undefined_signed_overflow/gimple_needing_rewrite_undefined/
>         s/rewrite_to_defined_overflow/rewrite_to_defined_unconditional.
>         * tree-scalar-evolution.cc: Likewise
>         * tree-ssa-ifcombine.cc: Likewise.
>         * tree-ssa-loop-im.cc: Likewise.
>         * tree-ssa-loop-split.cc: Likewise.
>         * tree-ssa-reassoc.cc: Likewise.
>
> gcc/testsuite/ChangeLog:
>
>         * gcc.dg/torture/pr120122-1.c: New test.
>
> Signed-off-by: Andrew Pinski <quic_apin...@quicinc.com>
> ---
>  gcc/gimple-fold.cc                        | 56 ++++++++++++++++++-----
>  gcc/gimple-fold.h                         |  6 +--
>  gcc/testsuite/gcc.dg/torture/pr120122-1.c | 51 +++++++++++++++++++++
>  gcc/tree-if-conv.cc                       |  6 +--
>  gcc/tree-scalar-evolution.cc              |  4 +-
>  gcc/tree-ssa-ifcombine.cc                 |  4 +-
>  gcc/tree-ssa-loop-im.cc                   |  4 +-
>  gcc/tree-ssa-loop-split.cc                |  4 +-
>  gcc/tree-ssa-reassoc.cc                   |  4 +-
>  9 files changed, 111 insertions(+), 28 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.dg/torture/pr120122-1.c
>
> diff --git a/gcc/gimple-fold.cc b/gcc/gimple-fold.cc
> index 7721795b20d..fd52b58905c 100644
> --- a/gcc/gimple-fold.cc
> +++ b/gcc/gimple-fold.cc
> @@ -10592,10 +10592,12 @@ arith_code_with_undefined_signed_overflow 
> (tree_code code)
>
>  /* Return true if STMT has an operation that operates on a signed
>     integer types involves undefined behavior on overflow and the
> -   operation can be expressed with unsigned arithmetic.  */
> +   operation can be expressed with unsigned arithmetic.
> +   Also returns true if STMT is a VCE that needs to be rewritten
> +   if moved to be executed unconditionally.   */
>
>  bool
> -gimple_with_undefined_signed_overflow (gimple *stmt)
> +gimple_needing_rewrite_undefined (gimple *stmt)
>  {
>    if (!is_gimple_assign (stmt))
>      return false;
> @@ -10606,6 +10608,16 @@ gimple_with_undefined_signed_overflow (gimple *stmt)
>    if (!INTEGRAL_TYPE_P (lhs_type)
>        && !POINTER_TYPE_P (lhs_type))
>      return false;
> +  tree rhs = gimple_assign_rhs1 (stmt);
> +  /* VCE from integral types to a integral types but with
> +     a smaller precision need to be changed into casts
> +     to be well defined. */
> +  if (gimple_assign_rhs_code (stmt) == VIEW_CONVERT_EXPR
> +      && INTEGRAL_TYPE_P (TREE_TYPE (TREE_OPERAND (rhs, 0)))
> +      && is_gimple_val (TREE_OPERAND (rhs, 0))
> +      && TYPE_PRECISION (lhs_type)
> +         < TYPE_PRECISION (TREE_TYPE (TREE_OPERAND (rhs, 0))))
> +    return true;
>    if (!TYPE_OVERFLOW_UNDEFINED (lhs_type))
>      return false;
>    if (!arith_code_with_undefined_signed_overflow
> @@ -10625,19 +10637,39 @@ gimple_with_undefined_signed_overflow (gimple *stmt)
>     contain a modified form of STMT itself.  */
>
>  static gimple_seq
> -rewrite_to_defined_overflow (gimple_stmt_iterator *gsi, gimple *stmt,
> -                            bool in_place)
> +rewrite_to_defined_unconditional (gimple_stmt_iterator *gsi, gimple *stmt,
> +                                 bool in_place)
>  {
> +  gcc_assert (gimple_needing_rewrite_undefined (stmt));
>    if (dump_file && (dump_flags & TDF_DETAILS))
>      {
> -      fprintf (dump_file, "rewriting stmt with undefined signed "
> -              "overflow ");
> +      fprintf (dump_file, "rewriting stmt for being uncondtional defined");
>        print_gimple_stmt (dump_file, stmt, 0, TDF_SLIM);
>      }
> -
> +  gimple_seq stmts = NULL;
> +  /* VCE from integral types to another integral types but with
> +     smaller precisions need to be changed into casts
> +     to be well defined. */
> +  if (gimple_assign_rhs_code (stmt) == VIEW_CONVERT_EXPR)
> +    {
> +      tree rhs = gimple_assign_rhs1 (stmt);
> +      tree new_rhs = TREE_OPERAND (rhs, 0);
> +      gcc_assert (TYPE_PRECISION (TREE_TYPE (rhs))
> +                 < TYPE_PRECISION (TREE_TYPE (new_rhs)));
> +      gcc_assert (is_gimple_val (new_rhs));
> +      gimple_assign_set_rhs_code (stmt, NOP_EXPR);
> +      gimple_assign_set_rhs1 (stmt, new_rhs);
> +      if (in_place)
> +         update_stmt (stmt);
> +      else
> +       {
> +         gimple_set_modified (stmt, true);
> +         gimple_seq_add_stmt (&stmts, stmt);
> +       }
> +      return stmts;
> +    }
>    tree lhs = gimple_assign_lhs (stmt);
>    tree type = unsigned_type_for (TREE_TYPE (lhs));
> -  gimple_seq stmts = NULL;
>    if (gimple_assign_rhs_code (stmt) == ABS_EXPR)
>      gimple_assign_set_rhs_code (stmt, ABSU_EXPR);
>    else
> @@ -10672,15 +10704,15 @@ rewrite_to_defined_overflow (gimple_stmt_iterator 
> *gsi, gimple *stmt,
>  }
>
>  void
> -rewrite_to_defined_overflow (gimple_stmt_iterator *gsi)
> +rewrite_to_defined_unconditional (gimple_stmt_iterator *gsi)
>  {
> -  rewrite_to_defined_overflow (gsi, gsi_stmt (*gsi), true);
> +  rewrite_to_defined_unconditional (gsi, gsi_stmt (*gsi), true);
>  }
>
>  gimple_seq
> -rewrite_to_defined_overflow (gimple *stmt)
> +rewrite_to_defined_unconditional (gimple *stmt)
>  {
> -  return rewrite_to_defined_overflow (nullptr, stmt, false);
> +  return rewrite_to_defined_unconditional (nullptr, stmt, false);
>  }
>
>  /* The valueization hook we use for the gimple_build API simplification.
> diff --git a/gcc/gimple-fold.h b/gcc/gimple-fold.h
> index 5fcfdcda81b..afecbb8ceef 100644
> --- a/gcc/gimple-fold.h
> +++ b/gcc/gimple-fold.h
> @@ -59,9 +59,9 @@ extern tree gimple_get_virt_method_for_vtable 
> (HOST_WIDE_INT, tree,
>  extern tree gimple_fold_indirect_ref (tree);
>  extern bool gimple_fold_builtin_sprintf (gimple_stmt_iterator *);
>  extern bool gimple_fold_builtin_snprintf (gimple_stmt_iterator *);
> -extern bool gimple_with_undefined_signed_overflow (gimple *);
> -extern void rewrite_to_defined_overflow (gimple_stmt_iterator *);
> -extern gimple_seq rewrite_to_defined_overflow (gimple *);
> +extern bool gimple_needing_rewrite_undefined (gimple *);
> +extern void rewrite_to_defined_unconditional (gimple_stmt_iterator *);
> +extern gimple_seq rewrite_to_defined_unconditional (gimple *);
>  extern void replace_call_with_value (gimple_stmt_iterator *, tree);
>  extern tree tree_vec_extract (gimple_stmt_iterator *, tree, tree, tree, 
> tree);
>  extern void gsi_replace_with_seq_vops (gimple_stmt_iterator *, gimple_seq);
> diff --git a/gcc/testsuite/gcc.dg/torture/pr120122-1.c 
> b/gcc/testsuite/gcc.dg/torture/pr120122-1.c
> new file mode 100644
> index 00000000000..dde41d8ec06
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/torture/pr120122-1.c
> @@ -0,0 +1,51 @@
> +/* { dg-do run } */
> +/* PR tree-optimization/120122 */
> +
> +#include <stdbool.h>
> +
> +struct Value {
> +    int type;
> +    union {
> +        bool boolean;
> +        long long t;
> +    };
> +};
> +
> +static struct Value s_item_mem;
> +
> +/* truthy was being miscompiled for the value.type==2 case,
> +   because we would have a VCE from unsigned char to bool
> +   that went from being conditional in the value.type==1 case
> +   to unconditional when `value.type!=0`.
> +   The move of the VCE from conditional to unconditional,
> +   needs to changed into a convert (NOP_EXPR). */
> +static bool truthy(void) __attribute__((noipa));
> +static bool
> +truthy(void)
> +{
> +    bool tt = false;
> +    for(int i = 0; i < 10; i++)
> +    {
> +      struct Value value = s_item_mem;
> +      if (value.type == 0)
> +        tt = tt | 0;
> +      else if (value.type == 1)
> +        tt = tt |  value.boolean;
> +      else
> +        tt = tt |  1;
> +    }
> +    return tt;
> +}
> +
> +int
> +main(void)
> +{
> +    s_item_mem.type = 2;
> +    s_item_mem.t = -1;
> +    bool b1 = !truthy();
> +    s_item_mem.type = 1;
> +    s_item_mem.boolean = b1;
> +    bool b = truthy();
> +    if (b1 != b)  __builtin_abort();
> +    if (b) __builtin_abort();
> +}
> diff --git a/gcc/tree-if-conv.cc b/gcc/tree-if-conv.cc
> index fe8aee057b3..636361e7c36 100644
> --- a/gcc/tree-if-conv.cc
> +++ b/gcc/tree-if-conv.cc
> @@ -1066,7 +1066,7 @@ if_convertible_gimple_assign_stmt_p (gimple *stmt,
>         fprintf (dump_file, "tree could trap...\n");
>        return false;
>      }
> -  else if (gimple_with_undefined_signed_overflow (stmt))
> +  else if (gimple_needing_rewrite_undefined (stmt))
>      /* We have to rewrite stmts with undefined overflow.  */
>      need_to_rewrite_undefined = true;
>
> @@ -2881,8 +2881,8 @@ predicate_statements (loop_p loop)
>
>               gsi_replace (&gsi, new_stmt, true);
>             }
> -         else if (gimple_with_undefined_signed_overflow (stmt))
> -           rewrite_to_defined_overflow (&gsi);
> +         else if (gimple_needing_rewrite_undefined (stmt))
> +           rewrite_to_defined_unconditional (&gsi);
>           else if (gimple_vdef (stmt))
>             {
>               tree lhs = gimple_assign_lhs (stmt);
> diff --git a/gcc/tree-scalar-evolution.cc b/gcc/tree-scalar-evolution.cc
> index 9d64d3a8721..43311e52f94 100644
> --- a/gcc/tree-scalar-evolution.cc
> +++ b/gcc/tree-scalar-evolution.cc
> @@ -3932,8 +3932,8 @@ final_value_replacement_loop (class loop *loop)
>           gsi2 = gsi_start (stmts);
>           while (!gsi_end_p (gsi2))
>             {
> -             if (gimple_with_undefined_signed_overflow (gsi_stmt (gsi2)))
> -               rewrite_to_defined_overflow (&gsi2);
> +             if (gimple_needing_rewrite_undefined (gsi_stmt (gsi2)))
> +               rewrite_to_defined_unconditional (&gsi2);
>               gsi_next (&gsi2);
>             }
>         }
> diff --git a/gcc/tree-ssa-ifcombine.cc b/gcc/tree-ssa-ifcombine.cc
> index 19990d67202..1fff9234198 100644
> --- a/gcc/tree-ssa-ifcombine.cc
> +++ b/gcc/tree-ssa-ifcombine.cc
> @@ -514,9 +514,9 @@ ifcombine_mark_ssa_name_walk (tree *t, int *, void *data_)
>  static inline void
>  ifcombine_rewrite_to_defined_overflow (gimple_stmt_iterator gsi)
>  {
> -  if (!gimple_with_undefined_signed_overflow (gsi_stmt (gsi)))
> +  if (!gimple_needing_rewrite_undefined (gsi_stmt (gsi)))
>      return;
> -  rewrite_to_defined_overflow (&gsi);
> +  rewrite_to_defined_unconditional (&gsi);
>  }
>
>
> diff --git a/gcc/tree-ssa-loop-im.cc b/gcc/tree-ssa-loop-im.cc
> index ae2fd87d589..50d2ee9b99d 100644
> --- a/gcc/tree-ssa-loop-im.cc
> +++ b/gcc/tree-ssa-loop-im.cc
> @@ -1419,11 +1419,11 @@ move_computations_worker (basic_block bb)
>           when the target loop header is executed and the stmt may
>          invoke undefined integer or pointer overflow rewrite it to
>          unsigned arithmetic.  */
> -      if (gimple_with_undefined_signed_overflow (stmt)
> +      if (gimple_needing_rewrite_undefined (stmt)
>           && (!ALWAYS_EXECUTED_IN (bb)
>               || !(ALWAYS_EXECUTED_IN (bb) == level
>                    || flow_loop_nested_p (ALWAYS_EXECUTED_IN (bb), level))))
> -       gsi_insert_seq_on_edge (e, rewrite_to_defined_overflow (stmt));
> +       gsi_insert_seq_on_edge (e, rewrite_to_defined_unconditional (stmt));
>        else
>         gsi_insert_on_edge (e, stmt);
>      }
> diff --git a/gcc/tree-ssa-loop-split.cc b/gcc/tree-ssa-loop-split.cc
> index 80f488a4f9a..492abf849b8 100644
> --- a/gcc/tree-ssa-loop-split.cc
> +++ b/gcc/tree-ssa-loop-split.cc
> @@ -663,8 +663,8 @@ split_loop (class loop *loop1)
>                 gsi = gsi_start (stmts2);
>                 while (!gsi_end_p (gsi))
>                   {
> -                   if (gimple_with_undefined_signed_overflow (gsi_stmt 
> (gsi)))
> -                     rewrite_to_defined_overflow (&gsi);
> +                   if (gimple_needing_rewrite_undefined (gsi_stmt (gsi)))
> +                     rewrite_to_defined_unconditional (&gsi);
>                     gsi_next (&gsi);
>                   }
>               }
> diff --git a/gcc/tree-ssa-reassoc.cc b/gcc/tree-ssa-reassoc.cc
> index 13bb85c4b71..3c38f3d7a19 100644
> --- a/gcc/tree-ssa-reassoc.cc
> +++ b/gcc/tree-ssa-reassoc.cc
> @@ -2925,13 +2925,13 @@ update_range_test (struct range_entry *range, struct 
> range_entry *otherrange,
>          !gsi_end_p (gsi); gsi_next (&gsi))
>        {
>         gimple *stmt = gsi_stmt (gsi);
> -       if (gimple_with_undefined_signed_overflow (stmt))
> +       if (gimple_needing_rewrite_undefined (stmt))
>           {
>             gimple_stmt_iterator gsip = gsi;
>             gimple_stmt_iterator gsin = gsi;
>             gsi_prev (&gsip);
>             gsi_next (&gsin);
> -           rewrite_to_defined_overflow (&gsi);
> +           rewrite_to_defined_unconditional (&gsi);
>             unsigned uid = gimple_uid (stmt);
>             if (gsi_end_p (gsip))
>               gsip = gsi_after_labels (bb);
> --
> 2.34.1
>

Reply via email to