On December 13, 2020 12:02:48 PM GMT+01:00, Jakub Jelinek <ja...@redhat.com>
wrote:
>Hi!
>
>Unfortunately, my latest tree-ssa-math-opts.c patch broke the following
>testcase. The problem is that the code is adding .ADD_OVERFLOW or
>.SUB_OVERFLOW before or after the stmt on which the function has been
>called, which is normally a addition or subtraction that has all the
>operands.
>But in the a > ~b optimization that stmt is the ~b stmt and the other
>comparison operand might be defined only after that ~b stmt, so we
>can't
>insert the .ADD_OVERFLOW next to ~b that we want to delete, but need to
>insert it before the a > temp comparison that uses it; and in that case
>when removing the BIT_NOT_EXPR stmt we need to ensure the caller
>doesn't do
>gsi_next because gsi_remove already points the iterator to the next
>stmt.
>
>Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
Ok.
Richard.
>2020-12-13 Jakub Jelinek <ja...@redhat.com>
>
> PR tree-optimization/98256
> * tree-ssa-math-opts.c (match_uaddsub_overflow): For BIT_NOT_EXPR,
> only handle a single use, and insert .ADD_OVERFLOW before the
> comparison rather than after the BIT_NOT_EXPR. Return true iff
> it is BIT_NOT_EXPR and it has been removed.
> (math_opts_dom_walker::after_dom_children) <case BIT_NOT_EXPR>:
> If match_uaddsub_overflow returned true, continue instead of break.
>
> * gcc.c-torture/compile/pr98256.c: New test.
>
>--- gcc/tree-ssa-math-opts.c.jj 2020-12-12 14:48:40.195306781 +0100
>+++ gcc/tree-ssa-math-opts.c 2020-12-12 22:42:24.474396562 +0100
>@@ -3598,6 +3598,7 @@ match_uaddsub_overflow (gimple_stmt_iter
> gimple *use_stmt;
> gimple *add_stmt = NULL;
> bool add_first = false;
>+ gimple *cond_stmt = NULL;
>
> gcc_checking_assert (code == PLUS_EXPR
> || code == MINUS_EXPR
>@@ -3628,8 +3629,9 @@ match_uaddsub_overflow (gimple_stmt_iter
> return false;
> if (rhs2 == NULL)
> rhs2 = other;
>- else if (rhs2 != other)
>+ else
> return false;
>+ cond_stmt = use_stmt;
> }
> ovf_use_seen = true;
> }
>@@ -3818,6 +3820,9 @@ match_uaddsub_overflow (gimple_stmt_iter
> }
> }
>
>+ if (code == BIT_NOT_EXPR)
>+ *gsi = gsi_for_stmt (cond_stmt);
>+
> tree ctype = build_complex_type (type);
> gcall *g = gimple_build_call_internal (code != MINUS_EXPR
> ? IFN_ADD_OVERFLOW : IFN_SUB_OVERFLOW,
>@@ -3843,7 +3848,10 @@ match_uaddsub_overflow (gimple_stmt_iter
> tree ovf = make_ssa_name (type);
> g2 = gimple_build_assign (ovf, IMAGPART_EXPR,
> build1 (IMAGPART_EXPR, type, ctmp));
>- gsi_insert_after (gsi, g2, GSI_NEW_STMT);
>+ if (code != BIT_NOT_EXPR)
>+ gsi_insert_after (gsi, g2, GSI_NEW_STMT);
>+ else
>+ gsi_insert_before (gsi, g2, GSI_SAME_STMT);
>
> FOR_EACH_IMM_USE_STMT (use_stmt, iter, lhs)
> {
>@@ -3908,11 +3916,12 @@ match_uaddsub_overflow (gimple_stmt_iter
> }
> else if (code == BIT_NOT_EXPR)
> {
>- gimple_stmt_iterator gsi2 = gsi_for_stmt (stmt);
>- gsi_remove (&gsi2, true);
>+ *gsi = gsi_for_stmt (stmt);
>+ gsi_remove (gsi, true);
> release_ssa_name (lhs);
>+ return true;
> }
>- return true;
>+ return false;
> }
>
> /* Return true if target has support for divmod. */
>@@ -4238,7 +4247,8 @@ math_opts_dom_walker::after_dom_children
> break;
>
> case BIT_NOT_EXPR:
>- match_uaddsub_overflow (&gsi, stmt, code);
>+ if (match_uaddsub_overflow (&gsi, stmt, code))
>+ continue;
> break;
>
> case TRUNC_MOD_EXPR:
>--- gcc/testsuite/gcc.c-torture/compile/pr98256.c.jj 2020-12-12
>22:44:34.037940761 +0100
>+++ gcc/testsuite/gcc.c-torture/compile/pr98256.c 2020-12-12
>22:35:54.452778912 +0100
>@@ -0,0 +1,9 @@
>+/* PR tree-optimization/98256 */
>+
>+unsigned a, b;
>+
>+int
>+foo (void)
>+{
>+ return !!(~a / b);
>+}
>
> Jakub
