On Wed, Oct 2, 2024 at 1:11 AM Andrew Pinski <quic_apin...@quicinc.com> wrote: > > Phiopt match_and_simplify might move a well defined VCE assign statement > from being conditional to being uncondtitional; that VCE might no longer > being defined. It will need a rewrite into a cast instead. > > This adds the rewriting code to move_stmt for the VCE case.
Indeed. > This is enough to fix the issue at hand. It should also be using > rewrite_to_defined_overflow > but first I need to move the check to see a rewrite is needed into its own > function > and that is causing issues (see > https://gcc.gnu.org/pipermail/gcc-patches/2024-September/663938.html). > Plus this version is easiest to backport. OK. Thanks, Richard. > Bootstrapped and tested on x86_64-linux-gnu. > > PR tree-optimization/116098 > > gcc/ChangeLog: > > * tree-ssa-phiopt.cc (move_stmt): Rewrite VCEs from integer to integer > types to case. > > gcc/testsuite/ChangeLog: > > * c-c++-common/torture/pr116098-2.c: New test. > * g++.dg/torture/pr116098-1.C: New test. > > Signed-off-by: Andrew Pinski <quic_apin...@quicinc.com> > --- > .../c-c++-common/torture/pr116098-2.c | 46 +++++++++++++++++++ > gcc/testsuite/g++.dg/torture/pr116098-1.C | 33 +++++++++++++ > gcc/tree-ssa-phiopt.cc | 28 ++++++++++- > 3 files changed, 106 insertions(+), 1 deletion(-) > create mode 100644 gcc/testsuite/c-c++-common/torture/pr116098-2.c > create mode 100644 gcc/testsuite/g++.dg/torture/pr116098-1.C > > diff --git a/gcc/testsuite/c-c++-common/torture/pr116098-2.c > b/gcc/testsuite/c-c++-common/torture/pr116098-2.c > new file mode 100644 > index 00000000000..614ed049171 > --- /dev/null > +++ b/gcc/testsuite/c-c++-common/torture/pr116098-2.c > @@ -0,0 +1,46 @@ > +/* { dg-do run } */ > +/* PR tree-optimization/116098 */ > + > + > +#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) > +{ > + struct Value value = s_item_mem; > + if (value.type == 0) > + return 0; > + if (value.type == 1) > + return value.boolean; > + return 1; > +} > + > +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/testsuite/g++.dg/torture/pr116098-1.C > b/gcc/testsuite/g++.dg/torture/pr116098-1.C > new file mode 100644 > index 00000000000..90e44a6eeed > --- /dev/null > +++ b/gcc/testsuite/g++.dg/torture/pr116098-1.C > @@ -0,0 +1,33 @@ > +// { dg-do run } > +/* PR tree-optimization/116098 */ > + > + > +static bool truthy(int type, unsigned char data) __attribute__((noipa)); > +/* truthy was being miscompiled for the type==2 case, > + because we would have a VCE from unsigned char to bool > + that went from being conditional in the type==1 case > + to unconditional when `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(int type, unsigned char data) > +{ > + if (type == 0) > + return 0; > + if (type == 1) > + /* Emulate what SRA does, so this can be > + tested without depending on SRA. */ > + return __builtin_bit_cast (bool, data); > + return 1; > +} > + > +int > +main(void) > +{ > + bool b1 = !truthy(2, -1); > + bool b = truthy(1, b1); > + if (b1 != b) __builtin_abort(); > + if (b) __builtin_abort(); > +} > diff --git a/gcc/tree-ssa-phiopt.cc b/gcc/tree-ssa-phiopt.cc > index bd7f9607eb9..43b65b362a3 100644 > --- a/gcc/tree-ssa-phiopt.cc > +++ b/gcc/tree-ssa-phiopt.cc > @@ -742,7 +742,8 @@ empty_bb_or_one_feeding_into_p (basic_block bb, > } > > /* Move STMT to before GSI and insert its defining > - name into INSERTED_EXPRS bitmap. */ > + name into INSERTED_EXPRS bitmap. > + Also rewrite its if it might be undefined when unconditionalized. */ > static void > move_stmt (gimple *stmt, gimple_stmt_iterator *gsi, auto_bitmap > &inserted_exprs) > { > @@ -761,6 +762,31 @@ move_stmt (gimple *stmt, gimple_stmt_iterator *gsi, > auto_bitmap &inserted_exprs) > gimple_stmt_iterator gsi1 = gsi_for_stmt (stmt); > gsi_move_before (&gsi1, gsi); > reset_flow_sensitive_info (name); > + > + /* Rewrite some code which might be undefined when > + unconditionalized. */ > + if (gimple_assign_single_p (stmt)) > + { > + 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 when unconditional. */ > + if (gimple_assign_rhs_code (stmt) == VIEW_CONVERT_EXPR > + && INTEGRAL_TYPE_P (TREE_TYPE (name)) > + && INTEGRAL_TYPE_P (TREE_TYPE (TREE_OPERAND (rhs, 0)))) > + { > + if (dump_file && (dump_flags & TDF_DETAILS)) > + { > + fprintf (dump_file, "rewriting stmt with maybe undefined VCE "); > + print_gimple_stmt (dump_file, stmt, 0, TDF_SLIM); > + } > + 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); > + update_stmt (stmt); > + } > + } > } > > /* RAII style class to temporarily remove flow sensitive > -- > 2.34.1 >
