From patchwork Fri Jan 6 18:00:34 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Martin Jambor X-Patchwork-Id: 712063 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3twC5j4G4wz9t0t for ; Sat, 7 Jan 2017 05:00:57 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="lPA3F3ew"; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:cc:subject:message-id:references:mime-version :content-type:in-reply-to; q=dns; s=default; b=hyTfaFnhWUZOXEOCL r0EIKnPWlIopG4urYxx8cGcBfYxQABop6MgC7aVYZGyKVqZjp67PKOnhMyXaoIq5 J1znppMIRraF/NnZvgODwsXSvpxsZucl8cd2E/TEcOL3RZYtvLrXWX1YBGA3lBM9 olDbqB89A5ANrQQBc2mVgYoALY= 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:date :from:to:cc:subject:message-id:references:mime-version :content-type:in-reply-to; s=default; bh=8utSLS1vyJWaW5fInXvU2GK Pgds=; b=lPA3F3ewo71KiD87WrgsLo0y6JsrxJa071EMMLp6kGTsczzzXc+HZEY JdEX5J7/GFCL1Hl09/InTMKUtXYtb9LbTOb8sgpErIq1MlNyFtmm/D/jcrUn8MIh M7y3zCbe9RA6h5AuXceyZAoF37/1fDC0vpacyb+BTDqBw/DktR7Y= Received: (qmail 36909 invoked by alias); 6 Jan 2017 18:00:48 -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 36896 invoked by uid 89); 6 Jan 2017 18:00:47 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00, SPF_PASS autolearn=ham version=3.3.2 spammy=sk:stream_, OPERATION, laid, VR_RANGE X-HELO: mx2.suse.de Received: from mx2.suse.de (HELO mx2.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 06 Jan 2017 18:00:37 +0000 Received: from relay1.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id BCE70ABDD; Fri, 6 Jan 2017 18:00:34 +0000 (UTC) Date: Fri, 6 Jan 2017 19:00:34 +0100 From: Martin Jambor To: Richard Biener Cc: kugan , Jan Hubicka , "gcc-patches@gcc.gnu.org" Subject: Re: [PR78365] ICE in determine_value_range, at tree-ssa-loo p-niter.c:413 Message-ID: <20170106180034.fxnoubze6uvechzw@virgil.suse.cz> Mail-Followup-To: Richard Biener , kugan , 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> <20161209105127.mb3xasvrcipt2kqg@virgil.suse.cz> <20161214101523.mxvcgfqdwcbdy737@virgil.suse.cz> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.6.2 (2016-07-01) X-IsSubscribed: yes Hi, On Wed, Dec 14, 2016 at 01:12:11PM +0100, Richard Biener wrote: > On Wed, Dec 14, 2016 at 11:15 AM, Martin Jambor wrote: > > ... > > +/* Emulate effects of unary OPERATION and/or conversion from SRC_TYPE to > > + DST_TYPE on value range in SRC_VR and store it to DST_VR. Return true if > > + the result is a range or an anti-range. */ > > + > > +static bool > > +ipa_vr_operation_and_type_effects (value_range *dst_vr, value_range *src_vr, > > + enum tree_code operation, > > + tree dst_type, tree src_type) > > +{ > > + memset (dst_vr, 0, sizeof (*dst_vr)); > > The memset is not necessary. Apparently it is. Without it, I ended up with corrupted dst->vr_bitmup. I got ICEs when I removed the memset and tracked it down to: (gdb) p dst_vr->equiv->first->next $14 = (bitmap_element *) 0x16 after extract_range_from_unary_expr returns. > > > + extract_range_from_unary_expr (dst_vr, operation, dst_type, src_vr, src_type); > > + if (dst_vr->type == VR_RANGE || dst_vr->type == VR_ANTI_RANGE) > > + return true; > > + else > > + return false; > > +} > > + > > /* Propagate value range across jump function JFUNC that is associated with > > edge CS with param of callee of PARAM_TYPE and update DEST_PLATS > > accordingly. */ > > @@ -1849,7 +1866,6 @@ propagate_vr_accross_jump_function (cgraph_edge *cs, > > struct ipcp_param_lattices *dest_plats, > > tree param_type) > > { > > - struct ipcp_param_lattices *src_lats; > > ipcp_vr_lattice *dest_lat = &dest_plats->m_value_range; > > > > if (dest_lat->bottom_p ()) > > @@ -1862,31 +1878,23 @@ propagate_vr_accross_jump_function (cgraph_edge *cs, > > > > 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); > > + enum tree_code operation = ipa_get_jf_pass_through_operation (jfunc); > > > > - if (ipa_get_jf_pass_through_operation (jfunc) == NOP_EXPR) > > - return dest_lat->meet_with (src_lats->m_value_range); > > - else if (param_type > > - && (TREE_CODE_CLASS (ipa_get_jf_pass_through_operation (jfunc)) > > - == tcc_unary)) > > + if (TREE_CODE_CLASS (operation) == tcc_unary) > > { > > - value_range vr; > > - memset (&vr, 0, sizeof (vr)); > > + struct ipa_node_params *caller_info = IPA_NODE_REF (cs->caller); > > + int src_idx = ipa_get_jf_pass_through_formal_id (jfunc); > > tree operand_type = ipa_get_type (caller_info, src_idx); > > - enum tree_code operation = ipa_get_jf_pass_through_operation (jfunc); > > + struct ipcp_param_lattices *src_lats > > + = ipa_get_parm_lattices (caller_info, src_idx); > > > > if (src_lats->m_value_range.bottom_p ()) > > return dest_lat->set_to_bottom (); > > - > > - extract_range_from_unary_expr (&vr, > > - operation, > > - param_type, > > - &src_lats->m_value_range.m_vr, > > - operand_type); > > - if (vr.type == VR_RANGE > > - || vr.type == VR_ANTI_RANGE) > > + value_range vr; > > + if (ipa_vr_operation_and_type_effects (&vr, > > + &src_lats->m_value_range.m_vr, > > + operation, param_type, > > + operand_type)) > > return dest_lat->meet_with (&vr); > > } > > } > > @@ -1906,8 +1914,11 @@ propagate_vr_accross_jump_function (cgraph_edge *cs, > > } > > } > > > > - if (jfunc->vr_known) > > - return dest_lat->meet_with (&jfunc->m_vr); > > + value_range vr; > > + if (jfunc->vr_known > > + && ipa_vr_operation_and_type_effects (&vr, &jfunc->m_vr, NOP_EXPR, > > + param_type, jfunc->passed_type)) > > but instead of a new jfunc->passed_type you can use TREE_TYPE (jfunc->m_vr.min) > for example. Great, thanks a lot for this suggestion. I have used that and removed the new field addition from the patch and used your suggestion instead. > > I notice that ipa_jump_func is badly laid out: > > struct GTY (()) ipa_jump_func > { > /* Aggregate contants description. See struct ipa_agg_jump_function and its > description. */ > struct ipa_agg_jump_function agg; > > /* Information about zero/non-zero bits. */ > struct ipa_bits bits; > > /* Information about value range. */ > bool vr_known; > value_range m_vr; > > 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 > functions and member_cst holds constant c++ member functions. */ > union jump_func_value > { > struct ipa_constant_data GTY ((tag ("IPA_JF_CONST"))) constant; > struct ipa_pass_through_data GTY ((tag ("IPA_JF_PASS_THROUGH"))) > pass_through; > struct ipa_ancestor_jf_data GTY ((tag ("IPA_JF_ANCESTOR"))) ancestor; > } GTY ((desc ("%1.type"))) value; > }; > > vr_known should be moved to pack with type. I have swapped their position in this patch because it does not affect anything else. > and ipa_bits::known should be moved out of it as well. ...and will try to come up with a patch doing this soon. > I also think we do not use the equiv member of m_vr and thus that is > a waste. > > Not sure if memory use of this struct is an issue. We create it for each and every actual argument in every call statement we track with IPA-CP et al, so it is visible in mem-stats of LTO WPA of big programs. I have bootstrapped, lto-bootstrapped and tested the following on x86_64-linux. OK for trunk? Thanks, Martin 2017-01-04 Martin Jambor PR ipa/78365 PR ipa/78599 * ipa-prop.h (ipa_jump_func): Swap positions of vr_known and m_vr. * ipa-cp.c (ipa_vr_operation_and_type_effects): New function. (propagate_vr_accross_jump_function): Use the above function for all value range computations for pass-through jump functions and type converasion from explicit value range values. (ipcp_propagate_stage): Do not attempt to deduce types of formal parameters from TYPE_ARG_TYPES. * ipa-prop.c (ipa_write_jump_function): Remove trailing whitespace. (ipa_write_node_info): Stream type of the actual argument. (ipa_read_node_info): Likewise. Also remove trailing whitespace. testsuite/ * gcc.dg/torture/pr78365.c: New test. --- gcc/ipa-cp.c | 71 +++++++++++++++++----------------- gcc/ipa-prop.c | 14 +++++-- gcc/ipa-prop.h | 5 ++- gcc/testsuite/gcc.dg/torture/pr78365.c | 21 ++++++++++ 4 files changed, 69 insertions(+), 42 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/torture/pr78365.c diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c index 82bf35084b6..9cc903769e8 100644 --- a/gcc/ipa-cp.c +++ b/gcc/ipa-cp.c @@ -1837,6 +1837,23 @@ propagate_bits_across_jump_function (cgraph_edge *cs, int idx, return dest_lattice->set_to_bottom (); } +/* Emulate effects of unary OPERATION and/or conversion from SRC_TYPE to + DST_TYPE on value range in SRC_VR and store it to DST_VR. Return true if + the result is a range or an anti-range. */ + +static bool +ipa_vr_operation_and_type_effects (value_range *dst_vr, value_range *src_vr, + enum tree_code operation, + tree dst_type, tree src_type) +{ + memset (dst_vr, 0, sizeof (*dst_vr)); + extract_range_from_unary_expr (dst_vr, operation, dst_type, src_vr, src_type); + if (dst_vr->type == VR_RANGE || dst_vr->type == VR_ANTI_RANGE) + return true; + else + return false; +} + /* Propagate value range across jump function JFUNC that is associated with edge CS with param of callee of PARAM_TYPE and update DEST_PLATS accordingly. */ @@ -1846,7 +1863,6 @@ propagate_vr_across_jump_function (cgraph_edge *cs, ipa_jump_func *jfunc, struct ipcp_param_lattices *dest_plats, tree param_type) { - struct ipcp_param_lattices *src_lats; ipcp_vr_lattice *dest_lat = &dest_plats->m_value_range; if (dest_lat->bottom_p ()) @@ -1859,31 +1875,23 @@ propagate_vr_across_jump_function (cgraph_edge *cs, ipa_jump_func *jfunc, 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); + enum tree_code operation = ipa_get_jf_pass_through_operation (jfunc); - if (ipa_get_jf_pass_through_operation (jfunc) == NOP_EXPR) - return dest_lat->meet_with (src_lats->m_value_range); - else if (param_type - && (TREE_CODE_CLASS (ipa_get_jf_pass_through_operation (jfunc)) - == tcc_unary)) + if (TREE_CODE_CLASS (operation) == tcc_unary) { - value_range vr; - memset (&vr, 0, sizeof (vr)); + struct ipa_node_params *caller_info = IPA_NODE_REF (cs->caller); + int src_idx = ipa_get_jf_pass_through_formal_id (jfunc); tree operand_type = ipa_get_type (caller_info, src_idx); - enum tree_code operation = ipa_get_jf_pass_through_operation (jfunc); + struct ipcp_param_lattices *src_lats + = ipa_get_parm_lattices (caller_info, src_idx); if (src_lats->m_value_range.bottom_p ()) return dest_lat->set_to_bottom (); - - extract_range_from_unary_expr (&vr, - operation, - param_type, - &src_lats->m_value_range.m_vr, - operand_type); - if (vr.type == VR_RANGE - || vr.type == VR_ANTI_RANGE) + value_range vr; + if (ipa_vr_operation_and_type_effects (&vr, + &src_lats->m_value_range.m_vr, + operation, param_type, + operand_type)) return dest_lat->meet_with (&vr); } } @@ -1903,8 +1911,12 @@ propagate_vr_across_jump_function (cgraph_edge *cs, ipa_jump_func *jfunc, } } - if (jfunc->vr_known) - return dest_lat->meet_with (&jfunc->m_vr); + value_range vr; + if (jfunc->vr_known + && ipa_vr_operation_and_type_effects (&vr, &jfunc->m_vr, NOP_EXPR, + param_type, + TREE_TYPE (jfunc->m_vr.min))) + return dest_lat->meet_with (&vr); else return dest_lat->set_to_bottom (); } @@ -2244,7 +2256,7 @@ propagate_constants_across_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); + tree param_type = ipa_get_type (callee_info, i); dest_plats = ipa_get_parm_lattices (callee_info, i); if (availability == AVAIL_INTERPOSABLE) @@ -3230,19 +3242,6 @@ ipcp_propagate_stage (struct ipa_topo_info *topo) { struct ipa_node_params *info = IPA_NODE_REF (node); - /* In LTO we do not have PARM_DECLs but we would still like to be able to - look at types of parameters. */ - if (in_lto_p) - { - tree t = TYPE_ARG_TYPES (TREE_TYPE (node->decl)); - for (int k = 0; k < ipa_get_param_count (info) && t; k++) - { - gcc_assert (t != void_list_node); - info->descriptors[k].decl_or_type = TREE_VALUE (t); - t = t ? TREE_CHAIN (t) : NULL; - } - } - determine_versionability (node, info); if (node->has_gimple_body_p ()) { diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c index 1afa8fc7a05..1f68f736e46 100644 --- a/gcc/ipa-prop.c +++ b/gcc/ipa-prop.c @@ -4775,7 +4775,7 @@ ipa_write_jump_function (struct output_block *ob, { streamer_write_widest_int (ob, jump_func->bits.value); streamer_write_widest_int (ob, jump_func->bits.mask); - } + } bp_pack_value (&bp, jump_func->vr_known, 1); streamer_write_bitpack (&bp); if (jump_func->vr_known) @@ -4973,7 +4973,10 @@ ipa_write_node_info (struct output_block *ob, struct cgraph_node *node) bp_pack_value (&bp, ipa_is_param_used (info, j), 1); streamer_write_bitpack (&bp); for (j = 0; j < ipa_get_param_count (info); j++) - streamer_write_hwi (ob, ipa_get_controlled_uses (info, j)); + { + streamer_write_hwi (ob, ipa_get_controlled_uses (info, j)); + stream_write_tree (ob, ipa_get_type (info, j), true); + } for (e = node->callees; e; e = e->next_callee) { struct ipa_edge_args *args = IPA_EDGE_REF (e); @@ -5020,7 +5023,7 @@ ipa_read_node_info (struct lto_input_block *ib, struct cgraph_node *node, for (k = 0; k < ipa_get_param_count (info); k++) info->descriptors[k].move_cost = streamer_read_uhwi (ib); - + bp = streamer_read_bitpack (ib); if (ipa_get_param_count (info) != 0) info->analysis_done = true; @@ -5028,7 +5031,10 @@ ipa_read_node_info (struct lto_input_block *ib, struct cgraph_node *node, for (k = 0; k < ipa_get_param_count (info); k++) ipa_set_param_used (info, k, bp_unpack_value (&bp, 1)); for (k = 0; k < ipa_get_param_count (info); k++) - ipa_set_controlled_uses (info, k, streamer_read_hwi (ib)); + { + ipa_set_controlled_uses (info, k, streamer_read_hwi (ib)); + info->descriptors[k].decl_or_type = stream_read_tree (ib, data_in); + } for (e = node->callees; e; e = e->next_callee) { struct ipa_edge_args *args = IPA_EDGE_REF (e); diff --git a/gcc/ipa-prop.h b/gcc/ipa-prop.h index 341d9db6c63..c9a69ab1f53 100644 --- a/gcc/ipa-prop.h +++ b/gcc/ipa-prop.h @@ -178,9 +178,10 @@ struct GTY (()) ipa_jump_func /* Information about zero/non-zero bits. */ struct ipa_bits bits; - /* Information about value range. */ - bool vr_known; + /* Information about value range, containing valid data only when vr_known is + true. */ value_range m_vr; + bool vr_known; enum jump_func_type type; /* Represents a value of a jump function. pass_through is used only in jump diff --git a/gcc/testsuite/gcc.dg/torture/pr78365.c b/gcc/testsuite/gcc.dg/torture/pr78365.c new file mode 100644 index 00000000000..5180a0171ae --- /dev/null +++ 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); +}