On September 29, 2015 4:21:16 PM GMT+02:00, Marek Polacek <pola...@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
><pola...@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 <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 (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
