public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Richard Biener <richard.guenther@gmail.com>
To: Aldy Hernandez <aldyh@redhat.com>
Cc: GCC patches <gcc-patches@gcc.gnu.org>,
	Andrew MacLeod <amacleod@redhat.com>
Subject: Re: [PATCH] Remove ASSERT_EXPR.
Date: Wed, 23 Nov 2022 21:27:45 +0100	[thread overview]
Message-ID: <CAFiYyc0j7=BMfobtcnP==eosO7qL+YQ-VEToH-=9vczR-27VzA@mail.gmail.com> (raw)
In-Reply-To: <20221122135801.1945438-1-aldyh@redhat.com>

On Tue, Nov 22, 2022 at 2:58 PM Aldy Hernandez <aldyh@redhat.com> wrote:
>
> This removes all uses of ASSERT_EXPR except the internal one in ipa-*.
>
> OK pending tests?

OK.

> gcc/ChangeLog:
>
>         * doc/gimple.texi: Remove ASSERT_EXPR references.
>         * fold-const.cc (tree_expr_nonzero_warnv_p): Same.
>         (fold_binary_loc): Same.
>         (tree_expr_nonnegative_warnv_p): Same.
>         * gimple-array-bounds.cc (get_base_decl): Same.
>         * gimple-pretty-print.cc (dump_unary_rhs): Same.
>         * gimple.cc (get_gimple_rhs_num_ops): Same.
>         * pointer-query.cc (handle_ssa_name): Same.
>         * tree-cfg.cc (verify_gimple_assign_single): Same.
>         * tree-pretty-print.cc (dump_generic_node): Same.
>         * tree-scalar-evolution.cc (scev_dfs::follow_ssa_edge_expr):Same.
>         (interpret_rhs_expr): Same.
>         * tree-ssa-operands.cc (operands_scanner::get_expr_operands): Same.
>         * tree-ssa-propagate.cc
>         (substitute_and_fold_dom_walker::before_dom_children): Same.
>         * tree-ssa-threadedge.cc: Same.
>         * tree-vrp.cc (overflow_comparison_p): Same.
>         * tree.def (ASSERT_EXPR): Add note.
>         * tree.h (ASSERT_EXPR_VAR): Remove.
>         (ASSERT_EXPR_COND): Remove.
>         * vr-values.cc (simplify_using_ranges::vrp_visit_cond_stmt):
>         Remove comment.
> ---
>  gcc/doc/gimple.texi          |  3 +--
>  gcc/fold-const.cc            |  6 -----
>  gcc/gimple-array-bounds.cc   |  9 +-------
>  gcc/gimple-pretty-print.cc   |  1 -
>  gcc/gimple.cc                |  1 -
>  gcc/pointer-query.cc         |  6 -----
>  gcc/tree-cfg.cc              | 11 ---------
>  gcc/tree-pretty-print.cc     |  8 -------
>  gcc/tree-scalar-evolution.cc | 15 -------------
>  gcc/tree-ssa-operands.cc     |  1 -
>  gcc/tree-ssa-propagate.cc    |  5 +----
>  gcc/tree-ssa-threadedge.cc   |  6 ++---
>  gcc/tree-vrp.cc              |  7 +++---
>  gcc/tree.def                 |  5 ++++-
>  gcc/tree.h                   |  4 ----
>  gcc/vr-values.cc             | 43 ------------------------------------
>  16 files changed, 13 insertions(+), 118 deletions(-)
>
> diff --git a/gcc/doc/gimple.texi b/gcc/doc/gimple.texi
> index 7832fa6ff90..a4263922887 100644
> --- a/gcc/doc/gimple.texi
> +++ b/gcc/doc/gimple.texi
> @@ -682,8 +682,7 @@ more than two slots on the RHS.  For instance, a @code{COND_EXPR}
>  expression of the form @code{(a op b) ? x : y} could be flattened
>  out on the operand vector using 4 slots, but it would also
>  require additional processing to distinguish @code{c = a op b}
> -from @code{c = a op b ? x : y}.  Something similar occurs with
> -@code{ASSERT_EXPR}.   In time, these special case tree
> +from @code{c = a op b ? x : y}.  In time, these special case tree
>  expressions should be flattened into the operand vector.
>  @end itemize
>
> diff --git a/gcc/fold-const.cc b/gcc/fold-const.cc
> index b89cac91cae..114258fa182 100644
> --- a/gcc/fold-const.cc
> +++ b/gcc/fold-const.cc
> @@ -10751,7 +10751,6 @@ tree_expr_nonzero_warnv_p (tree t, bool *strict_overflow_p)
>      case COND_EXPR:
>      case CONSTRUCTOR:
>      case OBJ_TYPE_REF:
> -    case ASSERT_EXPR:
>      case ADDR_EXPR:
>      case WITH_SIZE_EXPR:
>      case SSA_NAME:
> @@ -12618,10 +12617,6 @@ fold_binary_loc (location_t loc, enum tree_code code, tree type,
>                                  : fold_convert_loc (loc, type, arg1);
>        return tem;
>
> -    case ASSERT_EXPR:
> -      /* An ASSERT_EXPR should never be passed to fold_binary.  */
> -      gcc_unreachable ();
> -
>      default:
>        return NULL_TREE;
>      } /* switch (code) */
> @@ -15117,7 +15112,6 @@ tree_expr_nonnegative_warnv_p (tree t, bool *strict_overflow_p, int depth)
>      case COND_EXPR:
>      case CONSTRUCTOR:
>      case OBJ_TYPE_REF:
> -    case ASSERT_EXPR:
>      case ADDR_EXPR:
>      case WITH_SIZE_EXPR:
>      case SSA_NAME:
> diff --git a/gcc/gimple-array-bounds.cc b/gcc/gimple-array-bounds.cc
> index 1eafd3fd3e1..eae49ab3910 100644
> --- a/gcc/gimple-array-bounds.cc
> +++ b/gcc/gimple-array-bounds.cc
> @@ -75,14 +75,7 @@ get_base_decl (tree ref)
>        if (gimple_assign_single_p (def))
>         {
>           base = gimple_assign_rhs1 (def);
> -         if (TREE_CODE (base) != ASSERT_EXPR)
> -           return base;
> -
> -         base = TREE_OPERAND (base, 0);
> -         if (TREE_CODE (base) != SSA_NAME)
> -           return base;
> -
> -         continue;
> +         return base;
>         }
>
>        if (!gimple_nop_p (def))
> diff --git a/gcc/gimple-pretty-print.cc b/gcc/gimple-pretty-print.cc
> index 7ec079f15c6..af704257633 100644
> --- a/gcc/gimple-pretty-print.cc
> +++ b/gcc/gimple-pretty-print.cc
> @@ -339,7 +339,6 @@ dump_unary_rhs (pretty_printer *buffer, const gassign *gs, int spc,
>    switch (rhs_code)
>      {
>      case VIEW_CONVERT_EXPR:
> -    case ASSERT_EXPR:
>        dump_generic_node (buffer, rhs, spc, flags, false);
>        break;
>
> diff --git a/gcc/gimple.cc b/gcc/gimple.cc
> index 6c23dd77609..dd054e16453 100644
> --- a/gcc/gimple.cc
> +++ b/gcc/gimple.cc
> @@ -2408,7 +2408,6 @@ get_gimple_rhs_num_ops (enum tree_code code)
>        || (SYM) == BIT_INSERT_EXPR) ? GIMPLE_TERNARY_RHS                            \
>     : ((SYM) == CONSTRUCTOR                                                 \
>        || (SYM) == OBJ_TYPE_REF                                             \
> -      || (SYM) == ASSERT_EXPR                                              \
>        || (SYM) == ADDR_EXPR                                                \
>        || (SYM) == WITH_SIZE_EXPR                                           \
>        || (SYM) == SSA_NAME) ? GIMPLE_SINGLE_RHS                                    \
> diff --git a/gcc/pointer-query.cc b/gcc/pointer-query.cc
> index 0f0100233c1..95565fd6005 100644
> --- a/gcc/pointer-query.cc
> +++ b/gcc/pointer-query.cc
> @@ -2144,12 +2144,6 @@ handle_ssa_name (tree ptr, bool addr, int ostype,
>
>    tree rhs = gimple_assign_rhs1 (stmt);
>
> -  if (code == ASSERT_EXPR)
> -    {
> -      rhs = TREE_OPERAND (rhs, 0);
> -      return compute_objsize_r (rhs, stmt, addr, ostype, pref, snlim, qry);
> -    }
> -
>    if (code == POINTER_PLUS_EXPR
>        && TREE_CODE (TREE_TYPE (rhs)) == POINTER_TYPE)
>      {
> diff --git a/gcc/tree-cfg.cc b/gcc/tree-cfg.cc
> index d982988048f..28175312afc 100644
> --- a/gcc/tree-cfg.cc
> +++ b/gcc/tree-cfg.cc
> @@ -4795,17 +4795,6 @@ verify_gimple_assign_single (gassign *stmt)
>         }
>        return res;
>
> -    case ASSERT_EXPR:
> -      /* FIXME.  */
> -      rhs1 = fold (ASSERT_EXPR_COND (rhs1));
> -      if (rhs1 == boolean_false_node)
> -       {
> -         error ("%qs with an always-false condition", code_name);
> -         debug_generic_stmt (rhs1);
> -         return true;
> -       }
> -      break;
> -
>      case WITH_SIZE_EXPR:
>        error ("%qs RHS in assignment statement",
>              get_tree_code_name (rhs_code));
> diff --git a/gcc/tree-pretty-print.cc b/gcc/tree-pretty-print.cc
> index e7a8c9481a6..cb2a88cbdc5 100644
> --- a/gcc/tree-pretty-print.cc
> +++ b/gcc/tree-pretty-print.cc
> @@ -3416,14 +3416,6 @@ dump_generic_node (pretty_printer *pp, tree node, int spc, dump_flags_t flags,
>        pp_greater (pp);
>        break;
>
> -    case ASSERT_EXPR:
> -      pp_string (pp, "ASSERT_EXPR <");
> -      dump_generic_node (pp, ASSERT_EXPR_VAR (node), spc, flags, false);
> -      pp_string (pp, ", ");
> -      dump_generic_node (pp, ASSERT_EXPR_COND (node), spc, flags, false);
> -      pp_greater (pp);
> -      break;
> -
>      case SCEV_KNOWN:
>        pp_string (pp, "scev_known");
>        break;
> diff --git a/gcc/tree-scalar-evolution.cc b/gcc/tree-scalar-evolution.cc
> index 7e2a3e98661..60060337804 100644
> --- a/gcc/tree-scalar-evolution.cc
> +++ b/gcc/tree-scalar-evolution.cc
> @@ -1122,13 +1122,11 @@ scev_dfs::follow_ssa_edge_expr (gimple *at_stmt, tree expr,
>       - a PLUS_EXPR,
>       - a POINTER_PLUS_EXPR,
>       - a MINUS_EXPR,
> -     - an ASSERT_EXPR,
>       - other cases are not yet handled.  */
>
>    /* For SSA_NAME look at the definition statement, handling
>       PHI nodes and otherwise expand appropriately for the expression
>       handling below.  */
> -tail_recurse:
>    if (TREE_CODE (expr) == SSA_NAME)
>      {
>        gimple *def = SSA_NAME_DEF_STMT (expr);
> @@ -1272,12 +1270,6 @@ tail_recurse:
>        return follow_ssa_edge_binary (at_stmt, type, rhs0, code, rhs1,
>                                      evolution_of_loop, limit);
>
> -    case ASSERT_EXPR:
> -      /* This assignment is of the form: "a_1 = ASSERT_EXPR <a_2, ...>"
> -        It must be handled as a copy assignment of the form a_1 = a_2.  */
> -      expr = ASSERT_EXPR_VAR (rhs0);
> -      goto tail_recurse;
> -
>      default:
>        return t_false;
>      }
> @@ -1640,13 +1632,6 @@ interpret_rhs_expr (class loop *loop, gimple *at_stmt,
>        if (code == SSA_NAME)
>         return chrec_convert (type, analyze_scalar_evolution (loop, rhs1),
>                               at_stmt);
> -
> -      if (code == ASSERT_EXPR)
> -       {
> -         rhs1 = ASSERT_EXPR_VAR (rhs1);
> -         return chrec_convert (type, analyze_scalar_evolution (loop, rhs1),
> -                               at_stmt);
> -       }
>      }
>
>    switch (code)
> diff --git a/gcc/tree-ssa-operands.cc b/gcc/tree-ssa-operands.cc
> index 9e85998b75e..4c90197860c 100644
> --- a/gcc/tree-ssa-operands.cc
> +++ b/gcc/tree-ssa-operands.cc
> @@ -897,7 +897,6 @@ operands_scanner::get_expr_operands (tree *expr_p, int flags)
>      case BIT_INSERT_EXPR:
>      case COMPOUND_EXPR:
>      case OBJ_TYPE_REF:
> -    case ASSERT_EXPR:
>      do_binary:
>        {
>         get_expr_operands (&TREE_OPERAND (expr, 0), flags);
> diff --git a/gcc/tree-ssa-propagate.cc b/gcc/tree-ssa-propagate.cc
> index 9dc4bfd85bf..6660d4f7cf1 100644
> --- a/gcc/tree-ssa-propagate.cc
> +++ b/gcc/tree-ssa-propagate.cc
> @@ -818,10 +818,7 @@ substitute_and_fold_dom_walker::before_dom_children (basic_block bb)
>               && sprime != lhs
>               && may_propagate_copy (lhs, sprime)
>               && !stmt_could_throw_p (cfun, stmt)
> -             && !gimple_has_side_effects (stmt)
> -             /* We have to leave ASSERT_EXPRs around for jump-threading.  */
> -             && (!is_gimple_assign (stmt)
> -                 || gimple_assign_rhs_code (stmt) != ASSERT_EXPR))
> +             && !gimple_has_side_effects (stmt))
>             {
>               if (dump_file && (dump_flags & TDF_DETAILS))
>                 {
> diff --git a/gcc/tree-ssa-threadedge.cc b/gcc/tree-ssa-threadedge.cc
> index 905a98c8c68..cc7020ddbb3 100644
> --- a/gcc/tree-ssa-threadedge.cc
> +++ b/gcc/tree-ssa-threadedge.cc
> @@ -497,9 +497,9 @@ jump_threader::simplify_control_stmt_condition_1
>      }
>
>    /* If the condition has the form (A & B) CMP 0 or (A | B) CMP 0 then
> -     recurse into the LHS to see if there is a dominating ASSERT_EXPR
> -     of A or of B that makes this condition always true or always false
> -     along the edge E.  */
> +     recurse into the LHS to see if there is a simplification that
> +     makes this condition always true or always false along the edge
> +     E.  */
>    if ((cond_code == EQ_EXPR || cond_code == NE_EXPR)
>        && TREE_CODE (op0) == SSA_NAME
>        && integer_zerop (op1))
> diff --git a/gcc/tree-vrp.cc b/gcc/tree-vrp.cc
> index df2f1eae203..d29941d0f2d 100644
> --- a/gcc/tree-vrp.cc
> +++ b/gcc/tree-vrp.cc
> @@ -782,10 +782,9 @@ overflow_comparison_p (tree_code code, tree name, tree val,
>     <bb 6>:
>     __builtin_unreachable ();
>     <bb 7>:
> -   x_5 = ASSERT_EXPR <x_3, ...>;
> -   If x_3 has no other immediate uses (checked by caller),
> -   var is the x_3 var from ASSERT_EXPR, we can clear low 5 bits
> -   from the non-zero bitmask.  */
> +
> +   If x_3 has no other immediate uses (checked by caller), var is the
> +   x_3 var, we can clear low 5 bits from the non-zero bitmask.  */
>
>  void
>  maybe_set_nonzero_bits (edge e, tree var)
> diff --git a/gcc/tree.def b/gcc/tree.def
> index 62650b6934b..69783bd44fc 100644
> --- a/gcc/tree.def
> +++ b/gcc/tree.def
> @@ -1025,7 +1025,10 @@ DEFTREECODE (POLYNOMIAL_CHREC, "polynomial_chrec", tcc_expression, 2)
>     Use the interface in tree-iterator.h to access this node.  */
>  DEFTREECODE (STATEMENT_LIST, "statement_list", tcc_exceptional, 0)
>
> -/* Predicate assertion.  Artificial expression generated by the optimizers
> +/* NOTE: This code is deprecated and should only be used internally by ipa* as
> +   temporary construct.
> +
> +   Predicate assertion.  Artificial expression generated by the optimizers
>     to keep track of predicate values.  This expression may only appear on
>     the RHS of assignments.
>
> diff --git a/gcc/tree.h b/gcc/tree.h
> index a863d2e50e5..4a19de1c94d 100644
> --- a/gcc/tree.h
> +++ b/gcc/tree.h
> @@ -1406,10 +1406,6 @@ class auto_suppress_location_wrappers
>  #define OBJ_TYPE_REF_OBJECT(NODE) TREE_OPERAND (OBJ_TYPE_REF_CHECK (NODE), 1)
>  #define OBJ_TYPE_REF_TOKEN(NODE)  TREE_OPERAND (OBJ_TYPE_REF_CHECK (NODE), 2)
>
> -/* ASSERT_EXPR accessors.  */
> -#define ASSERT_EXPR_VAR(NODE)  TREE_OPERAND (ASSERT_EXPR_CHECK (NODE), 0)
> -#define ASSERT_EXPR_COND(NODE) TREE_OPERAND (ASSERT_EXPR_CHECK (NODE), 1)
> -
>  /* CALL_EXPR accessors.  */
>  #define CALL_EXPR_FN(NODE) TREE_OPERAND (CALL_EXPR_CHECK (NODE), 1)
>  #define CALL_EXPR_STATIC_CHAIN(NODE) TREE_OPERAND (CALL_EXPR_CHECK (NODE), 2)
> diff --git a/gcc/vr-values.cc b/gcc/vr-values.cc
> index 798bfdc904a..0347c29b216 100644
> --- a/gcc/vr-values.cc
> +++ b/gcc/vr-values.cc
> @@ -944,49 +944,6 @@ simplify_using_ranges::vrp_visit_cond_stmt (gcond *stmt, edge *taken_edge_p)
>        fprintf (dump_file, "\n");
>      }
>
> -  /* Compute the value of the predicate COND by checking the known
> -     ranges of each of its operands.
> -
> -     Note that we cannot evaluate all the equivalent ranges here
> -     because those ranges may not yet be final and with the current
> -     propagation strategy, we cannot determine when the value ranges
> -     of the names in the equivalence set have changed.
> -
> -     For instance, given the following code fragment
> -
> -        i_5 = PHI <8, i_13>
> -       ...
> -       i_14 = ASSERT_EXPR <i_5, i_5 != 0>
> -       if (i_14 == 1)
> -         ...
> -
> -     Assume that on the first visit to i_14, i_5 has the temporary
> -     range [8, 8] because the second argument to the PHI function is
> -     not yet executable.  We derive the range ~[0, 0] for i_14 and the
> -     equivalence set { i_5 }.  So, when we visit 'if (i_14 == 1)' for
> -     the first time, since i_14 is equivalent to the range [8, 8], we
> -     determine that the predicate is always false.
> -
> -     On the next round of propagation, i_13 is determined to be
> -     VARYING, which causes i_5 to drop down to VARYING.  So, another
> -     visit to i_14 is scheduled.  In this second visit, we compute the
> -     exact same range and equivalence set for i_14, namely ~[0, 0] and
> -     { i_5 }.  But we did not have the previous range for i_5
> -     registered, so vrp_visit_assignment thinks that the range for
> -     i_14 has not changed.  Therefore, the predicate 'if (i_14 == 1)'
> -     is not visited again, which stops propagation from visiting
> -     statements in the THEN clause of that if().
> -
> -     To properly fix this we would need to keep the previous range
> -     value for the names in the equivalence set.  This way we would've
> -     discovered that from one visit to the other i_5 changed from
> -     range [8, 8] to VR_VARYING.
> -
> -     However, fixing this apparent limitation may not be worth the
> -     additional checking.  Testing on several code bases (GCC, DLV,
> -     MICO, TRAMP3D and SPEC2000) showed that doing this results in
> -     4 more predicates folded in SPEC.  */
> -
>    bool sop;
>    val = vrp_evaluate_conditional_warnv_with_ops (stmt,
>                                                  gimple_cond_code (stmt),
> --
> 2.38.1
>

      parent reply	other threads:[~2022-11-23 20:27 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-22 13:57 Aldy Hernandez
2022-11-22 13:58 ` [PATCH] Remove follow_assert_exprs from overflow_comparison Aldy Hernandez
2022-11-23 20:28   ` Richard Biener
2022-11-22 13:58 ` [PATCH] Remove use_equiv_p in vr-values.cc Aldy Hernandez
2022-11-23 20:29   ` Richard Biener
2022-11-23 20:27 ` Richard Biener [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAFiYyc0j7=BMfobtcnP==eosO7qL+YQ-VEToH-=9vczR-27VzA@mail.gmail.com' \
    --to=richard.guenther@gmail.com \
    --cc=aldyh@redhat.com \
    --cc=amacleod@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).