From patchwork Fri Dec 9 04:36:44 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kugan Vivekanandarajah X-Patchwork-Id: 87380 Delivered-To: patch@linaro.org Received: by 10.140.20.101 with SMTP id 92csp123252qgi; Thu, 8 Dec 2016 20:37:20 -0800 (PST) X-Received: by 10.99.122.92 with SMTP id j28mr138027930pgn.64.1481258240141; Thu, 08 Dec 2016 20:37:20 -0800 (PST) Return-Path: Received: from sourceware.org (server1.sourceware.org. [209.132.180.131]) by mx.google.com with ESMTPS id z65si31871510plh.175.2016.12.08.20.37.19 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 08 Dec 2016 20:37:20 -0800 (PST) Received-SPF: pass (google.com: domain of gcc-patches-return-443854-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-443854-patch=linaro.org@gcc.gnu.org designates 209.132.180.131 as permitted sender) smtp.mailfrom=gcc-patches-return-443854-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=tC0hpaUOMSuM1LB+c /+J222GSnQk/Up8hRDAQ/6cZqI1YFQrOcBETQjPhkAK1vWXpIORj0gRmUGH/ozEj 2h4gUFbd4zrWVrDLSHTLjt9rzASuuRyE5bDaBdwBUqP1UmU2BsGlovZI9YcdnDid fYrx4LoPkT2fF8l5dJrzYnvrtU= 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=q0l4YjMY0meU9JdTtdRP4Zu q4ks=; b=S7bIJVn74ut5kZXpCDn9jYFa0cyZOsTNRZ8VUt31jyNkQbSDSN8u6AT VTvG5L/ljEfvPZpZwsIp4D/yyzPaFSoz2guF6NXK5BkyHRq1AGmgAPBYsiRtPGOS i6FpO+A+t2TndcgRM8+iAJjLFmBXDJI/6T1VMmGvv18zYqO1keNM= Received: (qmail 5275 invoked by alias); 9 Dec 2016 04:37:02 -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 5209 invoked by uid 89); 9 Dec 2016 04:37:01 -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=IPA, mjamborsusecz, Jambor, jambor X-HELO: mail-pg0-f53.google.com Received: from mail-pg0-f53.google.com (HELO mail-pg0-f53.google.com) (74.125.83.53) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 09 Dec 2016 04:36:51 +0000 Received: by mail-pg0-f53.google.com with SMTP id p66so3355518pga.2 for ; Thu, 08 Dec 2016 20:36:51 -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=qWXPcNSeJd2IYkVX8MLtq6Xp7Y2fTGljO6CCgv7szeI=; b=WWiiWAn169LrDVDSScmTkEgope61YYL6PQLXPOyU5XbKOTO3C8H+8uv1fwUSGM0YXP wUDE7/kR59kibQhNbtcqTTBFMInQsmf22kU7BxfwN6c9jfIqKGClL8XLmmwddjVt4Cez duEx8fKCh6rwhJyqUfTPDnmSbJdcdvd34vBgzR3pcof2X0DV65o7mRBU0+ia6mvEGKak 4geO7uDR35m/2V/rt/H4Kk/5IgtEMFgh4xivKpJAV29iu1D25ZqYt1TzX47+4TnvLc8w 5BFQKYhPzpxb5WfOdg756uf+mUQgrrAsh4f4l8sKhjduKTv7tm22pgikd/AdjT6Fho1P Ds1A== X-Gm-Message-State: AKaTC03gVq/xqGfD5CqRLrz5KF13A8/OtG8WtHqzDagn+RD4QpCal4roLS4Pfa7WyBwETw9q X-Received: by 10.99.99.195 with SMTP id x186mr141482771pgb.100.1481258209685; Thu, 08 Dec 2016 20:36:49 -0800 (PST) Received: from [10.1.1.4] (58-6-183-210.dyn.iinet.net.au. [58.6.183.210]) by smtp.gmail.com with ESMTPSA id t184sm53708314pgt.36.2016.12.08.20.36.46 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 08 Dec 2016 20:36:49 -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" , Martin Jambor References: <68396c67-fdcc-33ed-5463-a07502959865@linaro.org> <20161123153317.wjyjxgp6ltg5cp6t@virgil.suse.cz> <4f03c618-081b-e5b8-5ef6-6abdfddd9d9b@linaro.org> <20161207100853.ae3qcpo5ycblqxau@virgil.suse.cz> From: kugan Message-ID: Date: Fri, 9 Dec 2016 15:36:44 +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: <20161207100853.ae3qcpo5ycblqxau@virgil.suse.cz> X-IsSubscribed: yes Hi Martin, On 07/12/16 21:08, Martin Jambor wrote: > Hello Kugan, > > sorry, I have lost track of this patch and re-discovered it only now. > > On Mon, Nov 28, 2016 at 04:25:00PM +1100, kugan wrote: >> 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. >> > > So, do you have any numbers? I will do it. How do you measure the gcc's memory usage while building Firefox. I saw Honza's LTO blog talks about using vmstat result. Any tips please? > > >> 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); > > If TYPE_ARG_TYPES is inherently unreliable then I am afraid you must > not fallback on it but rather return NULL if cs->callee is not > available and adjust the caller to give up in that case. > > (I have checked both testcases that we hope to fix with types in jump > functions and the gimple_call_fntype is the same as > TREE_TYPE(e->callee->decl) so that does not help either). Do you like the attached patch where I have removed it. >> >> 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; > > Please add a comment describing what this is. Done. LTO bootstrapped and regression tested on x86_64-linux-gnu with https://gcc.gnu.org/ml/gcc-patches/2016-12/msg00716.html and there is no new regressions. Is this OK? Thanks, Kugan > > Otherwise, the intent looks fine to me. > > Thanks! > > Martin > diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c index 2ec671f..9853467 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; @@ -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 642111d..21ee251 100644 --- a/gcc/ipa-prop.c +++ b/gcc/ipa-prop.c @@ -1659,26 +1659,13 @@ 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 type = (e->callee - ? TREE_TYPE (e->callee->decl) - : gimple_call_fntype (e->call_stmt)); - tree t = TYPE_ARG_TYPES (type); - - for (n = 0; n < i; n++) - { - if (!t) - break; - t = TREE_CHAIN (t); - } - if (t) - return TREE_VALUE (t); if (!e->callee) - return NULL; - t = DECL_ARGUMENTS (e->callee->decl); + return NULL; + tree t = DECL_ARGUMENTS (e->callee->decl); for (n = 0; n < i; n++) { if (!t) @@ -1732,6 +1719,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; @@ -4717,6 +4705,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: @@ -4800,6 +4789,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..eeb0f6b 100644 --- a/gcc/ipa-prop.h +++ b/gcc/ipa-prop.h @@ -182,6 +182,9 @@ struct GTY (()) ipa_jump_func bool vr_known; value_range m_vr; + /* Type of callee param corresponding to this jump_func. */ + tree param_type; + enum jump_func_type type; /* Represents a value of a jump function. pass_through is used only in jump function context. constant represents the actual constant in constant jump @@ -818,7 +821,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); +}