[PATCH] Simplify conditions in EVRP, handle taken edge

Christophe Lyon christophe.lyon@linaro.org
Wed Oct 19 08:01:00 GMT 2016


On 18 October 2016 at 09:34, Richard Biener <rguenther@suse.de> wrote:
> On Mon, 17 Oct 2016, Richard Biener wrote:
>
>>
>> This refactors propagation vs. substitution and handles condition
>> simplification properly as well as passing a known taken edge down
>> to the DOM walker (avoiding useless work and properly handling PHIs).
>>
>> If we do all the work it's stupid to not fold away dead code...
>>
>> Bootstrap and regtest pending on x86_64-unknown-linux-gnu.
>
> The following is what I applied, also fixing a spelling mistake noticed
> by Bernhard.
>
Hi Richard,

This patch is causing regressions on aarch64. These tests now fail:
  gcc.target/aarch64/test_frame_12.c scan-assembler-times ldp\tx29,
x30, \\[sp, [0-9]+\\] 1
  gcc.target/aarch64/test_frame_12.c scan-assembler-times sub\tsp, sp, #[0-9]+ 1
  gcc.target/aarch64/test_frame_15.c scan-assembler-times stp\tx29,
x30, \\[sp, [0-9]+\\] 1
  gcc.target/aarch64/test_frame_15.c scan-assembler-times sub\tsp, sp, #[0-9]+ 1
  gcc.target/aarch64/test_frame_8.c scan-assembler-times ldr\tx30,
\\[sp, [0-9]+\\] 1
  gcc.target/aarch64/test_frame_8.c scan-assembler-times str\tx30,
\\[sp, [0-9]+\\] 1

Christophe

> Richard.
>
> 2016-10-18  Richard Biener  <rguenther@suse.de>
>
>         * tree-vrp.c (evrp_dom_walker::before_dom_children): Handle
>         not visited but non-executable predecessors.  Return taken edge.
>         Simplify conditions and refactor propagation vs. folding step.
>
>         * gcc.dg/tree-ssa/pr20318.c: Disable EVRP.
>         * gcc.dg/tree-ssa/pr21001.c: Likewise.
>         * gcc.dg/tree-ssa/pr21090.c: Likewise.
>         * gcc.dg/tree-ssa/pr21294.c: Likewise.
>         * gcc.dg/tree-ssa/pr21563.c: Likewise.
>         * gcc.dg/tree-ssa/pr23744.c: Likewise.
>         * gcc.dg/tree-ssa/pr25382.c: Likewise.
>         * gcc.dg/tree-ssa/pr68431.c: Likewise.
>         * gcc.dg/tree-ssa/vrp03.c: Likewise.
>         * gcc.dg/tree-ssa/vrp06.c: Likewise.
>         * gcc.dg/tree-ssa/vrp07.c: Likewise.
>         * gcc.dg/tree-ssa/vrp09.c: Likewise.
>         * gcc.dg/tree-ssa/vrp19.c: Likewise.
>         * gcc.dg/tree-ssa/vrp20.c: Likewise.
>         * gcc.dg/tree-ssa/vrp92.c: Likewise.
>         * gcc.dg/pr68217.c: Likewise.
>         * gcc.dg/predict-9.c: Likewise.
>         * gcc.dg/tree-prof/val-prof-5.c: Adjust.
>         * gcc.dg/predict-1.c: Likewise.
>
>
>
> Index: gcc/tree-vrp.c
> ===================================================================
> --- gcc/tree-vrp.c      (revision 241242)
> +++ gcc/tree-vrp.c      (working copy)
> @@ -10741,12 +10741,13 @@ evrp_dom_walker::before_dom_children (ba
>    gimple_stmt_iterator gsi;
>    edge e;
>    edge_iterator ei;
> -  bool has_unvisived_preds = false;
> +  bool has_unvisited_preds = false;
>
>    FOR_EACH_EDGE (e, ei, bb->preds)
> -    if (!(e->src->flags & BB_VISITED))
> +    if (e->flags & EDGE_EXECUTABLE
> +       && !(e->src->flags & BB_VISITED))
>        {
> -       has_unvisived_preds = true;
> +       has_unvisited_preds = true;
>         break;
>        }
>
> @@ -10756,7 +10757,7 @@ evrp_dom_walker::before_dom_children (ba
>        gphi *phi = gpi.phi ();
>        tree lhs = PHI_RESULT (phi);
>        value_range vr_result = VR_INITIALIZER;
> -      if (!has_unvisived_preds
> +      if (!has_unvisited_preds
>           && stmt_interesting_for_vrp (phi))
>         extract_range_from_phi_node (phi, &vr_result);
>        else
> @@ -10764,81 +10765,90 @@ evrp_dom_walker::before_dom_children (ba
>        update_value_range (lhs, &vr_result);
>      }
>
> +  edge taken_edge = NULL;
> +
>    /* Visit all other stmts and discover any new VRs possible.  */
>    for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
>      {
>        gimple *stmt = gsi_stmt (gsi);
> -      edge taken_edge;
>        tree output = NULL_TREE;
>        gimple *old_stmt = stmt;
>        bool was_noreturn = (is_gimple_call (stmt)
>                            && gimple_call_noreturn_p (stmt));
>
> -      /* TODO, if found taken_edge, we should visit (return it) and travel
> -        again to improve VR as done in DOM/SCCVN optimizations.  It should
> -        be done carefully as stmts might prematurely leave a BB like
> -        in EH.  */
> -      if (stmt_interesting_for_vrp (stmt))
> +      if (gcond *cond = dyn_cast <gcond *> (stmt))
> +       {
> +         vrp_visit_cond_stmt (cond, &taken_edge);
> +         if (taken_edge)
> +           {
> +             if (taken_edge->flags & EDGE_TRUE_VALUE)
> +               gimple_cond_make_true (cond);
> +             else if (taken_edge->flags & EDGE_FALSE_VALUE)
> +               gimple_cond_make_false (cond);
> +             else
> +               gcc_unreachable ();
> +           }
> +       }
> +      else if (stmt_interesting_for_vrp (stmt))
>         {
> +         edge taken_edge;
>           value_range vr = VR_INITIALIZER;
>           extract_range_from_stmt (stmt, &taken_edge, &output, &vr);
>           if (output
>               && (vr.type == VR_RANGE || vr.type == VR_ANTI_RANGE))
> -           update_value_range (output, &vr);
> -         else
> -           set_defs_to_varying (stmt);
> -
> -         /* Try folding stmts with the VR discovered.  */
> -         bool did_replace
> -           = replace_uses_in (stmt,
> -                              op_with_constant_singleton_value_range);
> -         if (fold_stmt (&gsi, follow_single_use_edges)
> -             || did_replace)
> -           update_stmt (gsi_stmt (gsi));
> -
> -         if (did_replace)
>             {
> -             /* If we cleaned up EH information from the statement,
> -                remove EH edges.  */
> -             if (maybe_clean_or_replace_eh_stmt (old_stmt, stmt))
> -               bitmap_set_bit (need_eh_cleanup, bb->index);
> -
> -             /* If we turned a not noreturn call into a noreturn one
> -                schedule it for fixup.  */
> -             if (!was_noreturn
> -                 && is_gimple_call (stmt)
> -                 && gimple_call_noreturn_p (stmt))
> -               stmts_to_fixup.safe_push (stmt);
> +             update_value_range (output, &vr);
>
> -             if (gimple_assign_single_p (stmt))
> +             /* Set the SSA with the value range.  */
> +             if (INTEGRAL_TYPE_P (TREE_TYPE (output)))
>                 {
> -                 tree rhs = gimple_assign_rhs1 (stmt);
> -                 if (TREE_CODE (rhs) == ADDR_EXPR)
> -                   recompute_tree_invariant_for_addr_expr (rhs);
> +                 value_range *vr = get_value_range (output);
> +
> +                 if ((vr->type == VR_RANGE
> +                      || vr->type == VR_ANTI_RANGE)
> +                     && (TREE_CODE (vr->min) == INTEGER_CST)
> +                     && (TREE_CODE (vr->max) == INTEGER_CST))
> +                   set_range_info (output, vr->type, vr->min, vr->max);
>                 }
>             }
> +         else
> +           set_defs_to_varying (stmt);
> +       }
> +      else
> +       set_defs_to_varying (stmt);
>
> -         def_operand_p def_p = SINGLE_SSA_DEF_OPERAND (stmt, SSA_OP_DEF);
> -         /* Set the SSA with the value range.  */
> -         if (def_p
> -             && TREE_CODE (DEF_FROM_PTR (def_p)) == SSA_NAME
> -             && INTEGRAL_TYPE_P (TREE_TYPE (DEF_FROM_PTR (def_p))))
> -           {
> -             tree def = DEF_FROM_PTR (def_p);
> -             value_range *vr = get_value_range (def);
> +      /* Try folding stmts with the VR discovered.  */
> +      bool did_replace
> +       = replace_uses_in (stmt, op_with_constant_singleton_value_range);
> +      if (fold_stmt (&gsi, follow_single_use_edges)
> +         || did_replace)
> +       update_stmt (gsi_stmt (gsi));
>
> -             if ((vr->type == VR_RANGE
> -                  || vr->type == VR_ANTI_RANGE)
> -                 && (TREE_CODE (vr->min) == INTEGER_CST)
> -                 && (TREE_CODE (vr->max) == INTEGER_CST))
> -               set_range_info (def, vr->type, vr->min, vr->max);
> +      if (did_replace)
> +       {
> +         /* If we cleaned up EH information from the statement,
> +            remove EH edges.  */
> +         if (maybe_clean_or_replace_eh_stmt (old_stmt, stmt))
> +           bitmap_set_bit (need_eh_cleanup, bb->index);
> +
> +         /* If we turned a not noreturn call into a noreturn one
> +            schedule it for fixup.  */
> +         if (!was_noreturn
> +             && is_gimple_call (stmt)
> +             && gimple_call_noreturn_p (stmt))
> +           stmts_to_fixup.safe_push (stmt);
> +
> +         if (gimple_assign_single_p (stmt))
> +           {
> +             tree rhs = gimple_assign_rhs1 (stmt);
> +             if (TREE_CODE (rhs) == ADDR_EXPR)
> +               recompute_tree_invariant_for_addr_expr (rhs);
>             }
>         }
> -      else
> -       set_defs_to_varying (stmt);
>      }
>    bb->flags |= BB_VISITED;
> -  return NULL;
> +
> +  return taken_edge;
>  }
>
>  /* Restore/pop VRs valid only for BB when we leave BB.  */
> Index: gcc/testsuite/gcc.dg/pr68217.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/pr68217.c      (revision 241241)
> +++ gcc/testsuite/gcc.dg/pr68217.c      (working copy)
> @@ -1,6 +1,5 @@
> -
>  /* { dg-do compile } */
> -/* { dg-options "-O2 -fdump-tree-vrp1" } */
> +/* { dg-options "-O2 -fdisable-tree-evrp -fdump-tree-vrp1" } */
>
>  int foo (void)
>  {
> Index: gcc/testsuite/gcc.dg/predict-1.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/predict-1.c    (revision 241241)
> +++ gcc/testsuite/gcc.dg/predict-1.c    (working copy)
> @@ -23,4 +23,4 @@ void foo (int bound)
>      }
>  }
>
> -/* { dg-final { scan-tree-dump-times "guess loop iv compare heuristics of edge\[^:\]*: 2.0%" 5 "profile_estimate"} } */
> +/* { dg-final { scan-tree-dump-times "guess loop iv compare heuristics of edge\[^:\]*: 2.0%" 4 "profile_estimate"} } */
> Index: gcc/testsuite/gcc.dg/predict-9.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/predict-9.c    (revision 241241)
> +++ gcc/testsuite/gcc.dg/predict-9.c    (working copy)
> @@ -1,5 +1,5 @@
>  /* { dg-do compile } */
> -/* { dg-options "-O2 -fdump-tree-profile_estimate" } */
> +/* { dg-options "-O2 -fdisable-tree-evrp -fdump-tree-profile_estimate" } */
>
>  extern int global;
>  extern int global2;
> Index: gcc/testsuite/gcc.dg/tree-prof/val-prof-5.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/tree-prof/val-prof-5.c (revision 241241)
> +++ gcc/testsuite/gcc.dg/tree-prof/val-prof-5.c (working copy)
> @@ -6,7 +6,7 @@ main()
>  {
>         int i;
>         for (i = 0; i < 1000; i++)
> -               if (a[i])
> +               if (a[i] != 1)
>                         a[i]/=b;
>                 else
>                         a[i]/=b;
> Index: gcc/testsuite/gcc.dg/tree-ssa/pr20318.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/tree-ssa/pr20318.c     (revision 241241)
> +++ gcc/testsuite/gcc.dg/tree-ssa/pr20318.c     (working copy)
> @@ -1,5 +1,5 @@
>  /* { dg-do compile { target { ! keeps_null_pointer_checks } } } */
> -/* { dg-options "-O2 -fdump-tree-original -fdump-tree-vrp1 -fdelete-null-pointer-checks" } */
> +/* { dg-options "-O2 -fdump-tree-original -fdump-tree-vrp1 -fdelete-null-pointer-checks -fdisable-tree-evrp" } */
>
>  extern int* f(int) __attribute__((returns_nonnull));
>  extern void eliminate ();
> Index: gcc/testsuite/gcc.dg/tree-ssa/pr21001.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/tree-ssa/pr21001.c     (revision 241241)
> +++ gcc/testsuite/gcc.dg/tree-ssa/pr21001.c     (working copy)
> @@ -5,7 +5,7 @@
>     range information out of the conditional.  */
>
>  /* { dg-do compile } */
> -/* { dg-options "-O2 -fno-tree-dominator-opts -fno-tree-fre -fdump-tree-vrp1-details" } */
> +/* { dg-options "-O2 -fno-tree-dominator-opts -fno-tree-fre -fdisable-tree-evrp -fdump-tree-vrp1-details" } */
>
>  int
>  foo (int a)
> Index: gcc/testsuite/gcc.dg/tree-ssa/pr21090.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/tree-ssa/pr21090.c     (revision 241241)
> +++ gcc/testsuite/gcc.dg/tree-ssa/pr21090.c     (working copy)
> @@ -1,5 +1,5 @@
>  /* { dg-do compile } */
> -/* { dg-options "-O2 -fdump-tree-vrp1 -fdelete-null-pointer-checks" } */
> +/* { dg-options "-O2 -fdisable-tree-evrp -fdump-tree-vrp1 -fdelete-null-pointer-checks" } */
>
>  int g, h;
>
> Index: gcc/testsuite/gcc.dg/tree-ssa/pr21294.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/tree-ssa/pr21294.c     (revision 241241)
> +++ gcc/testsuite/gcc.dg/tree-ssa/pr21294.c     (working copy)
> @@ -4,7 +4,7 @@
>     allows us to eliminate the second "if" statement.  */
>
>  /* { dg-do compile } */
> -/* { dg-options "-O2 -fno-tree-dominator-opts -fdump-tree-vrp1-details" } */
> +/* { dg-options "-O2 -fno-tree-dominator-opts -fdisable-tree-evrp -fdump-tree-vrp1-details" } */
>
>  struct f {
>    int i;
> Index: gcc/testsuite/gcc.dg/tree-ssa/pr21563.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/tree-ssa/pr21563.c     (revision 241241)
> +++ gcc/testsuite/gcc.dg/tree-ssa/pr21563.c     (working copy)
> @@ -2,7 +2,7 @@
>     Make sure VRP folds the second "if" statement.  */
>
>  /* { dg-do compile } */
> -/* { dg-options "-O2 -fno-tree-dominator-opts -fdump-tree-vrp1-details" } */
> +/* { dg-options "-O2 -fno-tree-dominator-opts -fdisable-tree-evrp -fdump-tree-vrp1-details" } */
>
>  int
>  foo (int a)
> Index: gcc/testsuite/gcc.dg/tree-ssa/pr23744.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/tree-ssa/pr23744.c     (revision 241241)
> +++ gcc/testsuite/gcc.dg/tree-ssa/pr23744.c     (working copy)
> @@ -1,5 +1,5 @@
>  /* { dg-do compile } */
> -/* { dg-options "-O2 -fno-tree-ccp -fdump-tree-vrp1" } */
> +/* { dg-options "-O2 -fno-tree-ccp -fdisable-tree-evrp -fdump-tree-vrp1" } */
>
>  void h (void);
>
> Index: gcc/testsuite/gcc.dg/tree-ssa/pr25382.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/tree-ssa/pr25382.c     (revision 241241)
> +++ gcc/testsuite/gcc.dg/tree-ssa/pr25382.c     (working copy)
> @@ -3,7 +3,7 @@
>     Check that VRP now gets ranges from BIT_AND_EXPRs.  */
>
>  /* { dg-do compile } */
> -/* { dg-options "-O2 -fno-tree-ccp -fdump-tree-vrp1" } */
> +/* { dg-options "-O2 -fno-tree-ccp -fdisable-tree-evrp -fdump-tree-vrp1" } */
>
>  int
>  foo (int a)
> Index: gcc/testsuite/gcc.dg/tree-ssa/pr68431.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/tree-ssa/pr68431.c     (revision 241241)
> +++ gcc/testsuite/gcc.dg/tree-ssa/pr68431.c     (working copy)
> @@ -1,5 +1,5 @@
>  /* PR tree-optimization/68431 */
> -/* { dg-options "-O2 -fdump-tree-vrp1-details" } */
> +/* { dg-options "-O2 -fdisable-tree-evrp -fdump-tree-vrp1-details" } */
>
>  unsigned int x = 1;
>  int
> Index: gcc/testsuite/gcc.dg/tree-ssa/vrp03.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/tree-ssa/vrp03.c       (revision 241241)
> +++ gcc/testsuite/gcc.dg/tree-ssa/vrp03.c       (working copy)
> @@ -1,5 +1,5 @@
>  /* { dg-do compile } */
> -/* { dg-options "-O2 -fdump-tree-vrp1" } */
> +/* { dg-options "-O2 -fdisable-tree-evrp -fdump-tree-vrp1" } */
>
>  struct A
>  {
> Index: gcc/testsuite/gcc.dg/tree-ssa/vrp06.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/tree-ssa/vrp06.c       (revision 241241)
> +++ gcc/testsuite/gcc.dg/tree-ssa/vrp06.c       (working copy)
> @@ -1,5 +1,5 @@
>  /* { dg-do compile } */
> -/* { dg-options "-O2 -fdump-tree-vrp1" } */
> +/* { dg-options "-O2 -fdisable-tree-evrp -fdump-tree-vrp1" } */
>
>  int baz (void);
>
> Index: gcc/testsuite/gcc.dg/tree-ssa/vrp07.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/tree-ssa/vrp07.c       (revision 241241)
> +++ gcc/testsuite/gcc.dg/tree-ssa/vrp07.c       (working copy)
> @@ -1,5 +1,5 @@
>  /* { dg-do compile } */
> -/* { dg-options "-O2 -fno-tree-fre -fdump-tree-vrp1-details -fdelete-null-pointer-checks" } */
> +/* { dg-options "-O2 -fno-tree-fre -fdisable-tree-evrp -fdump-tree-vrp1-details -fdelete-null-pointer-checks" } */
>
>  int
>  foo (int i, int *p)
> Index: gcc/testsuite/gcc.dg/tree-ssa/vrp09.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/tree-ssa/vrp09.c       (revision 241241)
> +++ gcc/testsuite/gcc.dg/tree-ssa/vrp09.c       (working copy)
> @@ -1,5 +1,5 @@
>  /* { dg-do compile } */
> -/* { dg-options "-O2 -fno-tree-fre -fdump-tree-vrp1 -std=gnu89" } */
> +/* { dg-options "-O2 -fno-tree-fre -fdisable-tree-evrp -fdump-tree-vrp1 -std=gnu89" } */
>
>  foo (int *p)
>  {
> Index: gcc/testsuite/gcc.dg/tree-ssa/vrp19.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/tree-ssa/vrp19.c       (revision 241241)
> +++ gcc/testsuite/gcc.dg/tree-ssa/vrp19.c       (working copy)
> @@ -1,5 +1,5 @@
>  /* { dg-do compile } */
> -/* { dg-options "-fwrapv -O1 -ftree-vrp -fdump-tree-vrp1" } */
> +/* { dg-options "-fwrapv -O1 -ftree-vrp -fdisable-tree-evrp -fdump-tree-vrp1" } */
>
>  #include <limits.h>
>  extern void abort ();
> Index: gcc/testsuite/gcc.dg/tree-ssa/vrp20.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/tree-ssa/vrp20.c       (revision 241241)
> +++ gcc/testsuite/gcc.dg/tree-ssa/vrp20.c       (working copy)
> @@ -1,5 +1,5 @@
>  /* { dg-do compile } */
> -/* { dg-options "-fwrapv -O1 -fno-tree-fre -ftree-vrp -fdump-tree-vrp1" } */
> +/* { dg-options "-fwrapv -O1 -fno-tree-fre -fdisable-tree-evrp -ftree-vrp -fdump-tree-vrp1" } */
>
>  extern void abort ();
>  extern void exit (int);
> Index: gcc/testsuite/gcc.dg/tree-ssa/vrp92.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/tree-ssa/vrp92.c       (revision 241241)
> +++ gcc/testsuite/gcc.dg/tree-ssa/vrp92.c       (working copy)
> @@ -1,5 +1,5 @@
>  /* { dg-do compile } */
> -/* { dg-options "-O2 -fdump-tree-vrp1-details -fdisable-tree-ethread" } */
> +/* { dg-options "-O2 -fdisable-tree-evrp -fdump-tree-vrp1-details -fdisable-tree-ethread" } */
>
>  void bar (void);
>  int foo (int i, int j)



More information about the Gcc-patches mailing list