From patchwork Mon Nov 28 05:25:00 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kugan Vivekanandarajah X-Patchwork-Id: 84355 Delivered-To: patch@linaro.org Received: by 10.140.20.101 with SMTP id 92csp989789qgi; Sun, 27 Nov 2016 21:25:33 -0800 (PST) X-Received: by 10.84.218.136 with SMTP id r8mr45718700pli.23.1480310733592; Sun, 27 Nov 2016 21:25:33 -0800 (PST) Return-Path: Received: from sourceware.org (server1.sourceware.org. [209.132.180.131]) by mx.google.com with ESMTPS id 43si25105782pla.193.2016.11.27.21.25.33 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sun, 27 Nov 2016 21:25:33 -0800 (PST) Received-SPF: pass (google.com: domain of gcc-patches-return-442746-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-442746-patch=linaro.org@gcc.gnu.org designates 209.132.180.131 as permitted sender) smtp.mailfrom=gcc-patches-return-442746-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:from:message-id:date:mime-version :in-reply-to:content-type; q=dns; s=default; b=Ovzd+ON7/n5Z2mvfv LpqqfvwVtX+tM2rzN6hDNSWouEIuvHuMOvhfSLCXxAlAKKYz28F1nFl80qtjbEUO AjtUce3ohBVZCJVoObN7llezgEpwXIdIb7KwE7wv5KG1IPvg8tN73L3DpdxDzObr iWDteGFXlBvNQVxVlJPre+CgEQ= 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:from:message-id:date:mime-version :in-reply-to:content-type; s=default; bh=+X33ZnJzIXPMlahPBUSCWlx L1mY=; b=JYCEpjtaHIk8KHDnw+Mgf9uVVVZywi1Wq8a5fepRNWSIPxbTXVdaCuP 0etldzRRim8ZGZiZL34pNWaCNgTzuRpM1MTeejHsRySRCQ3AZpCi8JJqO6nmOS0f MlSXqZLcitDMG+/izdooeYrBvQVqpII/h7JahsczC1LDh7ZzJRTw= Received: (qmail 18398 invoked by alias); 28 Nov 2016 05:25:20 -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 18378 invoked by uid 89); 28 Nov 2016 05:25:19 -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=H*p:D*org, fallout, preferable, fox X-HELO: mail-pg0-f44.google.com Received: from mail-pg0-f44.google.com (HELO mail-pg0-f44.google.com) (74.125.83.44) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 28 Nov 2016 05:25:09 +0000 Received: by mail-pg0-f44.google.com with SMTP id p66so53002806pga.2 for ; Sun, 27 Nov 2016 21:25:09 -0800 (PST) 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:from:message-id:date :user-agent:mime-version:in-reply-to; bh=N4t4pWDizG+giEDQx+N17MJjYLI9ZSixBlhSwbl/NKQ=; b=lgY7VI2au6UXW5H6TBCRaec1+ttE7KSqgouKggdyVUtIz1AxdXrSbAsojI1byVB7kJ Nu2/wPt8Ii3LC4xmsweB9Wgqhr9bFCsmalbfuqWMEhhUsoPGEDLl98s/Qys6f1k8yFd9 ti33yGcxpAXV944Ujm3JVCqPP1QjO/SZ9azaHmZtFqqhbk5fw6gfY1zAZLhE8uWBspuN sjZOppqGLYdk5vH6+CRzxfUtIeFSOCfB4REg+nMIOXaUeo55v1xL3CT58hUsgLMQHXK5 lu1Y3sdSjep1Lypqqh1HOUvARkMStDhhk/zojYuPBpzGtpJQyYW4kTtKUjs8FOvVB1AE 0T4g== X-Gm-Message-State: AKaTC02Kb/cRrRJRB+26+aIR0bAYW5kV0RTlaFjrOix0v731elH5rCtJK2v5qaS21PbKYtD7 X-Received: by 10.84.213.150 with SMTP id g22mr45494373pli.11.1480310706549; Sun, 27 Nov 2016 21:25:06 -0800 (PST) Received: from [10.1.1.2] (58-6-183-210.dyn.iinet.net.au. [58.6.183.210]) by smtp.gmail.com with ESMTPSA id p68sm83583583pfd.11.2016.11.27.21.25.04 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sun, 27 Nov 2016 21:25:05 -0800 (PST) Subject: Re: [PR78365] ICE in determine_value_range, at tree-ssa-loo p-niter.c:413 To: Richard Biener , Jan Hubicka , "gcc-patches@gcc.gnu.org" References: <68396c67-fdcc-33ed-5463-a07502959865@linaro.org> <20161123153317.wjyjxgp6ltg5cp6t@virgil.suse.cz> From: kugan Message-ID: <4f03c618-081b-e5b8-5ef6-6abdfddd9d9b@linaro.org> Date: Mon, 28 Nov 2016 16:25:00 +1100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: X-IsSubscribed: yes Hi, On 24/11/16 19:48, Richard Biener wrote: > On Wed, Nov 23, 2016 at 4:33 PM, Martin Jambor wrote: >> Hi, >> >> On Fri, Nov 18, 2016 at 12:38:18PM +1100, kugan wrote: >>> Hi, >>> >>> I was relying on ipa_get_callee_param_type to get type of parameter and then >>> convert arguments to this type while computing jump functions. However, in >>> cases like shown in PR78365, ipa_get_callee_param_type, instead of giving >>> up, would return the wrong type. >> >> At what stage does this happen? During analysis >> (ipa_compute_jump_functions_for_edge) or at WPA >> (propagate_constants_accross_call)? Both? > > Hmm, where does jump function compute require the callee type? > In my view the jump function should record > > (expected-incoming-type) arg [OP X] > > for each call argument in its body. Thus required conversions are > done at WPA propagation time. > >>> I think the current uses of >>> ipa_get_callee_param_type are fine with this. >>> >>> Attached patch now uses callee's DECL_ARGUMENTS to get the type. If it >>> cannot be found, then I would give up and set the jump function to varying. >> >> But DECL_ARGUMENTS is not available at WPA stage with LTO so your >> patch would make our IPA layer to optimize less with LTO. This was >> the reason to go through the hoops of TYPE_ARG_TYPES in the first >> place. >> >> If TYPE_ARG_TYPES cannot be trusted, then I'm afraid we are back to >> square one and indeed need to put the correct type in jump functions. > > If DECL_ARGUMENTS is not available at WPA stage then I see no other > way than to put the types on the jump functions. Here is a patch that does this. To fox PR78365, in ipa_get_callee_param_type, I am now checking DECL_ARGUMENTS first. I lto bootstrapped and regression tested on x86_64-linux-gnu and ppc64le-linux with no new regressions. I will build Firefox and measure the memory usage as Honza suggested based on the feedback. Thanks, Kugan gcc/ChangeLog: 2016-11-28 Kugan Vivekanandarajah PR IPA/78365 * ipa-cp.c (propagate_vr_accross_jump_function): Remove param_type argument and use the one set in jump_func. (propagate_constants_accross_call): Likewise. * ipa-prop.c (ipa_get_callee_param_type): Chedk DECL_ARGUMENTS first. (ipa_compute_jump_functions_for_edge): Set param_type for jump_func. (ipa_write_jump_function): Stream param_type. (ipa_read_jump_function): Likewise. gcc/testsuite/ChangeLog: 2016-11-28 Kugan Vivekanandarajah PR IPA/78365 * gcc.dg/torture/pr78365.c: New test. >> If just preferring DECL_ARGUMENTS is enough, then changing >> ipa_get_callee_param_type to use that if is is available, as Richi >> suggested, would indeed be preferable. But if even falling back on it >> can cause errors, then I am not sure if it helps. >> >> In any event, thanks for diligently dealing with the fallout, >> >> Martin >> >> >>> >>> Bootstrapped and regression tested on x86_64-linux-gnu with no new >>> regressions. Is this OK for trunk? >>> >>> Thanks, >>> Kugan >>> >>> gcc/testsuite/ChangeLog: >>> >>> 2016-11-18 Kugan Vivekanandarajah >>> >>> PR IPA/78365 >>> * gcc.dg/torture/pr78365.c: New test. >>> >>> gcc/ChangeLog: >>> >>> 2016-11-18 Kugan Vivekanandarajah >>> >>> PR IPA/78365 >>> * ipa-cp.c (propagate_constants_accross_call): get param type from callees >>> DECL_ARGUMENTS if available. >>> * ipa-prop.c (ipa_compute_jump_functions_for_edge): Likewise. >>> (ipcp_update_vr): Remove now redundant conversion of precision for VR. >>> * ipa-prop.h: Make ipa_get_callee_param_type local again. diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c index 2ec671f..3d50041 100644 --- a/gcc/ipa-cp.c +++ b/gcc/ipa-cp.c @@ -1846,11 +1846,11 @@ propagate_bits_accross_jump_function (cgraph_edge *cs, int idx, ipa_jump_func *j static bool propagate_vr_accross_jump_function (cgraph_edge *cs, ipa_jump_func *jfunc, - struct ipcp_param_lattices *dest_plats, - tree param_type) + struct ipcp_param_lattices *dest_plats) { struct ipcp_param_lattices *src_lats; ipcp_vr_lattice *dest_lat = &dest_plats->m_value_range; + tree param_type = jfunc->param_type; if (dest_lat->bottom_p ()) return false; @@ -1895,9 +1895,9 @@ propagate_vr_accross_jump_function (cgraph_edge *cs, tree val = ipa_get_jf_constant (jfunc); if (TREE_CODE (val) == INTEGER_CST) { + val = fold_convert (param_type, val); if (TREE_OVERFLOW_P (val)) val = drop_tree_overflow (val); - val = fold_convert (param_type, val); jfunc->vr_known = true; jfunc->m_vr.type = VR_RANGE; jfunc->m_vr.min = val; @@ -2247,7 +2247,6 @@ propagate_constants_accross_call (struct cgraph_edge *cs) { struct ipa_jump_func *jump_func = ipa_get_ith_jump_func (args, i); struct ipcp_param_lattices *dest_plats; - tree param_type = ipa_get_callee_param_type (cs, i); dest_plats = ipa_get_parm_lattices (callee_info, i); if (availability == AVAIL_INTERPOSABLE) @@ -2264,8 +2263,7 @@ propagate_constants_accross_call (struct cgraph_edge *cs) dest_plats); if (opt_for_fn (callee->decl, flag_ipa_vrp)) ret |= propagate_vr_accross_jump_function (cs, - jump_func, dest_plats, - param_type); + jump_func, dest_plats); else ret |= dest_plats->m_value_range.set_to_bottom (); } diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c index 90c19fc..235531b 100644 --- a/gcc/ipa-prop.c +++ b/gcc/ipa-prop.c @@ -1651,14 +1651,24 @@ determine_locally_known_aggregate_parts (gcall *call, tree arg, /* Return the Ith param type of callee associated with call graph edge E. */ -tree +static tree ipa_get_callee_param_type (struct cgraph_edge *e, int i) { int n; + tree t = e->callee ? DECL_ARGUMENTS (e->callee->decl) : NULL_TREE; + if (t) + for (n = 0; n < i; n++) + { + if (!t) + return NULL; + t = TREE_CHAIN (t); + } + if (t) + return TREE_TYPE (t); tree type = (e->callee ? TREE_TYPE (e->callee->decl) : gimple_call_fntype (e->call_stmt)); - tree t = TYPE_ARG_TYPES (type); + t = TYPE_ARG_TYPES (type); for (n = 0; n < i; n++) { @@ -1670,15 +1680,6 @@ ipa_get_callee_param_type (struct cgraph_edge *e, int i) return TREE_VALUE (t); if (!e->callee) return NULL; - t = DECL_ARGUMENTS (e->callee->decl); - for (n = 0; n < i; n++) - { - if (!t) - return NULL; - t = TREE_CHAIN (t); - } - if (t) - return TREE_TYPE (t); return NULL; } @@ -1724,6 +1725,7 @@ ipa_compute_jump_functions_for_edge (struct ipa_func_body_info *fbi, useful_context = true; } + jfunc->param_type = param_type; if (POINTER_TYPE_P (TREE_TYPE (arg))) { bool addr_nonzero = false; @@ -4709,6 +4711,7 @@ ipa_write_jump_function (struct output_block *ob, int i, count; streamer_write_uhwi (ob, jump_func->type); + stream_write_tree (ob, jump_func->param_type, true); switch (jump_func->type) { case IPA_JF_UNKNOWN: @@ -4792,6 +4795,7 @@ ipa_read_jump_function (struct lto_input_block *ib, int i, count; jftype = (enum jump_func_type) streamer_read_uhwi (ib); + jump_func->param_type = stream_read_tree (ib, data_in); switch (jftype) { case IPA_JF_UNKNOWN: diff --git a/gcc/ipa-prop.h b/gcc/ipa-prop.h index 0e75cf4..8dcce99 100644 --- a/gcc/ipa-prop.h +++ b/gcc/ipa-prop.h @@ -180,6 +180,7 @@ struct GTY (()) ipa_jump_func /* Information about value range. */ bool vr_known; + tree param_type; value_range m_vr; enum jump_func_type type; @@ -818,7 +819,6 @@ ipa_parm_adjustment *ipa_get_adjustment_candidate (tree **, bool *, ipa_parm_adjustment_vec, bool); void ipa_release_body_info (struct ipa_func_body_info *); -tree ipa_get_callee_param_type (struct cgraph_edge *e, int i); /* From tree-sra.c: */ tree build_ref_for_offset (location_t, tree, HOST_WIDE_INT, bool, tree, diff --git a/gcc/testsuite/gcc.dg/torture/pr78365.c b/gcc/testsuite/gcc.dg/torture/pr78365.c index e69de29..5180a01 100644 --- a/gcc/testsuite/gcc.dg/torture/pr78365.c +++ b/gcc/testsuite/gcc.dg/torture/pr78365.c @@ -0,0 +1,21 @@ +/* { dg-do compile } */ + +int a, b, c; +char d; +static void fn1 (void *, int); +int fn2 (int); + +void fn1 (cc, yh) void *cc; +char yh; +{ + char y; + a = fn2(c - b + 1); + for (; y <= yh; y++) + ; +} + +void fn3() +{ + fn1((void *)fn3, 1); + fn1((void *)fn3, d - 1); +}