From patchwork Mon Oct 17 08:19:33 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kugan Vivekanandarajah X-Patchwork-Id: 77715 Delivered-To: patch@linaro.org Received: by 10.140.97.247 with SMTP id m110csp283250qge; Mon, 17 Oct 2016 01:20:05 -0700 (PDT) X-Received: by 10.98.25.67 with SMTP id 64mr36477301pfz.46.1476692405420; Mon, 17 Oct 2016 01:20:05 -0700 (PDT) Return-Path: Received: from sourceware.org (server1.sourceware.org. [209.132.180.131]) by mx.google.com with ESMTPS id yr4si24797449pab.266.2016.10.17.01.20.05 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 17 Oct 2016 01:20:05 -0700 (PDT) Received-SPF: pass (google.com: domain of gcc-patches-return-438745-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-438745-patch=linaro.org@gcc.gnu.org designates 209.132.180.131 as permitted sender) smtp.mailfrom=gcc-patches-return-438745-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=VaQwFteiAbIf5SZM2 tJ5QLkrI2ioBtoJPSOLS5HhWxtRKf9hHibh/QQFhfNvl1JDENMoPyqwuphTBKd/I RLqT0p98Z7J6USirqMTLiXIkTOmn4fdcvjlDkVzIJSZFUOiXFRjvKEAmT+RABfvA EenTtjCbGd6d6jminBcHtIY5uE= 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=k8FPL2rrBgSvrLn49qgIjT5 atmE=; b=mXsiTHwLTHQV/XFVc928u8I3KAG5VzdgRYUvPbbHT2Q2rHfwjXahO30 h9PaukuqZWoQXeAzXBL9w2gr7lYjepC11kBw9mcmqDvr4uiYN528ouzvytWZ1YNt NVfMj9GkAIGj+YuOztJvz3BTf+R3VI3s6qazvRR3PRGjbt/E7BKU= Received: (qmail 126666 invoked by alias); 17 Oct 2016 08:19:52 -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 126652 invoked by uid 89); 17 Oct 2016 08:19:51 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.2 spammy=y_, 1467, 146, 7, 88, 6 X-HELO: mail-pa0-f41.google.com Received: from mail-pa0-f41.google.com (HELO mail-pa0-f41.google.com) (209.85.220.41) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 17 Oct 2016 08:19:41 +0000 Received: by mail-pa0-f41.google.com with SMTP id ry6so61649313pac.3 for ; Mon, 17 Oct 2016 01:19:40 -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=13ufVTnAFosUM2RGOR5u/Gi4aWGm/+BlAUsgKEYg/d0=; b=fQ6rLRGbcY4R9LB2xwRAkq7/gjRWSWM67vB1UQEAHVuEOBGYPwGREVdGr5KdnsQr8C jBdWFu6JrRKQ0+S+qdZBuYpl14b045bXukZ6h1pr/C3ceBMDYSy7FbnC1a6G3uhJLxe/ 17oIB8QQmmyGQ5UQD/p21JMHhsMahycsSddlXTWmYytD+jk/iw0MbKZvNIRUZx+vKgiq TFGevSXDckGyYkIFIaWhNbdj9jEEIUoTSZFwbroIZOOxXreG48wQMsedJHI74GEgZeKX OGSmO7e2PtZ+NsGKyWB8Cq8iYBT139y4YNTMK89jp4hHnWym247KvFnCXa61UHaOmKVL YGMA== X-Gm-Message-State: AA6/9RnXIfipmHLf7J7FZKf1hUnHmSccn0ZPtYZL7kI5s+w2ENszZ7e0/44vT5u6FJdqWcxW X-Received: by 10.66.41.17 with SMTP id b17mr30617378pal.53.1476692379478; Mon, 17 Oct 2016 01:19:39 -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 w125sm32016912pfw.12.2016.10.17.01.19.36 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 17 Oct 2016 01:19:38 -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> <8c2fd9cf-2504-387a-04e5-5ba4db2d87f3@linaro.org> Cc: Jakub Jelinek , "gcc-patches@gcc.gnu.org" , Jeff Law From: kugan Message-ID: Date: Mon, 17 Oct 2016 19:19:33 +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 14/10/16 23:53, Richard Biener wrote: > On Fri, Oct 14, 2016 at 1:12 AM, kugan > wrote: >> 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. > > Hmm, sorry for another request, but seeing the testcase change I rather > want to expose the conservatism only at find_what_p_points_to time. > > Thus, > > @@ -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. */ > > remove this hunk > > @@ -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); > > and add pi->pt.null = true; here (with the comment from above). This > preserves the (possibly incorrect) PTA solution in the dumps but does > not leak it to SSA_NAME_PTR_INFO. > > +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 > > set to true > > + 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. > + > > Ok with those changes. Thanks for the review. Here is the updated patch. I also had to add pt.null to true in pt_solution_reset. I will commit this version if there is no objection. Thanks, Kugan > Thanks, > Richard. > > >> 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 8349bce90dcf2b17b6f129149b2073e7acc6fa8c 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 | 12 +++++++-- gcc/tree-ssanames.c | 29 +++++++++++++++++++++ gcc/tree-ssanames.h | 2 ++ gcc/tree-vrp.c | 44 +++++++++++++++++++++----------- 8 files changed, 74 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..78f4533 100644 --- a/gcc/tree-ssa-structalias.c +++ b/gcc/tree-ssa-structalias.c @@ -6451,6 +6451,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 +6467,12 @@ find_what_p_points_to (tree fndecl, tree p) pi = get_ptr_info (p); pi->pt = find_what_var_points_to (fndecl, vi); + /* Conservatively set to NULL from PTA (to true). */ + pi->pt.null = 1; + /* Preserve pointer nonnull computed by VRP. See get_ptr_nonnull + in gcc/tree-ssaname.c for more information. */ + if (nonnull) + set_ptr_nonnull (p); } @@ -6505,6 +6512,7 @@ pt_solution_reset (struct pt_solution *pt) { memset (pt, 0, sizeof (struct pt_solution)); pt->anything = true; + pt->null = true; } /* Set the points-to solution *PT to point only to the variables @@ -6599,10 +6607,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..913d142 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 true 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