From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 81006 invoked by alias); 30 Aug 2016 18:12:31 -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 80990 invoked by uid 89); 30 Aug 2016 18:12:31 -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_95,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=no version=3.3.2 spammy=ts, vrp, IPA, *ts X-HELO: mail-it0-f50.google.com Received: from mail-it0-f50.google.com (HELO mail-it0-f50.google.com) (209.85.214.50) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 30 Aug 2016 18:12:20 +0000 Received: by mail-it0-f50.google.com with SMTP id n75so52707203ith.0 for ; Tue, 30 Aug 2016 11:12:20 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=/+nNJobQFROn1WqRKU0dodGBOMclg8xN1MuZs4ESlgk=; b=SfIq9pPSkGMkbruWjSxVAZhRZfcpZAZG0zP14wxaFzPF1xT6MzVUTGl1jsf+CZ9SoG GNda/CcB3kwJT++6E4qKHSa2/s6YhH7qxsyCbR0jqMpuiTDoSp4SuEtHg/uV1kadQyFQ QHty/Zrf4MjiMBfWzYiXIanB1nSIsDJiWTLZsrtlUnQoJF0beCCKmUEJEZPzTcomM6os aNmjk8JKUyuui+NUOp8lfx2wtVJh8TqJ+Eh2gXZwaNXofWy7gx+ilOyHdp6dUB24MG3m qSpASo6uWIgX1mnfIEwaOXmmDPGlbXmNC1LM30LQ90okefCBbKxl6NHHCfxTdWbropVN cMaA== X-Gm-Message-State: AE9vXwMVf1gojHFAGh/8PWfL2JloSxPXYoYWWXSILszG9VfkeRzJB450HY3V6DfSuFI1eBLno9kddbe2C4+ijXTi X-Received: by 10.107.32.135 with SMTP id g129mr549193iog.137.1472580739151; Tue, 30 Aug 2016 11:12:19 -0700 (PDT) MIME-Version: 1.0 Received: by 10.36.65.137 with HTTP; Tue, 30 Aug 2016 11:12:18 -0700 (PDT) In-Reply-To: References: <57886949.8010300@linaro.org> <57886A2A.4070703@linaro.org> <57886ABA.2060404@linaro.org> <20160715122309.glir5vk5ttwoagdp@virgil.suse.cz> <578DE340.4010904@linaro.org> <578E9B16.2080100@linaro.org> <20160721125416.GA23760@kam.mff.cuni.cz> From: Prathamesh Kulkarni Date: Tue, 30 Aug 2016 18:12:00 -0000 Message-ID: Subject: Re: [RFC][IPA-VRP] Add support for IPA VRP in ipa-cp/ipa-prop To: Kugan Vivekanandarajah Cc: Jan Hubicka , "gcc-patches@gcc.gnu.org" , Richard Biener Content-Type: text/plain; charset=UTF-8 X-IsSubscribed: yes X-SW-Source: 2016-08/txt/msg02083.txt.bz2 On 30 August 2016 at 10:50, Kugan Vivekanandarajah wrote: > Hi Honza, > > Here is a re-based version which also addresses the review comments. > > On 21/07/16 22:54, Jan Hubicka wrote: >>> Maybe it is better to separate value range and alignment summary >>> writing/reading to different functions. Here is another updated >>> version which does this. >> >> Makes sense to me. Note that the alignment summary propagation can be either >> handled by doing bitwise constant propagation and/or extending our value ranges >> by stride (as described in >> http://www.lighterra.com/papers/valuerangeprop/Patterson1995-ValueRangeProp.pdf >> I would like it to go eventually away in favour of more generic solution. >> >>> -/* If DEST_PLATS already has aggregate items, check that aggs_by_ref matches >>> +/* Propagate value range across jump function JFUNC that is associated with >>> + edge CS and update DEST_PLATS accordingly. */ >>> + >>> +static bool >>> +propagate_vr_accross_jump_function (cgraph_edge *cs, >>> + ipa_jump_func *jfunc, >>> + struct ipcp_param_lattices *dest_plats) >>> +{ >>> + struct ipcp_param_lattices *src_lats; >>> + ipcp_vr_lattice *dest_lat = &dest_plats->m_value_range; >>> + >>> + if (dest_lat->bottom_p ()) >>> + return false; >>> + >>> + if (jfunc->type == IPA_JF_PASS_THROUGH) >>> + { >>> + struct ipa_node_params *caller_info = IPA_NODE_REF (cs->caller); >>> + int src_idx = ipa_get_jf_pass_through_formal_id (jfunc); >>> + src_lats = ipa_get_parm_lattices (caller_info, src_idx); >>> + >>> + if (ipa_get_jf_pass_through_operation (jfunc) == NOP_EXPR) >>> + return dest_lat->meet_with (src_lats->m_value_range); >> >> Clearly we can propagate thorugh expressions here (PLUS_EXPR). I have run >> into similar issue in loop code that builds simple generic expresisons >> (like (int)ssa_name+10) and it would be nice to have easy way to deterine >> their value range based on the knowledge of SSA_NAME's valur range. >> >> Bit this is fine for initial implementaiotn for sure. > > Indeed. I will do this as a follow up. > >>> >>> +/* Look up all VR information that we have discovered and copy it over >>> + to the transformation summary. */ >>> + >>> +static void >>> +ipcp_store_vr_results (void) >>> +{ >>> + cgraph_node *node; >>> + >>> + FOR_EACH_FUNCTION_WITH_GIMPLE_BODY (node) >>> + { >>> + ipa_node_params *info = IPA_NODE_REF (node); >>> + bool found_useful_result = false; >>> + >>> + if (!opt_for_fn (node->decl, flag_ipa_vrp)) >>> + { >>> + if (dump_file) >>> + fprintf (dump_file, "Not considering %s for VR discovery " >>> + "and propagate; -fipa-ipa-vrp: disabled.\n", >>> + node->name ()); >>> + continue; >> >> I belive you need to also prevent propagation through functions copmiled with >> -fno-ipa-vrp, not only prevent any transformations. > > Do you mean the following, I was following other implementations. > > @@ -2264,6 +2430,11 @@ propagate_constants_accross_call (struct > cgraph_edge *cs) > &dest_plats->bits_lattice); > ret |= propagate_aggs_accross_jump_function (cs, jump_func, > dest_plats); > + if (opt_for_fn (callee->decl, flag_ipa_vrp)) > + ret |= propagate_vr_accross_jump_function (cs, > + jump_func, dest_plats); > + else > + ret |= dest_plats->m_value_range.set_to_bottom (); > >>> +/* Update value range of formal parameters as described in >>> + ipcp_transformation_summary. */ >>> + >>> +static void >>> +ipcp_update_vr (struct cgraph_node *node) >>> +{ >>> + tree fndecl = node->decl; >>> + tree parm = DECL_ARGUMENTS (fndecl); >>> + tree next_parm = parm; >>> + ipcp_transformation_summary *ts = ipcp_get_transformation_summary (node); >>> + if (!ts || vec_safe_length (ts->m_vr) == 0) >>> + return; >>> + const vec &vr = *ts->m_vr; >>> + unsigned count = vr.length (); >>> + >>> + for (unsigned i = 0; i < count; ++i, parm = next_parm) >>> + { >>> + if (node->clone.combined_args_to_skip >>> + && bitmap_bit_p (node->clone.combined_args_to_skip, i)) >>> + continue; >>> + gcc_checking_assert (parm); >>> + next_parm = DECL_CHAIN (parm); >>> + tree ddef = ssa_default_def (DECL_STRUCT_FUNCTION (node->decl), parm); >>> + >>> + if (!ddef || !is_gimple_reg (parm)) >>> + continue; >>> + >>> + if (cgraph_local_p (node) >> The test of cgraph_local_p seems redundant here. The analysis phase should not determine >> anything if function is reachable non-locally. > > Removed it. > >>> +/* Info about value ranges. */ >>> + >>> +struct GTY(()) ipa_vr >>> +{ >>> + /* The data fields below are valid only if known is true. */ >>> + bool known; >>> + enum value_range_type type; >>> + tree min; >>> + tree max; >> What is the point of representing range as trees rather than wide ints. Can they >> be non-constant integer? > > Changed to wide_int after adding that support. > > LTO Bootstrapped and regression tested on x86_64-linux-gnu with no new > regressions, is this OK? Hi Kugan, Just noticed a small nit - why not reuse ipa_vr in ipa_jump_func instead of adding vr_known and m_vr ? Thanks, Prathamesh > > Thanks > Kugan