On Wed, May 7, 2025 at 3:55 AM Andrew Pinski <[email protected]> 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.
>
> 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.
>
> Bootstrappd and tested on x86_64-linux-gnu.
>
> 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.
> (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 <[email protected]>
> ---
> gcc/gimple-fold.cc | 51 +++++++++++++++++------
> 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, 106 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 c060ef81a42..4f45aeb7ff8 100644
> --- a/gcc/gimple-fold.cc
> +++ b/gcc/gimple-fold.cc
> @@ -10588,10 +10588,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;
> @@ -10602,6 +10604,14 @@ 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 another integral types but with
> + different precisions 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)))
So this does not perform the precision check and IMO we should never end
up with a V_C_E from a lower-precision operand to a higher precision. So
does it work with an additional
&& TYPE_PRECISION (TREE_TYPE (TREE_OPERAND (rhs, 0))
> TYPE_PRECISION (lhs_type)
check?
> + && is_gimple_val (TREE_OPERAND (rhs, 0)))
> + return true;
> if (!TYPE_OVERFLOW_UNDEFINED (lhs_type))
> return false;
> if (!arith_code_with_undefined_signed_overflow
> @@ -10621,19 +10631,36 @@ 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)
> {
> 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
> + different 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 (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
> @@ -10668,15 +10695,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
>
