From: Jan Hubicka <hubicka@ucw.cz>
To: kugan <kugan.vivekanandarajah@linaro.org>
Cc: Jan Hubicka <hubicka@ucw.cz>,
"gcc-patches@gcc.gnu.org" <gcc-patches@gcc.gnu.org>
Subject: Re: [RFC] Handle unary pass-through jump functions for ipa-vrp
Date: Sun, 13 Nov 2016 11:50:00 -0000 [thread overview]
Message-ID: <20161113114956.GA63611@kam.mff.cuni.cz> (raw)
In-Reply-To: <12996f41-2358-33ad-48ce-172e69ef44ee@linaro.org>
> 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 <kuganv@linaro.org>
>
> * g++.dg/torture/pr78268.C: New test.
>
>
> gcc/ChangeLog:
>
> 2016-11-13 Kugan Vivekanandarajah <kuganv@linaro.org>
>
> * 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<ipa_param_descriptor> descriptors,
> - gimple *stmt)
> +load_from_param_1 (struct ipa_func_body_info *fbi,
> + vec<ipa_param_descriptor> 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<ipa_param_descriptor> 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<ipa_param_descriptor> 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);
> +}
next prev parent reply other threads:[~2016-11-13 11:50 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-10-24 23:18 kugan
2016-10-27 12:11 ` Martin Jambor
2016-10-28 2:58 ` [ipa-vrp] ice in set_value_range kugan
2016-11-03 16:25 ` Martin Jambor
2016-11-08 10:11 ` kugan
2016-11-08 15:17 ` Jan Hubicka
2016-11-09 6:02 ` Andrew Pinski
2016-11-09 8:02 ` kugan
2016-11-10 6:14 ` Andrew Pinski
2016-11-10 6:18 ` kugan
2016-11-10 8:12 ` Andreas Schwab
2016-10-27 14:59 ` [RFC] Handle unary pass-through jump functions for ipa-vrp Jan Hubicka
2016-10-28 3:04 ` kugan
2016-11-03 17:36 ` Martin Jambor
2016-11-08 10:13 ` kugan
2016-11-08 15:17 ` Jan Hubicka
2016-11-13 8:20 ` kugan
2016-11-13 11:50 ` Jan Hubicka [this message]
2016-11-11 15:46 ` Bin.Cheng
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20161113114956.GA63611@kam.mff.cuni.cz \
--to=hubicka@ucw.cz \
--cc=gcc-patches@gcc.gnu.org \
--cc=kugan.vivekanandarajah@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).