From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 15110 invoked by alias); 7 Dec 2016 22:42:22 -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 15091 invoked by uid 89); 7 Dec 2016 22:42:21 -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=our X-HELO: mail-pg0-f45.google.com Received: from mail-pg0-f45.google.com (HELO mail-pg0-f45.google.com) (74.125.83.45) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 07 Dec 2016 22:42:11 +0000 Received: by mail-pg0-f45.google.com with SMTP id 3so166620388pgd.0 for ; Wed, 07 Dec 2016 14:42:11 -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:content-transfer-encoding; bh=yIApNeI5FbnTJ+Za6Sb6NlOj0A8ohi18dDzk2uFA1Xw=; b=lx/locwyHkxc3tRZn4v2ChS4xpsF8oYT7y2hR80ct8qQSyskp1AB1qmNm/Y7OS+Uqy pZkrmAS8x7hRFROhiq8mW9EU17Sb5/FwTJg3cvYnFuQ0Mg/BnbAThSpssHNfwXsj3D+l KayG4K+fXGPhg0xPoHWyD8ZECMDLmcCdPRPDcmXhuEnQpS66IPFxk6C9yRfymZjoRu3Z pq4+UutNOOOI+43l6YkA+9tCGb+j/3FQlZa5tK90GcdeS67438/YdMMOLpYqPv98VEBu CEBxhrIhZECuJ8kvM0sUBMlMduFD+BLSj3D7kmKRmv+2lwsri21bJJmxK/lhWCO+j2cq Nk3g== X-Gm-Message-State: AKaTC022RoaczGOHrEvWgcQGaCT9x5zHo8uCR+QudAw7cADTrha1FewhaGNehLvVHDeGOFxM X-Received: by 10.99.164.9 with SMTP id c9mr126279419pgf.141.1481150529570; Wed, 07 Dec 2016 14:42:09 -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 o29sm45132465pgn.28.2016.12.07.14.42.06 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 07 Dec 2016 14:42:08 -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> <4f03c618-081b-e5b8-5ef6-6abdfddd9d9b@linaro.org> <20161207100853.ae3qcpo5ycblqxau@virgil.suse.cz> From: kugan Message-ID: <5a4cf180-a57a-7b6e-7711-8eb4e73135f1@linaro.org> Date: Wed, 07 Dec 2016 22:42:00 -0000 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> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2016-12/txt/msg00608.txt.bz2 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 am starting this now. Do we have an easiest and accurate way to measure memory usage by gcc for Firefox compilation. Honza's LTO blog talks about using vmstat. Thanks, Kugan > > >> 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). > >> >> 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. > > Otherwise, the intent looks fine to me. > > Thanks! > > Martin >