From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 84130 invoked by alias); 25 Sep 2015 15:46:56 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 84119 invoked by uid 89); 25 Sep 2015 15:46:54 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.2 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Fri, 25 Sep 2015 15:46:53 +0000 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (Postfix) with ESMTPS id 2FAE792486; Fri, 25 Sep 2015 15:46:52 +0000 (UTC) Received: from redhat.com (ovpn-204-22.brq.redhat.com [10.40.204.22]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t8PFkmkR011631 (version=TLSv1/SSLv3 cipher=AES256-SHA bits=256 verify=NO); Fri, 25 Sep 2015 11:46:51 -0400 Date: Fri, 25 Sep 2015 16:04:00 -0000 From: Marek Polacek To: Richard Biener Cc: GCC Patches Subject: Re: [PATCH] Clear variables with stale SSA_NAME_RANGE_INFO (PR tree-optimization/67690) Message-ID: <20150925154647.GD6184@redhat.com> References: <20150924154452.GA6184@redhat.com> <20150925134934.GC6184@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150925134934.GC6184@redhat.com> User-Agent: Mutt/1.5.23 (2014-03-12) X-SW-Source: 2015-09/txt/msg01986.txt.bz2 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 , > > > 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 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