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 >