From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 90759 invoked by alias); 13 Nov 2016 11:50:12 -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 90745 invoked by uid 89); 13 Nov 2016 11:50:09 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.4 required=5.0 tests=AWL,BAYES_00,KAM_LAZY_DOMAIN_SECURITY,RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy= X-HELO: nikam.ms.mff.cuni.cz Received: from nikam.ms.mff.cuni.cz (HELO nikam.ms.mff.cuni.cz) (195.113.20.16) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sun, 13 Nov 2016 11:49:59 +0000 Received: by nikam.ms.mff.cuni.cz (Postfix, from userid 16202) id 7BC22541BFD; Sun, 13 Nov 2016 12:49:56 +0100 (CET) Date: Sun, 13 Nov 2016 11:50:00 -0000 From: Jan Hubicka To: kugan Cc: Jan Hubicka , "gcc-patches@gcc.gnu.org" Subject: Re: [RFC] Handle unary pass-through jump functions for ipa-vrp Message-ID: <20161113114956.GA63611@kam.mff.cuni.cz> References: <1549ba52-153b-6bf1-28f6-5a1d2a2562fd@linaro.org> <20161027145847.GA49036@kam.mff.cuni.cz> <6209da14-e430-493e-026d-6f1bc19a8265@linaro.org> <20161103173639.rbgqmu4glyzdgqlh@virgil.suse.cz> <052d4b8d-3a67-b2b6-c90a-d4427b0cf5be@linaro.org> <20161108151734.GB46676@kam.mff.cuni.cz> <12996f41-2358-33ad-48ce-172e69ef44ee@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <12996f41-2358-33ad-48ce-172e69ef44ee@linaro.org> User-Agent: Mutt/1.5.21 (2010-09-15) X-SW-Source: 2016-11/txt/msg01234.txt.bz2 > Hi Honza, > > I reverted this patch after it was reported that it resulted in > bootstrap compare failure in some targets. > > I reproduced it and tracked to a mistake in the patch that introduced it. > > That is, in propagate_vr_accross_jump_function, I had: > > if (src_lats->m_value_range.bottom_p ()) > return false; > > which should have been: > > if (src_lats->m_value_range.bottom_p ()) > return dest_lat->set_to_bottom (); Oops, sorry for missing that :) > > > I also fixed update_jump_functions_after_inlining as reported in pr78268. > > I now bootstrapped the patch (lto and normal) on two affected > targets aarch64-none-linux-gnu and powerpc64le-unknown-linux-gnu. I > also tested it on x86_64-linux-gnu with no new regressions. Is this > OK? Yes, thanks for fixing the issue! Can you, please, also send patch to https://gcc.gnu.org/gcc-7/changes.html which mention the new features and command line options? Honza > > > Thanks, > Kugan > > > gcc/testsuite/ChangeLog: > > 2016-11-13 Kugan Vivekanandarajah > > * g++.dg/torture/pr78268.C: New test. > > > gcc/ChangeLog: > > 2016-11-13 Kugan Vivekanandarajah > > * ipa-cp.c (ipa_get_jf_pass_through_result): Skip unary expressions. > (propagate_vr_accross_jump_function): Handle unary expressions. > * ipa-prop.c (ipa_set_jf_unary_pass_through): New. > (load_from_param_1): New. > (load_from_unmodified_param): Factor common part into load_from_param_1. > (load_from_param): New. > (compute_complex_assign_jump_func): Handle unary expressions. > (update_jump_functions_after_inlining): Likewise. > (ipa_write_jump_function): Likewise. > (ipa_read_jump_function): Likewise. > > diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c > index 79e621a..2ec671f 100644 > --- a/gcc/ipa-cp.c > +++ b/gcc/ipa-cp.c > @@ -1219,13 +1219,19 @@ ipa_get_jf_pass_through_result (struct ipa_jump_func *jfunc, tree input) > return NULL_TREE; > > if (TREE_CODE_CLASS (ipa_get_jf_pass_through_operation (jfunc)) > - == tcc_comparison) > - restype = boolean_type_node; > + == tcc_unary) > + res = fold_unary (ipa_get_jf_pass_through_operation (jfunc), > + TREE_TYPE (input), input); > else > - restype = TREE_TYPE (input); > - res = fold_binary (ipa_get_jf_pass_through_operation (jfunc), restype, > - input, ipa_get_jf_pass_through_operand (jfunc)); > - > + { > + if (TREE_CODE_CLASS (ipa_get_jf_pass_through_operation (jfunc)) > + == tcc_comparison) > + restype = boolean_type_node; > + else > + restype = TREE_TYPE (input); > + res = fold_binary (ipa_get_jf_pass_through_operation (jfunc), restype, > + input, ipa_get_jf_pass_through_operand (jfunc)); > + } > if (res && !is_gimple_ip_invariant (res)) > return NULL_TREE; > > @@ -1857,13 +1863,32 @@ 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); > - if (dest_lat->bottom_p ()) > - return false; > 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); > + else if (param_type > + && (TREE_CODE_CLASS (ipa_get_jf_pass_through_operation (jfunc)) > + == tcc_unary)) > + { > + value_range vr; > + memset (&vr, 0, sizeof (vr)); > + tree operand_type = ipa_get_type (caller_info, src_idx); > + enum tree_code operation = ipa_get_jf_pass_through_operation (jfunc); > + > + 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) > + return dest_lat->meet_with (&vr); > + } > } > else if (jfunc->type == IPA_JF_CONST) > { > diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c > index 74fe199..6321fdd 100644 > --- a/gcc/ipa-prop.c > +++ b/gcc/ipa-prop.c > @@ -446,6 +446,18 @@ ipa_set_jf_simple_pass_through (struct ipa_jump_func *jfunc, int formal_id, > jfunc->value.pass_through.agg_preserved = agg_preserved; > } > > +/* Set JFUNC to be an unary pass through jump function. */ > + > +static void > +ipa_set_jf_unary_pass_through (struct ipa_jump_func *jfunc, int formal_id, > + enum tree_code operation) > +{ > + jfunc->type = IPA_JF_PASS_THROUGH; > + jfunc->value.pass_through.operand = NULL_TREE; > + jfunc->value.pass_through.formal_id = formal_id; > + jfunc->value.pass_through.operation = operation; > + jfunc->value.pass_through.agg_preserved = false; > +} > /* Set JFUNC to be an arithmetic pass through jump function. */ > > static void > @@ -849,21 +861,19 @@ parm_preserved_before_stmt_p (struct ipa_func_body_info *fbi, int index, > return !modified; > } > > -/* If STMT is an assignment that loads a value from an parameter declaration, > - return the index of the parameter in ipa_node_params which has not been > - modified. Otherwise return -1. */ > +/* Main worker for load_from_unmodified_param and load_from_param. > + If STMT is an assignment that loads a value from an parameter declaration, > + return the index of the parameter in ipa_node_params. Otherwise return -1. */ > > static int > -load_from_unmodified_param (struct ipa_func_body_info *fbi, > - vec descriptors, > - gimple *stmt) > +load_from_param_1 (struct ipa_func_body_info *fbi, > + vec descriptors, > + gimple *stmt) > { > int index; > tree op1; > > - if (!gimple_assign_single_p (stmt)) > - return -1; > - > + gcc_checking_assert (is_gimple_assign (stmt)); > op1 = gimple_assign_rhs1 (stmt); > if (TREE_CODE (op1) != PARM_DECL) > return -1; > @@ -876,6 +886,40 @@ load_from_unmodified_param (struct ipa_func_body_info *fbi, > return index; > } > > +/* If STMT is an assignment that loads a value from an parameter declaration, > + return the index of the parameter in ipa_node_params which has not been > + modified. Otherwise return -1. */ > + > +static int > +load_from_unmodified_param (struct ipa_func_body_info *fbi, > + vec descriptors, > + gimple *stmt) > +{ > + if (!gimple_assign_single_p (stmt)) > + return -1; > + > + return load_from_param_1 (fbi, descriptors, stmt); > +} > + > +/* If STMT is an assignment that loads a value from an parameter declaration, > + return the index of the parameter in ipa_node_params. Otherwise return -1. */ > + > +static int > +load_from_param (struct ipa_func_body_info *fbi, > + vec descriptors, > + gimple *stmt) > +{ > + if (!is_gimple_assign (stmt)) > + return -1; > + > + enum tree_code rhs_code = gimple_assign_rhs_code (stmt); > + if ((get_gimple_rhs_class (rhs_code) != GIMPLE_SINGLE_RHS) > + && (get_gimple_rhs_class (rhs_code) != GIMPLE_UNARY_RHS)) > + return -1; > + > + return load_from_param_1 (fbi, descriptors, stmt); > +} > + > /* Return true if memory reference REF (which must be a load through parameter > with INDEX) loads data that are known to be unmodified in this function > before reaching statement STMT. */ > @@ -1109,6 +1153,7 @@ compute_complex_assign_jump_func (struct ipa_func_body_info *fbi, > tree op1, tc_ssa, base, ssa; > bool reverse; > int index; > + gimple *stmt2 = stmt; > > op1 = gimple_assign_rhs1 (stmt); > > @@ -1117,13 +1162,16 @@ compute_complex_assign_jump_func (struct ipa_func_body_info *fbi, > if (SSA_NAME_IS_DEFAULT_DEF (op1)) > index = ipa_get_param_decl_index (info, SSA_NAME_VAR (op1)); > else > - index = load_from_unmodified_param (fbi, info->descriptors, > - SSA_NAME_DEF_STMT (op1)); > + { > + index = load_from_param (fbi, info->descriptors, > + SSA_NAME_DEF_STMT (op1)); > + stmt2 = SSA_NAME_DEF_STMT (op1); > + } > tc_ssa = op1; > } > else > { > - index = load_from_unmodified_param (fbi, info->descriptors, stmt); > + index = load_from_param (fbi, info->descriptors, stmt); > tc_ssa = gimple_assign_lhs (stmt); > } > > @@ -1147,6 +1195,11 @@ compute_complex_assign_jump_func (struct ipa_func_body_info *fbi, > bool agg_p = parm_ref_data_pass_through_p (fbi, index, call, tc_ssa); > ipa_set_jf_simple_pass_through (jfunc, index, agg_p); > } > + else if (is_gimple_assign (stmt2) > + && (gimple_expr_code (stmt2) != NOP_EXPR) > + && (TREE_CODE_CLASS (gimple_expr_code (stmt2)) == tcc_unary)) > + ipa_set_jf_unary_pass_through (jfunc, index, > + gimple_assign_rhs_code (stmt2)); > return; > } > > @@ -2518,6 +2571,12 @@ update_jump_functions_after_inlining (struct cgraph_edge *cs, > dst->value.ancestor.agg_preserved &= > src->value.pass_through.agg_preserved; > } > + else if (src->type == IPA_JF_PASS_THROUGH > + && TREE_CODE_CLASS (src->value.pass_through.operation) == tcc_unary) > + { > + dst->value.ancestor.formal_id = src->value.pass_through.formal_id; > + dst->value.ancestor.agg_preserved = false; > + } > else if (src->type == IPA_JF_ANCESTOR) > { > dst->value.ancestor.formal_id = src->value.ancestor.formal_id; > @@ -2583,6 +2642,8 @@ update_jump_functions_after_inlining (struct cgraph_edge *cs, > && ipa_get_jf_pass_through_agg_preserved (src); > ipa_set_jf_simple_pass_through (dst, formal_id, agg_p); > } > + else if (TREE_CODE_CLASS (operation) == tcc_unary) > + ipa_set_jf_unary_pass_through (dst, formal_id, operation); > else > { > tree operand = ipa_get_jf_pass_through_operand (src); > @@ -4666,6 +4727,9 @@ ipa_write_jump_function (struct output_block *ob, > bp_pack_value (&bp, jump_func->value.pass_through.agg_preserved, 1); > streamer_write_bitpack (&bp); > } > + else if (TREE_CODE_CLASS (jump_func->value.pass_through.operation) > + == tcc_unary) > + streamer_write_uhwi (ob, jump_func->value.pass_through.formal_id); > else > { > stream_write_tree (ob, jump_func->value.pass_through.operand, true); > @@ -4745,6 +4809,11 @@ ipa_read_jump_function (struct lto_input_block *ib, > bool agg_preserved = bp_unpack_value (&bp, 1); > ipa_set_jf_simple_pass_through (jump_func, formal_id, agg_preserved); > } > + else if (TREE_CODE_CLASS (operation) == tcc_unary) > + { > + int formal_id = streamer_read_uhwi (ib); > + ipa_set_jf_unary_pass_through (jump_func, formal_id, operation); > + } > else > { > tree operand = stream_read_tree (ib, data_in); > diff --git a/gcc/testsuite/g++.dg/torture/pr78268.C b/gcc/testsuite/g++.dg/torture/pr78268.C > index e69de29..ef4547c 100644 > --- a/gcc/testsuite/g++.dg/torture/pr78268.C > +++ b/gcc/testsuite/g++.dg/torture/pr78268.C > @@ -0,0 +1,25 @@ > +// { dg-do compile } > +typedef enum {} nsresult; > + > +struct A { > + virtual nsresult m_fn1(bool); > +}; > + > +struct B { > + A *operator[](int); > +}; > + > +struct C { > + nsresult m_fn2(bool); > + bool m_fn3(bool); > + B mDataSources; > +}; > +nsresult C::m_fn2(bool p1) > +{ > + m_fn3(!p1); > +} > + > +bool C::m_fn3(bool p1) > +{ > + mDataSources[0]->m_fn1(p1); > +}