On Fri, Sep 25, 2015 at 03:49:34PM +0200, Marek Polacek 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?
And this is gcc-5 variant of the patch, bootstrapped/regtested on x86_64-linux.
2015-09-25 Marek Polacek <pola...@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: Include "stringpool.h" and "tree-ssanames.h".
(replace_block_by): Call reset_flow_sensitive_info_in_bb.
* 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.
--- 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;
+}
--- gcc/tree-ssa-ifcombine.c
+++ gcc/tree-ssa-ifcombine.c
@@ -806,16 +806,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;
}
}
--- gcc/tree-ssa-tail-merge.c
+++ gcc/tree-ssa-tail-merge.c
@@ -235,6 +235,8 @@ along with GCC; see the file COPYING3. If not see
#include "cfgloop.h"
#include "tree-pass.h"
#include "trans-mem.h"
+#include "stringpool.h"
+#include "tree-ssanames.h"
/* Describes a group of bbs with the same successors. The successor bbs are
cached in succs, and the successor edge flags are cached in succ_flags.
@@ -1558,6 +1560,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);
--- gcc/tree-ssanames.c
+++ gcc/tree-ssanames.c
@@ -44,6 +44,7 @@ along with GCC; see the file COPYING3. If not see
#include "is-a.h"
#include "gimple.h"
#include "gimple-ssa.h"
+#include "gimple-iterator.h"
#include "tree-phinodes.h"
#include "ssa-iterators.h"
#include "stringpool.h"
@@ -563,6 +564,27 @@ 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;
+ def_operand_p def;
+ FOR_EACH_PHI_OR_STMT_DEF (def, stmt, i, SSA_OP_DEF)
+ {
+ tree var = DEF_FROM_PTR (def);
+ if (TREE_CODE (var) != SSA_NAME)
+ continue;
+ reset_flow_sensitive_info (var);
+ }
+ }
+}
/* Release all the SSA_NAMEs created by STMT. */
--- 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
