On Mon, Aug 7, 2023 at 4:46 PM Andrew Pinski via Gcc-patches
<[email protected]> wrote:
>
> This patch fixes the oldish (2006) bug where VRP was not
> optimizing the comparison for assignments while handling
> them for GIMPLE_COND only.
> It just happens to also solves PR 103281 due to allowing
> to optimize `c < 1` to `c == 0` and then we get
> `(c == 0) == c` (which was handled by r14-2501-g285c9d04).
>
> OK? Bootstrapped and tested on x86_64-linux-gnu with no
> regressions.
OK.
Thanks,
Richard.
> PR tree-optmization/103281
> PR tree-optimization/28794
>
> gcc/ChangeLog:
>
> * vr-values.cc (simplify_using_ranges::simplify_cond_using_ranges_1):
> Split out
> majority to ...
> (simplify_using_ranges::simplify_compare_using_ranges_1): Here.
> (simplify_using_ranges::simplify_casted_cond): Rename to ...
> (simplify_using_ranges::simplify_casted_compare): This
> and change arguments to take op0 and op1.
> (simplify_using_ranges::simplify_compare_assign_using_ranges_1): New
> method.
> (simplify_using_ranges::simplify): For tcc_comparison assignments call
> simplify_compare_assign_using_ranges_1.
> * vr-values.h (simplify_using_ranges): Add
> new methods, simplify_compare_using_ranges_1 and
> simplify_compare_assign_using_ranges_1.
> Rename simplify_casted_cond and simplify_casted_compare and
> update argument types.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.dg/tree-ssa/pr103281-1.c: New test.
> * gcc.dg/tree-ssa/vrp-compare-1.c: New test.
> ---
> gcc/testsuite/gcc.dg/tree-ssa/pr103281-1.c | 19 +++
> gcc/testsuite/gcc.dg/tree-ssa/vrp-compare-1.c | 13 ++
> gcc/vr-values.cc | 160 +++++++++++-------
> gcc/vr-values.h | 4 +-
> 4 files changed, 134 insertions(+), 62 deletions(-)
> create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr103281-1.c
> create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/vrp-compare-1.c
>
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr103281-1.c
> b/gcc/testsuite/gcc.dg/tree-ssa/pr103281-1.c
> new file mode 100644
> index 00000000000..09964d0b46b
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr103281-1.c
> @@ -0,0 +1,19 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fdump-tree-optimized" } */
> +/* PR tree-optmization/103281 */
> +
> +void foo(void);
> +
> +static unsigned b;
> +
> +int main() {
> + for (; b < 3; b++) {
> + char c = b;
> + char a = c ? c : c << 1;
> + if (!(a < 1 ^ b))
> + foo();
> + }
> +}
> +
> +/* the call to foo should be optimized away. */
> +/* { dg-final { scan-tree-dump-not "foo " "optimized" } } */
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/vrp-compare-1.c
> b/gcc/testsuite/gcc.dg/tree-ssa/vrp-compare-1.c
> new file mode 100644
> index 00000000000..9889cf34706
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/vrp-compare-1.c
> @@ -0,0 +1,13 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fdump-tree-evrp-details" } */
> +/* PR tree-optimization/28794 */
> +
> +void g(int);
> +void f1(int x)
> +{
> + if (x < 0) return;
> + g(x>0);
> +}
> +
> +/* `x > 0` should be optimized to just `x != 0` */
> +/* { dg-final { scan-tree-dump-times "Simplified relational" 1 "evrp" } } */
> diff --git a/gcc/vr-values.cc b/gcc/vr-values.cc
> index ac4a83c6097..a4fddd62841 100644
> --- a/gcc/vr-values.cc
> +++ b/gcc/vr-values.cc
> @@ -1139,6 +1139,87 @@ simplify_using_ranges::simplify_cond_using_ranges_1
> (gcond *stmt)
> if (fold_cond (stmt))
> return true;
>
> + if (simplify_compare_using_ranges_1 (cond_code, op0, op1, stmt))
> + {
> + if (dump_file)
> + {
> + fprintf (dump_file, "Simplified relational ");
> + print_gimple_stmt (dump_file, stmt, 0);
> + fprintf (dump_file, " into ");
> + }
> +
> + gimple_cond_set_code (stmt, cond_code);
> + gimple_cond_set_lhs (stmt, op0);
> + gimple_cond_set_rhs (stmt, op1);
> +
> + update_stmt (stmt);
> +
> + if (dump_file)
> + {
> + print_gimple_stmt (dump_file, stmt, 0);
> + fprintf (dump_file, "\n");
> + }
> + return true;
> + }
> + return false;
> +}
> +
> +/* Like simplify_cond_using_ranges_1 but for assignments rather
> + than GIMPLE_COND. */
> +
> +bool
> +simplify_using_ranges::simplify_compare_assign_using_ranges_1
> + (gimple_stmt_iterator *gsi,
> + gimple *stmt)
> +{
> + enum tree_code code = gimple_assign_rhs_code (stmt);
> + tree op0 = gimple_assign_rhs1 (stmt);
> + tree op1 = gimple_assign_rhs2 (stmt);
> + gcc_assert (TREE_CODE_CLASS (code) == tcc_comparison);
> + bool happened = false;
> +
> + if (simplify_compare_using_ranges_1 (code, op0, op1, stmt))
> + {
> + if (dump_file)
> + {
> + fprintf (dump_file, "Simplified relational ");
> + print_gimple_stmt (dump_file, stmt, 0);
> + fprintf (dump_file, " into ");
> + }
> +
> + gimple_assign_set_rhs_code (stmt, code);
> + gimple_assign_set_rhs1 (stmt, op0);
> + gimple_assign_set_rhs2 (stmt, op1);
> +
> + update_stmt (stmt);
> +
> + if (dump_file)
> + {
> + print_gimple_stmt (dump_file, stmt, 0);
> + fprintf (dump_file, "\n");
> + }
> + happened = true;
> + }
> +
> + /* Transform EQ_EXPR, NE_EXPR into BIT_XOR_EXPR or identity
> + if the RHS is zero or one, and the LHS are known to be boolean
> + values. */
> + if ((code == EQ_EXPR || code == NE_EXPR)
> + && INTEGRAL_TYPE_P (TREE_TYPE (op0))
> + && simplify_truth_ops_using_ranges (gsi, stmt))
> + happened = true;
> +
> + return happened;
> +}
> +
> +/* Try to simplify OP0 COND_CODE OP1 using a relational operator to an
> + equality test if the range information indicates only one value can
> + satisfy the original conditional. */
> +
> +bool
> +simplify_using_ranges::simplify_compare_using_ranges_1 (tree_code
> &cond_code, tree &op0, tree &op1, gimple *stmt)
> +{
> + bool happened = false;
> if (cond_code != NE_EXPR
> && cond_code != EQ_EXPR
> && TREE_CODE (op0) == SSA_NAME
> @@ -1157,26 +1238,9 @@ simplify_using_ranges::simplify_cond_using_ranges_1
> (gcond *stmt)
> tree new_tree = test_for_singularity (cond_code, op0, op1, &vr);
> if (new_tree)
> {
> - if (dump_file)
> - {
> - fprintf (dump_file, "Simplified relational ");
> - print_gimple_stmt (dump_file, stmt, 0);
> - fprintf (dump_file, " into ");
> - }
> -
> - gimple_cond_set_code (stmt, EQ_EXPR);
> - gimple_cond_set_lhs (stmt, op0);
> - gimple_cond_set_rhs (stmt, new_tree);
> -
> - update_stmt (stmt);
> -
> - if (dump_file)
> - {
> - print_gimple_stmt (dump_file, stmt, 0);
> - fprintf (dump_file, "\n");
> - }
> -
> - return true;
> + cond_code = EQ_EXPR;
> + op1 = new_tree;
> + happened = true;
> }
>
> /* Try again after inverting the condition. We only deal
> @@ -1187,45 +1251,25 @@ simplify_using_ranges::simplify_cond_using_ranges_1
> (gcond *stmt)
> op0, op1, &vr);
> if (new_tree)
> {
> - if (dump_file)
> - {
> - fprintf (dump_file, "Simplified relational ");
> - print_gimple_stmt (dump_file, stmt, 0);
> - fprintf (dump_file, " into ");
> - }
> -
> - gimple_cond_set_code (stmt, NE_EXPR);
> - gimple_cond_set_lhs (stmt, op0);
> - gimple_cond_set_rhs (stmt, new_tree);
> -
> - update_stmt (stmt);
> -
> - if (dump_file)
> - {
> - print_gimple_stmt (dump_file, stmt, 0);
> - fprintf (dump_file, "\n");
> - }
> -
> - return true;
> + cond_code = NE_EXPR;
> + op1 = new_tree;
> + happened = true;
> }
> }
> }
> // Try to simplify casted conditions.
> - return simplify_casted_cond (stmt);
> + if (simplify_casted_compare (cond_code, op0, op1))
> + happened = true;
> + return happened;
> }
>
> -/* STMT is a conditional at the end of a basic block.
> -
> - If the conditional is of the form SSA_NAME op constant and the SSA_NAME
> - was set via a type conversion, try to replace the SSA_NAME with the RHS
> - of the type conversion. Doing so makes the conversion dead which helps
> - subsequent passes. */
> +/* Simplify OP0 code OP1 when OP1 is a constant and OP0 was a SSA_NAME
> + defined by a type conversion. Replacing OP0 with RHS of the type
> conversion.
> + Doing so makes the conversion dead which helps subsequent passes. */
>
> bool
> -simplify_using_ranges::simplify_casted_cond (gcond *stmt)
> +simplify_using_ranges::simplify_casted_compare (tree_code &, tree &op0, tree
> &op1)
> {
> - tree op0 = gimple_cond_lhs (stmt);
> - tree op1 = gimple_cond_rhs (stmt);
>
> /* If we have a comparison of an SSA_NAME (OP0) against a constant,
> see if OP0 was set by a type conversion where the source of
> @@ -1274,9 +1318,8 @@ simplify_using_ranges::simplify_casted_cond (gcond
> *stmt)
> && int_fits_type_p (op1, TREE_TYPE (innerop)))
> {
> tree newconst = fold_convert (TREE_TYPE (innerop), op1);
> - gimple_cond_set_lhs (stmt, innerop);
> - gimple_cond_set_rhs (stmt, newconst);
> - update_stmt (stmt);
> + op0 = innerop;
> + op1 = newconst;
> return true;
> }
> }
> @@ -1880,16 +1923,11 @@ simplify_using_ranges::simplify (gimple_stmt_iterator
> *gsi)
> }
> }
>
> + if (TREE_CODE_CLASS (rhs_code) == tcc_comparison)
> + return simplify_compare_assign_using_ranges_1 (gsi, stmt);
> +
> switch (rhs_code)
> {
> - case EQ_EXPR:
> - case NE_EXPR:
> - /* Transform EQ_EXPR, NE_EXPR into BIT_XOR_EXPR or identity
> - if the RHS is zero or one, and the LHS are known to be boolean
> - values. */
> - if (INTEGRAL_TYPE_P (TREE_TYPE (rhs1)))
> - return simplify_truth_ops_using_ranges (gsi, stmt);
> - break;
>
> /* Transform TRUNC_DIV_EXPR and TRUNC_MOD_EXPR into RSHIFT_EXPR
> and BIT_AND_EXPR respectively if the first operand is greater
> diff --git a/gcc/vr-values.h b/gcc/vr-values.h
> index df79a3a570b..bc17aa1af15 100644
> --- a/gcc/vr-values.h
> +++ b/gcc/vr-values.h
> @@ -38,13 +38,15 @@ private:
> void legacy_fold_cond (gcond *, edge *);
> tree legacy_fold_cond_overflow (gimple *stmt);
> tree fold_cond_with_ops (tree_code, tree, tree, gimple *s);
> - bool simplify_casted_cond (gcond *);
> + bool simplify_casted_compare (tree_code &cond_code, tree &op0, tree &op1);
> bool simplify_truth_ops_using_ranges (gimple_stmt_iterator *, gimple *);
> bool simplify_div_or_mod_using_ranges (gimple_stmt_iterator *, gimple *);
> bool simplify_abs_using_ranges (gimple_stmt_iterator *, gimple *);
> bool simplify_bit_ops_using_ranges (gimple_stmt_iterator *, gimple *);
> bool simplify_min_or_max_using_ranges (gimple_stmt_iterator *, gimple *);
> bool simplify_cond_using_ranges_1 (gcond *);
> + bool simplify_compare_using_ranges_1 (tree_code &, tree &, tree &, gimple
> *);
> + bool simplify_compare_assign_using_ranges_1 (gimple_stmt_iterator *,
> gimple *);
> bool simplify_switch_using_ranges (gswitch *);
> bool simplify_float_conversion_using_ranges (gimple_stmt_iterator *,
> gimple *);
> --
> 2.31.1
>
