Message ID | 20150924154452.GA6184@redhat.com |
---|---|
State | New |
Headers | show |
On Thu, 24 Sep 2015, Marek Polacek wrote: > As Richi said in <https://gcc.gnu.org/ml/gcc-patches/2015-01/msg02279.html>, > using recorded SSA name range infos in VRP is likely to expose errors in the > ranges. This PR is such a case. As discussed in the PR, after tail merging > via PRE the range infos cannot be relied upon anymore, so we need to clear > them. > > Since tree-ssa-ifcombine.c already had code to clean up the flow data in a BB, > I've factored it out to a common function. > > Bootstrapped/regtested on x86_64-linux, ok for trunk and 5? I believe for tail-merge you also need to clear range info on PHI defs in the BB. For ifcombine this wasn't necessary (no PHI nodes in the relevant CFG), but it's ok to extend the new reset_flow_sensitive_info_in_bb function to also reset PHI defs. Ok with that change. Thanks, Richard. > 2015-09-24 Marek Polacek <polacek@redhat.com> > > PR tree-optimization/67690 > * tree-ssa-ifcombine.c (pass_tree_ifcombine::execute): Call > reset_flow_sensitive_info_in_bb. > * tree-ssa-tail-merge.c (replace_block_by): Likewise. > * tree-ssanames.c: Include "gimple-iterator.h". > (reset_flow_sensitive_info_in_bb): New function. > * tree-ssanames.h (reset_flow_sensitive_info_in_bb): Declare. > > * gcc.dg/torture/pr67690.c: New test. > > diff --git gcc/testsuite/gcc.dg/torture/pr67690.c gcc/testsuite/gcc.dg/torture/pr67690.c > index e69de29..491de51 100644 > --- gcc/testsuite/gcc.dg/torture/pr67690.c > +++ gcc/testsuite/gcc.dg/torture/pr67690.c > @@ -0,0 +1,32 @@ > +/* { dg-do run } */ > + > +const int c1 = 1; > +const int c2 = 2; > + > +int > +check (int i) > +{ > + int j; > + if (i >= 0) > + j = c2 - i; > + else > + j = c2 - i; > + return c2 - c1 + 1 > j; > +} > + > +int invoke (int *pi) __attribute__ ((noinline,noclone)); > +int > +invoke (int *pi) > +{ > + return check (*pi); > +} > + > +int > +main () > +{ > + int i = c1; > + int ret = invoke (&i); > + if (!ret) > + __builtin_abort (); > + return 0; > +} > diff --git gcc/tree-ssa-ifcombine.c gcc/tree-ssa-ifcombine.c > index 9f04174..66be430 100644 > --- gcc/tree-ssa-ifcombine.c > +++ gcc/tree-ssa-ifcombine.c > @@ -769,16 +769,7 @@ pass_tree_ifcombine::execute (function *fun) > { > /* Clear range info from all stmts in BB which is now executed > conditional on a always true/false condition. */ > - for (gimple_stmt_iterator gsi = gsi_start_bb (bb); > - !gsi_end_p (gsi); gsi_next (&gsi)) > - { > - gimple *stmt = gsi_stmt (gsi); > - ssa_op_iter i; > - tree op; > - FOR_EACH_SSA_TREE_OPERAND (op, stmt, i, SSA_OP_DEF) > - reset_flow_sensitive_info (op); > - } > - > + reset_flow_sensitive_info_in_bb (bb); > cfg_changed |= true; > } > } > diff --git gcc/tree-ssa-tail-merge.c gcc/tree-ssa-tail-merge.c > index 0ce59e8..487961e 100644 > --- gcc/tree-ssa-tail-merge.c > +++ gcc/tree-ssa-tail-merge.c > @@ -1534,6 +1534,10 @@ replace_block_by (basic_block bb1, basic_block bb2) > e2->probability = GCOV_COMPUTE_SCALE (e2->count, out_sum); > } > > + /* Clear range info from all stmts in BB2 -- this transformation > + could make them out of date. */ > + reset_flow_sensitive_info_in_bb (bb2); > + > /* Do updates that use bb1, before deleting bb1. */ > release_last_vdef (bb1); > same_succ_flush_bb (bb1); > diff --git gcc/tree-ssanames.c gcc/tree-ssanames.c > index 4199290..5393865 100644 > --- gcc/tree-ssanames.c > +++ gcc/tree-ssanames.c > @@ -23,6 +23,7 @@ along with GCC; see the file COPYING3. If not see > #include "backend.h" > #include "tree.h" > #include "gimple.h" > +#include "gimple-iterator.h" > #include "hard-reg-set.h" > #include "ssa.h" > #include "alias.h" > @@ -544,6 +545,21 @@ reset_flow_sensitive_info (tree name) > SSA_NAME_RANGE_INFO (name) = NULL; > } > > +/* Clear all flow sensitive data from all statements in BB. */ > + > +void > +reset_flow_sensitive_info_in_bb (basic_block bb) > +{ > + for (gimple_stmt_iterator gsi = gsi_start_bb (bb); !gsi_end_p (gsi); > + gsi_next (&gsi)) > + { > + gimple *stmt = gsi_stmt (gsi); > + ssa_op_iter i; > + tree op; > + FOR_EACH_SSA_TREE_OPERAND (op, stmt, i, SSA_OP_DEF) > + reset_flow_sensitive_info (op); > + } > +} > > /* Release all the SSA_NAMEs created by STMT. */ > > diff --git gcc/tree-ssanames.h gcc/tree-ssanames.h > index 22ff609..5688ca5 100644 > --- gcc/tree-ssanames.h > +++ gcc/tree-ssanames.h > @@ -95,6 +95,7 @@ extern tree duplicate_ssa_name_fn (struct function *, tree, gimple *); > extern void duplicate_ssa_name_range_info (tree, enum value_range_type, > struct range_info_def *); > extern void reset_flow_sensitive_info (tree); > +extern void reset_flow_sensitive_info_in_bb (basic_block); > extern void release_defs (gimple *); > extern void replace_ssa_name_symbol (tree, tree); > > > Marek > >
diff --git gcc/testsuite/gcc.dg/torture/pr67690.c gcc/testsuite/gcc.dg/torture/pr67690.c index e69de29..491de51 100644 --- gcc/testsuite/gcc.dg/torture/pr67690.c +++ gcc/testsuite/gcc.dg/torture/pr67690.c @@ -0,0 +1,32 @@ +/* { dg-do run } */ + +const int c1 = 1; +const int c2 = 2; + +int +check (int i) +{ + int j; + if (i >= 0) + j = c2 - i; + else + j = c2 - i; + return c2 - c1 + 1 > j; +} + +int invoke (int *pi) __attribute__ ((noinline,noclone)); +int +invoke (int *pi) +{ + return check (*pi); +} + +int +main () +{ + int i = c1; + int ret = invoke (&i); + if (!ret) + __builtin_abort (); + return 0; +} diff --git gcc/tree-ssa-ifcombine.c gcc/tree-ssa-ifcombine.c index 9f04174..66be430 100644 --- gcc/tree-ssa-ifcombine.c +++ gcc/tree-ssa-ifcombine.c @@ -769,16 +769,7 @@ pass_tree_ifcombine::execute (function *fun) { /* Clear range info from all stmts in BB which is now executed conditional on a always true/false condition. */ - for (gimple_stmt_iterator gsi = gsi_start_bb (bb); - !gsi_end_p (gsi); gsi_next (&gsi)) - { - gimple *stmt = gsi_stmt (gsi); - ssa_op_iter i; - tree op; - FOR_EACH_SSA_TREE_OPERAND (op, stmt, i, SSA_OP_DEF) - reset_flow_sensitive_info (op); - } - + reset_flow_sensitive_info_in_bb (bb); cfg_changed |= true; } } diff --git gcc/tree-ssa-tail-merge.c gcc/tree-ssa-tail-merge.c index 0ce59e8..487961e 100644 --- gcc/tree-ssa-tail-merge.c +++ gcc/tree-ssa-tail-merge.c @@ -1534,6 +1534,10 @@ replace_block_by (basic_block bb1, basic_block bb2) e2->probability = GCOV_COMPUTE_SCALE (e2->count, out_sum); } + /* Clear range info from all stmts in BB2 -- this transformation + could make them out of date. */ + reset_flow_sensitive_info_in_bb (bb2); + /* Do updates that use bb1, before deleting bb1. */ release_last_vdef (bb1); same_succ_flush_bb (bb1); diff --git gcc/tree-ssanames.c gcc/tree-ssanames.c index 4199290..5393865 100644 --- gcc/tree-ssanames.c +++ gcc/tree-ssanames.c @@ -23,6 +23,7 @@ along with GCC; see the file COPYING3. If not see #include "backend.h" #include "tree.h" #include "gimple.h" +#include "gimple-iterator.h" #include "hard-reg-set.h" #include "ssa.h" #include "alias.h" @@ -544,6 +545,21 @@ reset_flow_sensitive_info (tree name) SSA_NAME_RANGE_INFO (name) = NULL; } +/* Clear all flow sensitive data from all statements in BB. */ + +void +reset_flow_sensitive_info_in_bb (basic_block bb) +{ + for (gimple_stmt_iterator gsi = gsi_start_bb (bb); !gsi_end_p (gsi); + gsi_next (&gsi)) + { + gimple *stmt = gsi_stmt (gsi); + ssa_op_iter i; + tree op; + FOR_EACH_SSA_TREE_OPERAND (op, stmt, i, SSA_OP_DEF) + reset_flow_sensitive_info (op); + } +} /* Release all the SSA_NAMEs created by STMT. */ diff --git gcc/tree-ssanames.h gcc/tree-ssanames.h index 22ff609..5688ca5 100644 --- gcc/tree-ssanames.h +++ gcc/tree-ssanames.h @@ -95,6 +95,7 @@ extern tree duplicate_ssa_name_fn (struct function *, tree, gimple *); extern void duplicate_ssa_name_range_info (tree, enum value_range_type, struct range_info_def *); extern void reset_flow_sensitive_info (tree); +extern void reset_flow_sensitive_info_in_bb (basic_block); extern void release_defs (gimple *); extern void replace_ssa_name_symbol (tree, tree);