From patchwork Thu Oct 13 23:12:03 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kugan Vivekanandarajah X-Patchwork-Id: 77638 Delivered-To: patch@linaro.org Received: by 10.140.97.247 with SMTP id m110csp53446qge; Thu, 13 Oct 2016 16:12:34 -0700 (PDT) X-Received: by 10.99.119.68 with SMTP id s65mr11322361pgc.160.1476400354423; Thu, 13 Oct 2016 16:12:34 -0700 (PDT) Return-Path: Received: from sourceware.org (server1.sourceware.org. [209.132.180.131]) by mx.google.com with ESMTPS id xl10si2157603pab.228.2016.10.13.16.12.34 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 13 Oct 2016 16:12:34 -0700 (PDT) Received-SPF: pass (google.com: domain of gcc-patches-return-438576-patch=linaro.org@gcc.gnu.org designates 209.132.180.131 as permitted sender) client-ip=209.132.180.131; Authentication-Results: mx.google.com; dkim=pass header.i=@gcc.gnu.org; spf=pass (google.com: domain of gcc-patches-return-438576-patch=linaro.org@gcc.gnu.org designates 209.132.180.131 as permitted sender) smtp.mailfrom=gcc-patches-return-438576-patch=linaro.org@gcc.gnu.org; dmarc=fail (p=NONE dis=NONE) header.from=linaro.org DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :subject:to:references:cc:from:message-id:date:mime-version :in-reply-to:content-type; q=dns; s=default; b=htNkPCyxXe5H9famx Sa/yrvmaYOy9hFgiL/pLNhSRdoNCc7IxayXknGrv08totWDHmB7dgXMmXpB3lO3K +YWYpnQ+taDSeKeruyVWGZ8V/Gh/AFoJjFH4pGRB+MuetqYc9gXKDPPwLjSQPR+2 MdgtRgFwGXmcsnSCrElmeFd+fk= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :subject:to:references:cc:from:message-id:date:mime-version :in-reply-to:content-type; s=default; bh=hGcLYpU9tnzpl5BNZ/MhNqk 4Rfc=; b=ULo1LPIJYMTrV1uYVi6ufcHTR2ZEocDkQFwmTVB6tVDpArci5Y5K/D4 gktJVlo54Oz156qkc0ENXI68UJPAI1eQeMDpqEfRj8WNS6KBf5TNmxvyoPiQU+0k z/fv8vLD1ADYpyoBDB/D3kdaYU9xvJzOz6OCnm0qLodzfiLIwn54= Received: (qmail 125042 invoked by alias); 13 Oct 2016 23:12:22 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 124449 invoked by uid 89); 13 Oct 2016 23:12:21 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=1.0 required=5.0 tests=AWL, BAYES_50, RCVD_IN_DNSWL_NONE, RCVD_IN_SORBS_SPAM, SPF_PASS autolearn=no version=3.3.2 spammy=y_, UD:tree-ssa-alias.h, 88, 6, 374, 6 X-HELO: mail-pf0-f174.google.com Received: from mail-pf0-f174.google.com (HELO mail-pf0-f174.google.com) (209.85.192.174) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 13 Oct 2016 23:12:11 +0000 Received: by mail-pf0-f174.google.com with SMTP id r16so17413431pfg.1 for ; Thu, 13 Oct 2016 16:12:10 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:subject:to:references:cc:from:message-id:date :user-agent:mime-version:in-reply-to; bh=xKOLZWLcxmOhT4RwJjFYPmM3QwFwqhp3AVsXDo22jFw=; b=bGIoyBk6VbI9UfyxdwnPmnplZXd3R890Jb0jYBgt6BTP7NFOmzgPg/Y3GKC0NjC0qA Z0xYb7O+byVCz9c8q23sQy+vMWYUs7fQD52VHtQUAip/g9sL6+56Fo8vKfqBRMr2V7Ne qKZthRRKcw1+QGUDzfqKHPPapqDtNvSGRWJQg6xdWA4CiT7XwGiXwxH7Usx7A0vtl5XC cCFi/lf6Sx8PCve8lqqwRWKyS54MfB3NaGlxTL2v+G6EWYdSh2udU1HdcMMFL8LiKlZa lms0bochYiFMBa+FJPQmOVJk/v7TcStLQybvDRywpVB6dxRlGqKf2VNufw/ky1f/91nw ua4Q== X-Gm-Message-State: AA6/9RlnI3B5oYiYP9MeHdPI1tT2naa2rsAF0iIFuQq+HpwprBdJVOjT9TeG6gDyzf2TDlDH X-Received: by 10.98.16.203 with SMTP id 72mr13581355pfq.57.1476400329551; Thu, 13 Oct 2016 16:12:09 -0700 (PDT) Received: from [10.1.1.7] (58-6-183-210.dyn.iinet.net.au. [58.6.183.210]) by smtp.gmail.com with ESMTPSA id s64sm22049868pfk.81.2016.10.13.16.12.06 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 13 Oct 2016 16:12:08 -0700 (PDT) Subject: Re: Set nonnull attribute to ptr_info_def based on VRP To: Richard Biener References: <717fb095-f0a9-6980-ddbd-e755a4fd6457@linaro.org> <20160808064028.GF14857@tucnak.redhat.com> <1ec93250-340d-1f6d-6556-7372c23b8a24@linaro.org> <3b8ae8ba-97e7-7363-fecc-bd0e0f85abcf@linaro.org> Cc: Jakub Jelinek , "gcc-patches@gcc.gnu.org" , Jeff Law From: kugan Message-ID: <8c2fd9cf-2504-387a-04e5-5ba4db2d87f3@linaro.org> Date: Fri, 14 Oct 2016 10:12:03 +1100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.3.0 MIME-Version: 1.0 In-Reply-To: X-IsSubscribed: yes Hi Richard, On 13/10/16 20:44, Richard Biener wrote: > On Thu, Oct 13, 2016 at 6:49 AM, kugan > wrote: >> Hi Richard, >> >>> >>> what does this try to do? Preserve info VRP computed across PTA? >>> >>> I think we didn't yet sort out the nonlocal/escaped vs. null handling >>> properly >>> (or how PTA should handle get_ptr_nonnull). The way you are using it >>> asks for pt.null to be orthogonal to nonlocal/escaped and thus having >>> nonlocal or escaped would also require setting ptr.null in PTA. It then >>> would be also more canonical to set it for pt.anything as well. Which >>> means conservatively handling it would be equivalent to flipping its >>> semantic and changing its name to pt.nonnull. >>> >>> That said, you seem to be simply "reserving" the bit for VRP, keeping it >>> conservatively true when "not computed". I guess I'm fine with this for >>> now >>> but it should be documented in the header file that way. >>> >> >> Thanks for the comments. >> >> To summarize, currently I am not relying on PTA analysis at all. Just saving >> null from VRP (or rather nonnull) and preserving it across PTA. Primary >> intention is to pass it for PARM_DECL SSA names (from ipa-vrp). >> >> In this case, using pt.anything/nonlocal/escaped will only make the result >> more pessimistic. >> >> Ideally, we should improve pt.null within PTA but for now as you said, I >> will document it. >> >> When we start using pt.null from PTA analysis, we would also have to take >> into account pt.anything/nonlocal/escaped. >> >> Does that make sense? > > Yes. Here is the revised patch based on the review. I also had to adjust two testcases since we set pt.null conservatively and dumps that too. Thanks, Kugan gcc/ChangeLog: 2016-10-14 Kugan Vivekanandarajah * tree-ssa-alias.h (pt_solution_singleton_or_null_p): Renamed from pt_solution_singleton_p. * tree-ssa-ccp.c (fold_builtin_alloca_with_align): Use renamed pt_solution_singleton_or_null_p from pt_solution_singleton_p. * tree-ssa-structalias.c (find_what_var_points_to): Conservatively set pt.null to 1. (find_what_p_points_to): Preserve pointer nonnull computed by VRP. (pt_solution_singleton_or_null_p): Renamed from pt_solution_singleton_p. * tree-ssanames.h (set_ptr_nonnull): Declare. (get_ptr_nonnull): Likewise. * tree-ssanames.c (set_ptr_nonnull): New. (get_ptr_nonnull): Likewise. * tree-vrp.c (vrp_finalize): Set ptr that are nonnull. (evrp_dom_walker::before_dom_children): Likewise. gcc/testsuite/ChangeLog: 2016-10-14 Kugan Vivekanandarajah * gcc.dg/torture/pr39074-2.c: Adjust testcase. * gcc.dg/torture/pr39074.c: Likewise. >From 0bc1c2efb600854148889bfdd9d121a3edc2841b Mon Sep 17 00:00:00 2001 From: Kugan Vivekanandarajah Date: Wed, 12 Oct 2016 13:48:07 +1100 Subject: [PATCH 1/3] Add-nonnull-to-pointer-from-VRP --- gcc/testsuite/gcc.dg/torture/pr39074-2.c | 2 +- gcc/testsuite/gcc.dg/torture/pr39074.c | 2 +- gcc/tree-ssa-alias.h | 2 +- gcc/tree-ssa-ccp.c | 2 +- gcc/tree-ssa-structalias.c | 11 ++++++-- gcc/tree-ssanames.c | 29 +++++++++++++++++++++ gcc/tree-ssanames.h | 2 ++ gcc/tree-vrp.c | 44 +++++++++++++++++++++----------- 8 files changed, 73 insertions(+), 21 deletions(-) diff --git a/gcc/testsuite/gcc.dg/torture/pr39074-2.c b/gcc/testsuite/gcc.dg/torture/pr39074-2.c index 740b463..0693f2d 100644 --- a/gcc/testsuite/gcc.dg/torture/pr39074-2.c +++ b/gcc/testsuite/gcc.dg/torture/pr39074-2.c @@ -31,4 +31,4 @@ int main() } /* { dg-final { scan-tree-dump "y.._. = { i }" "alias" } } */ -/* { dg-final { scan-tree-dump "y.._., points-to vars: { D..... }" "alias" } } */ +/* { dg-final { scan-tree-dump "y.._., points-to NULL, points-to vars: { D..... }" "alias" } } */ diff --git a/gcc/testsuite/gcc.dg/torture/pr39074.c b/gcc/testsuite/gcc.dg/torture/pr39074.c index 31ed499..54c444e 100644 --- a/gcc/testsuite/gcc.dg/torture/pr39074.c +++ b/gcc/testsuite/gcc.dg/torture/pr39074.c @@ -30,4 +30,4 @@ int main() } /* { dg-final { scan-tree-dump "y.._. = { i }" "alias" } } */ -/* { dg-final { scan-tree-dump "y.._., points-to vars: { D..... }" "alias" } } */ +/* { dg-final { scan-tree-dump "y.._., points-to NULL, points-to vars: { D..... }" "alias" } } */ diff --git a/gcc/tree-ssa-alias.h b/gcc/tree-ssa-alias.h index 6680cc0..27a06fc 100644 --- a/gcc/tree-ssa-alias.h +++ b/gcc/tree-ssa-alias.h @@ -146,7 +146,7 @@ extern void dump_alias_stats (FILE *); /* In tree-ssa-structalias.c */ extern unsigned int compute_may_aliases (void); extern bool pt_solution_empty_p (struct pt_solution *); -extern bool pt_solution_singleton_p (struct pt_solution *, unsigned *); +extern bool pt_solution_singleton_or_null_p (struct pt_solution *, unsigned *); extern bool pt_solution_includes_global (struct pt_solution *); extern bool pt_solution_includes (struct pt_solution *, const_tree); extern bool pt_solutions_intersect (struct pt_solution *, struct pt_solution *); diff --git a/gcc/tree-ssa-ccp.c b/gcc/tree-ssa-ccp.c index fe9a313..61754d8 100644 --- a/gcc/tree-ssa-ccp.c +++ b/gcc/tree-ssa-ccp.c @@ -2135,7 +2135,7 @@ fold_builtin_alloca_with_align (gimple *stmt) { bool singleton_p; unsigned uid; - singleton_p = pt_solution_singleton_p (&pi->pt, &uid); + singleton_p = pt_solution_singleton_or_null_p (&pi->pt, &uid); gcc_assert (singleton_p); SET_DECL_PT_UID (var, uid); } diff --git a/gcc/tree-ssa-structalias.c b/gcc/tree-ssa-structalias.c index 2a4ab2f..ad65c94 100644 --- a/gcc/tree-ssa-structalias.c +++ b/gcc/tree-ssa-structalias.c @@ -6382,6 +6382,8 @@ find_what_var_points_to (tree fndecl, varinfo_t orig_vi) *slot = pt = XOBNEW (&final_solutions_obstack, struct pt_solution); memset (pt, 0, sizeof (struct pt_solution)); + /* Conservatively set to NULL from PTA (to true). */ + pt->null = 1; /* Translate artificial variables into SSA_NAME_PTR_INFO attributes. */ @@ -6451,6 +6453,7 @@ find_what_p_points_to (tree fndecl, tree p) struct ptr_info_def *pi; tree lookup_p = p; varinfo_t vi; + bool nonnull = get_ptr_nonnull (p); /* For parameters, get at the points-to set for the actual parm decl. */ @@ -6466,6 +6469,10 @@ find_what_p_points_to (tree fndecl, tree p) pi = get_ptr_info (p); pi->pt = find_what_var_points_to (fndecl, vi); + /* Preserve pointer nonnull computed by VRP. See get_ptr_nonnull + in gcc/tree-ssaname.c for more information. */ + if (nonnull) + set_ptr_nonnull (p); } @@ -6599,10 +6606,10 @@ pt_solution_empty_p (struct pt_solution *pt) return the var uid in *UID. */ bool -pt_solution_singleton_p (struct pt_solution *pt, unsigned *uid) +pt_solution_singleton_or_null_p (struct pt_solution *pt, unsigned *uid) { if (pt->anything || pt->nonlocal || pt->escaped || pt->ipa_escaped - || pt->null || pt->vars == NULL + || pt->vars == NULL || !bitmap_single_bit_set_p (pt->vars)) return false; diff --git a/gcc/tree-ssanames.c b/gcc/tree-ssanames.c index 64ab13a..fe6a0f7 100644 --- a/gcc/tree-ssanames.c +++ b/gcc/tree-ssanames.c @@ -374,6 +374,35 @@ get_range_info (const_tree name, wide_int *min, wide_int *max) return SSA_NAME_RANGE_TYPE (name); } +/* Set nonnull attribute to pointer NAME. */ + +void +set_ptr_nonnull (tree name) +{ + gcc_assert (POINTER_TYPE_P (TREE_TYPE (name))); + struct ptr_info_def *pi = get_ptr_info (name); + pi->pt.null = 0; +} + +/* Return nonnull attribute of pointer NAME. */ +bool +get_ptr_nonnull (const_tree name) +{ + gcc_assert (POINTER_TYPE_P (TREE_TYPE (name))); + struct ptr_info_def *pi = SSA_NAME_PTR_INFO (name); + if (pi == NULL) + return false; + /* TODO Now pt->null is conservatively set to null in PTA + analysis. vrp is the only pass (including ipa-vrp) + that clears pt.null via set_ptr_nonull when it knows + for sure. PTA will preserves the pt.null value set by VRP. + + When PTA analysis is improved, pt.anything, pt.nonlocal + and pt.escaped may also has to be considered before + deciding that pointer cannot point to NULL. */ + return !pi->pt.null; +} + /* Change non-zero bits bitmask of NAME. */ void diff --git a/gcc/tree-ssanames.h b/gcc/tree-ssanames.h index 4496e1d..d39cc9d 100644 --- a/gcc/tree-ssanames.h +++ b/gcc/tree-ssanames.h @@ -88,6 +88,8 @@ extern void set_ptr_info_alignment (struct ptr_info_def *, unsigned int, extern void adjust_ptr_info_misalignment (struct ptr_info_def *, unsigned int); extern struct ptr_info_def *get_ptr_info (tree); +extern void set_ptr_nonnull (tree); +extern bool get_ptr_nonnull (const_tree); extern tree copy_ssa_name_fn (struct function *, tree, gimple *); extern void duplicate_ssa_name_ptr_info (tree, struct ptr_info_def *); diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c index 8a129c6..7e4f947 100644 --- a/gcc/tree-vrp.c +++ b/gcc/tree-vrp.c @@ -10603,18 +10603,24 @@ vrp_finalize (bool warn_array_bounds_p) { tree name = ssa_name (i); - if (!name - || POINTER_TYPE_P (TREE_TYPE (name)) - || (vr_value[i]->type == VR_VARYING) - || (vr_value[i]->type == VR_UNDEFINED)) - continue; + if (!name + || (vr_value[i]->type == VR_VARYING) + || (vr_value[i]->type == VR_UNDEFINED) + || (TREE_CODE (vr_value[i]->min) != INTEGER_CST) + || (TREE_CODE (vr_value[i]->max) != INTEGER_CST)) + continue; - if ((TREE_CODE (vr_value[i]->min) == INTEGER_CST) - && (TREE_CODE (vr_value[i]->max) == INTEGER_CST) - && (vr_value[i]->type == VR_RANGE - || vr_value[i]->type == VR_ANTI_RANGE)) - set_range_info (name, vr_value[i]->type, vr_value[i]->min, - vr_value[i]->max); + if (POINTER_TYPE_P (TREE_TYPE (name)) + && ((vr_value[i]->type == VR_RANGE + && range_includes_zero_p (vr_value[i]->min, + vr_value[i]->max) == 0) + || (vr_value[i]->type == VR_ANTI_RANGE + && range_includes_zero_p (vr_value[i]->min, + vr_value[i]->max) == 1))) + set_ptr_nonnull (name); + else if (!POINTER_TYPE_P (TREE_TYPE (name))) + set_range_info (name, vr_value[i]->type, vr_value[i]->min, + vr_value[i]->max); } substitute_and_fold (op_with_constant_singleton_value_range, @@ -10817,17 +10823,25 @@ evrp_dom_walker::before_dom_children (basic_block bb) def_operand_p def_p = SINGLE_SSA_DEF_OPERAND (stmt, SSA_OP_DEF); /* Set the SSA with the value range. */ if (def_p - && TREE_CODE (DEF_FROM_PTR (def_p)) == SSA_NAME - && INTEGRAL_TYPE_P (TREE_TYPE (DEF_FROM_PTR (def_p)))) + && TREE_CODE (DEF_FROM_PTR (def_p)) == SSA_NAME) { tree def = DEF_FROM_PTR (def_p); value_range *vr = get_value_range (def); - if ((vr->type == VR_RANGE - || vr->type == VR_ANTI_RANGE) + if (INTEGRAL_TYPE_P (TREE_TYPE (DEF_FROM_PTR (def_p))) + && (vr->type == VR_RANGE + || vr->type == VR_ANTI_RANGE) && (TREE_CODE (vr->min) == INTEGER_CST) && (TREE_CODE (vr->max) == INTEGER_CST)) set_range_info (def, vr->type, vr->min, vr->max); + else if (POINTER_TYPE_P (TREE_TYPE (DEF_FROM_PTR (def_p))) + && ((vr->type == VR_RANGE + && range_includes_zero_p (vr->min, + vr->max) == 0) + || (vr->type == VR_ANTI_RANGE + && range_includes_zero_p (vr->min, + vr->max) == 1))) + set_ptr_nonnull (def); } } else -- 2.7.4