| Message ID | 20150929142116.GF6184@redhat.com |
|---|---|
| State | New |
| Headers | show |
On September 29, 2015 4:21:16 PM GMT+02:00, Marek Polacek <polacek@redhat.com> wrote: >On Fri, Sep 25, 2015 at 06:22:44PM +0200, Richard Biener wrote: >> On September 25, 2015 3:49:34 PM GMT+02:00, Marek Polacek ><polacek@redhat.com> wrote: >> >On Fri, Sep 25, 2015 at 09:29:30AM +0200, Richard Biener wrote: >> >> 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. >> > >> >All right. >> > >> >> Ok with that change. >> > >> >Since I'm not completely sure if I did the right thing here, could >you >> >please have another look at the new function? >> >> Doesn't work that way. You need to iterate over the PHI sequence >separately via gsi_start_phis(bb), etc. > >Oops, sorry. So like this? > >Bootstrapped/regtested on x86_64-linux, ok for trunk (and a similar >patch for 5)? Yes, thanks Richard. >2015-09-29 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..7235dc3 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,29 @@ reset_flow_sensitive_info (tree name) > SSA_NAME_RANGE_INFO (name) = NULL; > } > >+/* Clear all flow sensitive data from all statements and PHI >definitions >+ 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); >+ } >+ >+ for (gphi_iterator gsi = gsi_start_phis (bb); !gsi_end_p (gsi); >+ gsi_next (&gsi)) >+ { >+ tree phi_def = gimple_phi_result (gsi.phi ()); >+ reset_flow_sensitive_info (phi_def); >+ } >+} > > /* 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..7235dc3 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,29 @@ reset_flow_sensitive_info (tree name) SSA_NAME_RANGE_INFO (name) = NULL; } +/* Clear all flow sensitive data from all statements and PHI definitions + 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); + } + + for (gphi_iterator gsi = gsi_start_phis (bb); !gsi_end_p (gsi); + gsi_next (&gsi)) + { + tree phi_def = gimple_phi_result (gsi.phi ()); + reset_flow_sensitive_info (phi_def); + } +} /* 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);