Hi,
On Mon, Nov 04 2019, Feng Xue OS wrote:
> Hi, Honza & Martin,
>
> This is a new patch merged with the newest IPA changes. Would you
> please take a look at the patch? Together with the other patch on
> recursive function versioning, we can find more than 30% performance
> boost on exchange2 in spec2017. So, it will be good if two patches can
> enter the gcc 10 release, though time schedule seems to be somewhat
> urgent. Thanks.
Sorry that it took so long. Next time, please consider making the
review a bit easier by writing a ChangeLog (yes, I usually read them and
you'll have to write one anyway).
The good news is that I found no real objection to the patch, I'd only
like to request a few minor changes. Thanks a lot for working on this,
this extension of aggregate jump functions is really appreciated. But
it also shows that we really need to beat a bit more sense into the
various involved data structures now since they grew a bit out of hand.
Nevertheless, that is something for next stage1 and should not block
this.
Anyway, comments inline, there is really just a few of them. Honza,
please have a final look but overall I like the patch.
>
> Feng
>
> From 2dfa5e8b5a828ad8d46c2af5f66ee97fb04ebc16 Mon Sep 17 00:00:00 2001
> From: Feng Xue <f...@os.amperecomputing.com>
> Date: Thu, 15 Aug 2019 15:47:14 +0800
> Subject: [PATCH 1/2] temp
>
> ---
> gcc/ipa-cp.c | 498 ++++++++++++++++------
> gcc/ipa-fnsummary.c | 48 +--
> gcc/ipa-fnsummary.h | 8 +-
> gcc/ipa-inline-analysis.c | 6 +-
> gcc/ipa-prop.c | 569 +++++++++++++++++++++----
> gcc/ipa-prop.h | 182 ++++++--
> gcc/testsuite/gcc.dg/ipa/ipcp-agg-10.c | 8 +-
> gcc/testsuite/gcc.dg/ipa/ipcp-agg-11.c | 77 ++++
> 8 files changed, 1105 insertions(+), 291 deletions(-)
> create mode 100644 gcc/testsuite/gcc.dg/ipa/ipcp-agg-11.c
>
> diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c
> index 8a5f8d362f6..e100c5f3426 100644
> --- a/gcc/ipa-cp.c
> +++ b/gcc/ipa-cp.c
> @@ -1459,6 +1473,146 @@ ipa_context_from_jfunc (ipa_node_params *info,
> cgraph_edge *cs, int csidx,
> return ctx;
> }
>
> +/* See if NODE is a clone with a known aggregate value at a given OFFSET of a
> + parameter with the given INDEX. */
> +
> +static tree
> +get_clone_agg_value (struct cgraph_node *node, HOST_WIDE_INT offset,
> + int index)
> +{
> + struct ipa_agg_replacement_value *aggval;
> +
> + aggval = ipa_get_agg_replacements_for_node (node);
> + while (aggval)
> + {
> + if (aggval->offset == offset
> + && aggval->index == index)
> + return aggval->value;
> + aggval = aggval->next;
> + }
> + return NULL_TREE;
> +}
> +
> +/* Determine whether ITEM, jump function for an aggregate part, evaluates to
> a
> + single known constant value and if so, return it. Otherwise return NULL.
> + NODE and INFO describes the caller node or the one it is inlined to, and
> + its related info. */
> +
> +static tree
> +ipa_agg_value_from_node (class ipa_node_params *info,
> + struct cgraph_node *node,
> + struct ipa_agg_jf_item *item)
> +{
> + tree value = NULL_TREE;
> + int src_idx;
> +
> + if (item->offset < 0 || item->jftype == IPA_JF_UNKNOWN)
> + return NULL_TREE;
> +
> + if (item->jftype == IPA_JF_CONST)
> + return item->value.constant;
> +
> + gcc_checking_assert (item->jftype == IPA_JF_PASS_THROUGH
> + || item->jftype == IPA_JF_LOAD_AGG);
> +
> + src_idx = item->value.pass_through.formal_id;
> +
> + if (info->ipcp_orig_node)
> + {
> + if (item->jftype == IPA_JF_PASS_THROUGH)
> + value = info->known_csts[src_idx];
> + else
> + value = get_clone_agg_value (node, item->value.load_agg.offset,
> + src_idx);
> + }
> + else if (info->lattices)
> + {
> + class ipcp_param_lattices *src_plats
> + = ipa_get_parm_lattices (info, src_idx);
Wrong indentation for GNU coding standard.
> +
> + if (item->jftype == IPA_JF_PASS_THROUGH)
> + {
> + struct ipcp_lattice<tree> *lat = &src_plats->itself;
> +
> + if (!lat->is_single_const ())
> + return NULL_TREE;
> +
> + value = lat->values->value;
> + }
> + else if (src_plats->aggs
> + && !src_plats->aggs_bottom
> + && !src_plats->aggs_contain_variable
> + && src_plats->aggs_by_ref == item->value.load_agg.by_ref)
> + {
> + struct ipcp_agg_lattice *aglat;
> +
> + for (aglat = src_plats->aggs; aglat; aglat = aglat->next)
> + {
> + if (aglat->offset > item->value.load_agg.offset)
> + break;
> +
> + if (aglat->offset == item->value.load_agg.offset)
> + {
> + if (aglat->is_single_const ())
> + value = aglat->values->value;
> + break;
> + }
> + }
> + }
> + }
> +
> + if (!value)
> + return NULL_TREE;
> +
> + if (item->jftype == IPA_JF_LOAD_AGG)
> + {
> + tree load_type = item->value.load_agg.type;
> + tree value_type = TREE_TYPE (value);
> +
> + /* Ensure value type is compatible with load type. */
> + if (!useless_type_conversion_p (load_type, value_type))
> + return NULL_TREE;
> + }
> +
> + return ipa_get_jf_arith_result (item->value.pass_through.operation,
> + value,
> + item->value.pass_through.operand,
> + item->type);
> +}
> +
> +/* Determine whether AGG_JFUNC evaluates to a set of known constant value for
> + an aggregate and if so, return it. Otherwise return an empty set. NODE
> + and INFO describes the caller node or the one it is inlined to, and its
> + related info. */
> +
> +struct ipa_agg_value_set
> +ipa_agg_value_set_from_jfunc (class ipa_node_params *info, cgraph_node *node,
> + struct ipa_agg_jump_function *agg_jfunc)
> +{
> + struct ipa_agg_value_set agg;
> + struct ipa_agg_jf_item *item;
> + int i;
> +
> + agg.items = vNULL;
> + agg.by_ref = agg_jfunc->by_ref;
> +
> + FOR_EACH_VEC_SAFE_ELT (agg_jfunc->items, i, item)
> + {
> + tree value = ipa_agg_value_from_node (info, node, item);
> +
> + if (value)
> + {
> + struct ipa_agg_value value_item;
> +
> + value_item.offset = item->offset;
> + value_item.value = value;
> +
> + agg.items.safe_push (value_item);
> + }
> + }
> + return agg;
> +}
> +
> /* If checking is enabled, verify that no lattice is in the TOP state, i.e.
> not
> bottom, not containing a variable component and without any known value at
> the same time. */
...
> diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c
> index 83cf4d1c7ba..194c31ff54a 100644
> --- a/gcc/ipa-prop.c
> +++ b/gcc/ipa-prop.c
> @@ -1454,25 +1542,31 @@ get_ssa_def_if_simple_copy (tree rhs)
> rhs = gimple_assign_rhs1 (def_stmt);
> else
> break;
> + *rhs_stmt = def_stmt;
> }
> return rhs;
> }
>
> -/* Simple linked list, describing known contents of an aggregate before
> - call. */
> +/* Simple linked list, describing contents of an aggregate before call. */
>
> struct ipa_known_agg_contents_list
> {
> /* Offset and size of the described part of the aggregate. */
> HOST_WIDE_INT offset, size;
> - /* Known constant value or NULL if the contents is known to be unknown. */
> - tree constant;
> +
> + /* Type of the described part of the aggregate. */
> + tree type;
> +
> + /* Known constant value or jump function data describing contents. */
> + struct ipa_load_agg_data value;
I wonder whether it would be cleaner to repeat the fields of
ipa_load_agg_dat here. But I don't insist.
> +
> /* Pointer to the next structure in the list. */
> struct ipa_known_agg_contents_list *next;
> };
>
> -/* Add a known content item into a linked list of ipa_known_agg_contents_list
> - structure, in which all elements are sorted ascendingly by offset. */
> +/* Add an aggregate content item into a linked list of
> + ipa_known_agg_contents_list structure, in which all elements
> + are sorted ascendingly by offset. */
>
> static inline void
> add_to_agg_contents_list (struct ipa_known_agg_contents_list **plist,
> @@ -1492,7 +1586,7 @@ add_to_agg_contents_list (struct
> ipa_known_agg_contents_list **plist,
> *plist = item;
> }
>
> -/* Check whether a given known content is clobbered by certain element in
> +/* Check whether a given aggregate content is clobbered by certain element in
> a linked list of ipa_known_agg_contents_list. */
>
> static inline bool
> @@ -1512,27 +1606,189 @@ clobber_by_agg_contents_list_p (struct
> ipa_known_agg_contents_list *list,
> }
>
> /* Build aggregate jump function from LIST, assuming there are exactly
> - CONST_COUNT constant entries there and that offset of the passed argument
> + VALUE_COUNT entries there and that offset of the passed argument
> is ARG_OFFSET and store it into JFUNC. */
>
> static void
> build_agg_jump_func_from_list (struct ipa_known_agg_contents_list *list,
> - int const_count, HOST_WIDE_INT arg_offset,
> + int value_count, HOST_WIDE_INT arg_offset,
> struct ipa_jump_func *jfunc)
> {
> - vec_alloc (jfunc->agg.items, const_count);
> - while (list)
> + vec_alloc (jfunc->agg.items, value_count);
> + for (; list; list = list->next)
> + {
> + struct ipa_agg_jf_item item;
> + tree operand = list->value.pass_through.operand;
> +
> + if (list->value.pass_through.formal_id >= 0)
> + {
> + /* Content value is derived from some formal paramerter. */
> + if (list->value.offset >= 0)
> + item.jftype = IPA_JF_LOAD_AGG;
> + else
> + item.jftype = IPA_JF_PASS_THROUGH;
> +
> + item.value.load_agg = list->value;
> + if (operand)
> + item.value.pass_through.operand
> + = unshare_expr_without_location (operand);
Wrong indentation for GNU coding standard.
> + }
> + else if (operand)
> + {
> + /* Content value is known constant. */
> + item.jftype = IPA_JF_CONST;
> + item.value.constant = unshare_expr_without_location (operand);
> + }
> + else
> + continue;
> +
> + item.type = list->type;
> + gcc_assert (tree_to_shwi (TYPE_SIZE (list->type)) == list->size);
> +
> + item.offset = list->offset - arg_offset;
> + gcc_assert ((item.offset % BITS_PER_UNIT) == 0);
> +
> + jfunc->agg.items->quick_push (item);
> + }
> +}
> +
> +/* Given an assignment statement STMT, try to collect information into
> + AGG_VALUE that will be used to construct jump function for RHS of the
> + assignment, from which content value of an aggregate part comes.
> +
> + Besides constant and simple pass-through jump functions, also try to
> + identify whether it matches the following pattern that can be described by
> + a load-value-from-aggregate jump function, which is a derivative of simple
> + pass-through jump function.
> +
> + foo (int *p)
> + {
> + ...
> +
> + *(q_5 + 4) = *(p_3(D) + 28) op 1;
> + bar (q_5);
> + }
> +
> + Since load-value-from-aggregate jump function data structure is
> informative
> + enough to describe constant and simple pass-through jump function, here we
> + do not need a jump function type, merely use FORMAL_ID and OPERAND in
> + IPA_LOAD_AGG_DATA to distinguish different jump functions. */
This last comment is difficult to understand to the point when IMHO one
has to read the code anyway. Perhaps you could just list which special
values imply which final jump function type? And perhaps that list
should go to the comment describing ipa_known_agg_contents_list.
> +
> +static void
> +compute_assign_agg_jump_func (struct ipa_func_body_info *fbi,
> + struct ipa_load_agg_data *agg_value,
My preference would be for this function to receive a pointer to the
whole ipa_known_agg_contents_list as a parameter instead of agg_value,
and to be called something like extract_agg_content_or_origin or
something that would not suggest it creates a real jump function.
Please at least consider changing the name.
> + gimple *stmt)
> +{
> + tree lhs = gimple_assign_lhs (stmt);
> + tree rhs1 = gimple_assign_rhs1 (stmt);
> + enum tree_code code;
> + int index = -1;
> +
> + /* Initialize jump function data for the aggregate part. */
> + memset (agg_value, 0, sizeof (*agg_value));
> + agg_value->pass_through.operation = NOP_EXPR;
> + agg_value->pass_through.formal_id = -1;
> + agg_value->offset = -1;
> +
> + if (AGGREGATE_TYPE_P (TREE_TYPE (lhs)) /* TODO: Support aggregate type.
> */
> + || TREE_THIS_VOLATILE (lhs)
> + || TREE_CODE (lhs) == BIT_FIELD_REF
> + || contains_bitfld_component_ref_p (lhs))
> + return;
> +
> + /* Skip SSA copies. */
> + while (gimple_assign_rhs_class (stmt) == GIMPLE_SINGLE_RHS)
> + {
> + if (TREE_CODE (rhs1) != SSA_NAME || SSA_NAME_IS_DEFAULT_DEF (rhs1))
> + break;
> +
> + if (!is_gimple_assign (stmt = SSA_NAME_DEF_STMT (rhs1)))
Please put the assignment into a separate statement.
> + return;
> +
> + rhs1 = gimple_assign_rhs1 (stmt);
> + }
> +
> + code = gimple_assign_rhs_code (stmt);
> + switch (gimple_assign_rhs_class (stmt))
> {
> - if (list->constant)
> + case GIMPLE_SINGLE_RHS:
> + if (is_gimple_ip_invariant (rhs1))
> {
> - struct ipa_agg_jf_item item;
> - item.offset = list->offset - arg_offset;
> - gcc_assert ((item.offset % BITS_PER_UNIT) == 0);
> - item.value = unshare_expr_without_location (list->constant);
> - jfunc->agg.items->quick_push (item);
> + agg_value->pass_through.operand = rhs1;
> + return;
> }
> - list = list->next;
> + code = NOP_EXPR;
> + break;
> +
> + case GIMPLE_UNARY_RHS:
> + /* NOTE: A GIMPLE_UNARY_RHS operation might not be tcc_unary
> + (truth_not_expr is example), GIMPLE_BINARY_RHS does not imply
> + tcc_binary, this subtleness is somewhat misleading.
> +
> + Since tcc_unary is widely used in IPA-CP code to check an operation
> + with one operand, here we only allow tc_unary operation to avoid
> + possible problem. Then we can use (opclass == tc_unary) or not to
> + distinguish unary and binary. */
> + if (TREE_CODE_CLASS (code) != tcc_unary || CONVERT_EXPR_CODE_P (code))
> + return;
> +
> + rhs1 = get_ssa_def_if_simple_copy (rhs1, &stmt);
> + break;
> +
> + case GIMPLE_BINARY_RHS:
> + {
> + gimple *rhs1_stmt = stmt;
> + gimple *rhs2_stmt = stmt;
> + tree rhs2 = gimple_assign_rhs2 (stmt);
> +
> + rhs1 = get_ssa_def_if_simple_copy (rhs1, &rhs1_stmt);
> + rhs2 = get_ssa_def_if_simple_copy (rhs2, &rhs2_stmt);
> +
> + if (is_gimple_ip_invariant (rhs2))
> + {
> + agg_value->pass_through.operand = rhs2;
> + stmt = rhs1_stmt;
> + }
> + else if (is_gimple_ip_invariant (rhs1))
> + {
> + if (TREE_CODE_CLASS (code) == tcc_comparison)
> + code = swap_tree_comparison (code);
> + else if (!commutative_tree_code (code))
> + return;
> +
> + agg_value->pass_through.operand = rhs1;
> + stmt = rhs2_stmt;
> + rhs1 = rhs2;
> + }
> + else
> + return;
> +
> + if (TREE_CODE_CLASS (code) != tcc_comparison
> + && !useless_type_conversion_p (TREE_TYPE (lhs), TREE_TYPE (rhs1)))
> + return;
> + }
> + break;
> +
> + default:
> + return;
> + }
> +
> + if (TREE_CODE (rhs1) != SSA_NAME)
> + index = load_from_unmodified_param_or_agg (fbi, fbi->info, stmt,
> + &agg_value->offset,
> + &agg_value->by_ref);
> + else if (SSA_NAME_IS_DEFAULT_DEF (rhs1))
> + index = ipa_get_param_decl_index (fbi->info, SSA_NAME_VAR (rhs1));
> +
> + if (index >= 0)
> + {
> + if (agg_value->offset >= 0)
> + agg_value->type = TREE_TYPE (rhs1);
> + agg_value->pass_through.formal_id = index;
> + agg_value->pass_through.operation = code;
> }
> + else
> + agg_value->pass_through.operand = NULL_TREE;
> }
>
> /* If STMT is a memory store to the object whose address is BASE, extract
> @@ -1542,26 +1798,19 @@ build_agg_jump_func_from_list (struct
> ipa_known_agg_contents_list *list,
> is expected to be in form of MEM_REF expression. */
>
> static bool
> -extract_mem_content (gimple *stmt, tree base, bool check_ref,
> +extract_mem_content (struct ipa_func_body_info *fbi,
> + gimple *stmt, tree base, bool check_ref,
> struct ipa_known_agg_contents_list *content)
> {
> HOST_WIDE_INT lhs_offset, lhs_size;
> - tree lhs, rhs, lhs_base;
> bool reverse;
>
> - if (!gimple_assign_single_p (stmt))
> - return false;
> -
> - lhs = gimple_assign_lhs (stmt);
> - rhs = gimple_assign_rhs1 (stmt);
> -
> - if (!is_gimple_reg_type (TREE_TYPE (rhs))
> - || TREE_CODE (lhs) == BIT_FIELD_REF
> - || contains_bitfld_component_ref_p (lhs))
> + if (!is_gimple_assign (stmt))
> return false;
>
> - lhs_base = get_ref_base_and_extent_hwi (lhs, &lhs_offset,
> - &lhs_size, &reverse);
> + tree lhs = gimple_assign_lhs (stmt);
> + tree lhs_base = get_ref_base_and_extent_hwi (lhs, &lhs_offset, &lhs_size,
> + &reverse);
> if (!lhs_base)
> return false;
>
> @@ -1575,32 +1824,31 @@ extract_mem_content (gimple *stmt, tree base, bool
> check_ref,
> else if (lhs_base != base)
> return false;
>
> - rhs = get_ssa_def_if_simple_copy (rhs);
> -
> - content->size = lhs_size;
> content->offset = lhs_offset;
> - content->constant = is_gimple_ip_invariant (rhs) ? rhs : NULL_TREE;
> + content->size = lhs_size;
> + content->type = TREE_TYPE (lhs);
> content->next = NULL;
>
> + compute_assign_agg_jump_func (fbi, &content->value, stmt);
> return true;
> }
>
> /* Traverse statements from CALL backwards, scanning whether an aggregate
> given
> - in ARG is filled in with constant values. ARG can either be an aggregate
> - expression or a pointer to an aggregate. ARG_TYPE is the type of the
> - aggregate. JFUNC is the jump function into which the constants are
> - subsequently stored. AA_WALK_BUDGET_P points to limit on number of
> - statements we allow get_continuation_for_phi to examine. */
> + in ARG is filled in constant or value that is derived from caller's formal
Minor nit, please use plural "constants or values that are..."
> + parameter in the way described by some kind of jump function. FBI is the
> + context of the caller function for interprocedural analysis. ARG can
> either
> + be an aggregate expression or a pointer to an aggregate. ARG_TYPE is the
> + type of the aggregate. JFUNC is the jump function for the aggregate. */
>
> static void
> -determine_known_aggregate_parts (gcall *call, tree arg,
> +determine_known_aggregate_parts (struct ipa_func_body_info *fbi,
> + gcall *call, tree arg,
> tree arg_type,
> - struct ipa_jump_func *jfunc,
> - unsigned *aa_walk_budget_p)
> + struct ipa_jump_func *jfunc)
> {
> struct ipa_known_agg_contents_list *list = NULL, *all_list = NULL;
> bitmap visited = NULL;
> - int item_count = 0, const_count = 0;
> + int item_count = 0, value_count = 0;
> int ipa_max_agg_items = PARAM_VALUE (PARAM_IPA_MAX_AGG_ITEMS);
> HOST_WIDE_INT arg_offset, arg_size;
> tree arg_base;
> @@ -1679,7 +1927,7 @@ determine_known_aggregate_parts (gcall *call, tree arg,
> if (gimple_code (stmt) == GIMPLE_PHI)
> {
> dom_vuse = get_continuation_for_phi (stmt, &r, true,
> - *aa_walk_budget_p,
> + fbi->aa_walk_budget,
> &visited, false, NULL, NULL);
> continue;
> }
> @@ -1689,12 +1937,13 @@ determine_known_aggregate_parts (gcall *call, tree
> arg,
> struct ipa_known_agg_contents_list *content
> = XALLOCA (struct ipa_known_agg_contents_list);
>
> - if (!extract_mem_content (stmt, arg_base, check_ref, content))
> + if (!extract_mem_content (fbi, stmt, arg_base, check_ref, content))
> break;
>
> /* Now we get a dominating virtual operand, and need to check
> whether its value is clobbered any other dominating one. */
> - if (content->constant
> + if ((content->value.pass_through.formal_id >= 0
> + || content->value.pass_through.operand)
> && !clobber_by_agg_contents_list_p (all_list, content))
> {
> struct ipa_known_agg_contents_list *copy
> @@ -1704,7 +1953,7 @@ determine_known_aggregate_parts (gcall *call, tree arg,
> operands, whose definitions can finally reach the call. */
> add_to_agg_contents_list (&list, (*copy = *content, copy));
>
> - if (++const_count == ipa_max_agg_items)
> + if (++value_count == ipa_max_agg_items)
> break;
> }
>
> @@ -1722,12 +1971,12 @@ determine_known_aggregate_parts (gcall *call, tree
> arg,
>
> /* Third stage just goes over the list and creates an appropriate vector of
> ipa_agg_jf_item structures out of it, of course only if there are
> - any known constants to begin with. */
> + any meaningful items to begin with. */
>
> - if (const_count)
> + if (value_count)
> {
> jfunc->agg.by_ref = by_ref;
> - build_agg_jump_func_from_list (list, const_count, arg_offset, jfunc);
> + build_agg_jump_func_from_list (list, value_count, arg_offset, jfunc);
> }
> }
>
> @@ -2019,8 +2268,7 @@ ipa_compute_jump_functions_for_edge (struct
> ipa_func_body_info *fbi,
> || !ipa_get_jf_ancestor_agg_preserved (jfunc))
> && (AGGREGATE_TYPE_P (TREE_TYPE (arg))
> || POINTER_TYPE_P (param_type)))
> - determine_known_aggregate_parts (call, arg, param_type, jfunc,
> - &fbi->aa_walk_budget);
> + determine_known_aggregate_parts (fbi, call, arg, param_type, jfunc);
> }
> if (!useful_context)
> vec_free (args->polymorphic_call_contexts);
> @@ -2679,6 +2927,72 @@ update_jump_functions_after_inlining (struct
> cgraph_edge *cs,
> class ipa_polymorphic_call_context *dst_ctx
> = ipa_get_ith_polymorhic_call_context (args, i);
>
> + if (dst->agg.items)
> + {
> + struct ipa_agg_jf_item *item;
> + int j;
> +
> + FOR_EACH_VEC_ELT (*dst->agg.items, j, item)
> + {
> + int dst_fid;
> + struct ipa_jump_func *src;
> +
> + if (item->jftype != IPA_JF_PASS_THROUGH
> + && item->jftype != IPA_JF_LOAD_AGG)
> + continue;
> +
> + dst_fid = item->value.pass_through.formal_id;
> + if (dst_fid >= ipa_get_cs_argument_count (top))
> + {
> + item->jftype = IPA_JF_UNKNOWN;
> + continue;
> + }
> +
> + item->value.pass_through.formal_id = -1;
> + src = ipa_get_ith_jump_func (top, dst_fid);
> + if (src->type == IPA_JF_CONST)
> + {
> + if (item->jftype == IPA_JF_PASS_THROUGH
> + && item->value.pass_through.operation == NOP_EXPR)
> + {
> + item->jftype = IPA_JF_CONST;
> + item->value.constant = src->value.constant.value;
> + continue;
> + }
> + }
> + else if (src->type == IPA_JF_PASS_THROUGH
> + && src->value.pass_through.operation == NOP_EXPR)
> + {
> + if (item->jftype == IPA_JF_PASS_THROUGH
> + || !item->value.load_agg.by_ref
> + || src->value.pass_through.agg_preserved)
> + item->value.pass_through.formal_id
> + = src->value.pass_through.formal_id;
> + }
> + else if (src->type == IPA_JF_ANCESTOR)
> + {
> + if (item->jftype == IPA_JF_PASS_THROUGH)
> + {
> + if (!src->value.ancestor.offset)
> + item->value.pass_through.formal_id
> + = src->value.ancestor.formal_id;
> + }
> + else if (src->value.ancestor.agg_preserved)
> + {
> + gcc_checking_assert (item->value.load_agg.by_ref);
> +
> + item->value.pass_through.formal_id
> + = src->value.ancestor.formal_id;
> + item->value.load_agg.offset
> + += src->value.ancestor.offset;
> + }
> + }
> +
> + if (item->value.pass_through.formal_id < 0)
> + item->jftype = IPA_JF_UNKNOWN;
> + }
> + }
> +
> if (dst->type == IPA_JF_ANCESTOR)
> {
> struct ipa_jump_func *src;
> @@ -2718,8 +3032,11 @@ update_jump_functions_after_inlining (struct
> cgraph_edge *cs,
> }
> }
>
> - if (src->agg.items
> - && (dst->value.ancestor.agg_preserved || !src->agg.by_ref))
> + /* Parameter and argument in ancestor jump function must be pointer
> + type, which means access to aggregate must be by-reference. */
> + gcc_checking_assert (!src->agg.items || src->agg.by_ref);
I am slightly afraid that some type mismatches in between the call
statement fntype and callee type which are possible with LTO (and
horribly bad user input) might trigger this. Please make this a
non-checking assert so that we find out if that is indeed true.
> +
> + if (src->agg.items && dst->value.ancestor.agg_preserved)
> {
> struct ipa_agg_jf_item *item;
> int j;
...
> diff --git a/gcc/ipa-prop.h b/gcc/ipa-prop.h
> index 9f2479e7fdc..cb54d2547e1 100644
> --- a/gcc/ipa-prop.h
> +++ b/gcc/ipa-prop.h
> @@ -39,6 +39,15 @@ along with GCC; see the file COPYING3. If not see
> argument.
> Unknown - neither of the above.
>
> + IPA_JF_LOAD_AGG is a compound pass-through jump function, in which primary
> + operation on formal parameter is memory dereference that loads a value
> from
> + a part of an aggregate, which is represented or pointed to by the formal
> + parameter. Moreover, an additional unary/binary operation can be applied
> on
> + the loaded value, and final result is passed as actual argument of callee
> + (e.g. *(param_1(D) + 4) op 24 ). It is meant to describe usage of
> aggregate
> + parameter or by-reference parameter referenced in argument passing,
> commonly
> + found in C++ and Fortran.
> +
> IPA_JF_ANCESTOR is a special pass-through jump function, which means that
> the result is an address of a part of the object pointed to by the formal
> parameter to which the function refers. It is mainly intended to
> represent
> @@ -60,6 +69,7 @@ enum jump_func_type
> IPA_JF_UNKNOWN = 0, /* newly allocated and zeroed jump functions default
> */
> IPA_JF_CONST, /* represented by field costant */
> IPA_JF_PASS_THROUGH, /* represented by field pass_through */
> + IPA_JF_LOAD_AGG, /* represented by field load_agg */
> IPA_JF_ANCESTOR /* represented by field ancestor */
> };
>
> @@ -97,6 +107,26 @@ struct GTY(()) ipa_pass_through_data
> unsigned agg_preserved : 1;
> };
>
> +/* Structure holding data required to describe a load-value-from-aggregate
> + jump function. */
> +
> +struct GTY(()) ipa_load_agg_data
> +{
> + /* Inherit from pass through jump function, describing unary/binary
> + operation on the value loaded from aggregate that is represented or
> + pointed to by the formal parameter, specified by formal_id in this
> + pass_through jump function data structure. */
> + struct ipa_pass_through_data pass_through;
> + /* Type of the value loaded from the aggregate. */
> + tree type;
> + /* Offset at which the value is located within the aggregate. */
> + HOST_WIDE_INT offset;
> + /* True if loaded by reference (the aggregate is pointed to by the formal
> + parameter) or false if loaded by value (the aggregate is represented
> + by the formal parameter). */
> + bool by_ref;
> +};
> +
> /* Structure holding data required to describe an ancestor pass-through
> jump function. */
>
> @@ -110,58 +140,139 @@ struct GTY(()) ipa_ancestor_jf_data
> unsigned agg_preserved : 1;
> };
>
> -/* An element in an aggegate part of a jump function describing a known value
> - at a given offset. When it is part of a pass-through jump function with
> - agg_preserved set or an ancestor jump function with agg_preserved set, all
> - unlisted positions are assumed to be preserved but the value can be a type
> - node, which means that the particular piece (starting at offset and having
> - the size of the type) is clobbered with an unknown value. When
> - agg_preserved is false or the type of the containing jump function is
> - different, all unlisted parts are assumed to be unknown and all values
> must
> - fulfill is_gimple_ip_invariant. */
> +/* A jump function for an aggregate part at a given offset, which describes
> how
> + it content value is generated. All unlisted positions are assumed to
> have a
> + value defined in an unknown way. */
>
> struct GTY(()) ipa_agg_jf_item
> {
> - /* The offset at which the known value is located within the aggregate. */
> + /* The offset for the aggregate part. */
> HOST_WIDE_INT offset;
>
> - /* The known constant or type if this is a clobber. */
> - tree value;
> + /* Data type of the aggregate part. */
> + tree type;
>
> - /* Return true if OTHER describes same agg item. */
> - bool equal_to (const ipa_agg_jf_item &other);
> -};
> + /* Jump function type. */
> + enum jump_func_type jftype;
>
> + /* Represents a value of jump function. constant represents the actual
> constant
> + in constant jump function content. pass_through is used only in simple
> pass
> + through jump function context. load_agg is for
> load-value-from-aggregate
> + jump function context. */
> + union jump_func_agg_value
> + {
> + tree GTY ((tag ("IPA_JF_CONST"))) constant;
> + struct ipa_pass_through_data GTY ((tag ("IPA_JF_PASS_THROUGH")))
> pass_through;
> + struct ipa_load_agg_data GTY ((tag ("IPA_JF_LOAD_AGG"))) load_agg;
> + } GTY ((desc ("%1.jftype"))) value;
> +};
>
> -/* Aggregate jump function - i.e. description of contents of aggregates
> passed
> - either by reference or value. */
> +/* Jump functions describing a set of aggregate contents. */
>
> struct GTY(()) ipa_agg_jump_function
> {
> - /* Description of the individual items. */
> + /* Description of the individual jump function item. */
> vec<ipa_agg_jf_item, va_gc> *items;
> - /* True if the data was passed by reference (as opposed to by value). */
> + /* True if the data was passed by reference (as opposed to by value). */
> bool by_ref;
> +};
> +
> +/* An element in an aggregate part describing a known value at a given
> offset.
> + All unlisted positions are assumed to be unknown and all listed values
> must
> + fulfill is_gimple_ip_invariant. */
> +
> +struct GTY(()) ipa_agg_value
Why the GTY marker, is this structure ever allocated in garbage
collected memory? I don't think so (but it is getting late here).
> +{
> + /* The offset at which the known value is located within the aggregate. */
> + HOST_WIDE_INT offset;
>
> - /* Return true if OTHER describes same agg items. */
> - bool equal_to (const ipa_agg_jump_function &other)
> + /* The known constant. */
> + tree value;
> +
> + /* Return true if OTHER describes same agg value. */
> + bool equal_to (const ipa_agg_value &other);
> +};
> +
> +/* Structure describing a set of known offset/value for aggregate. */
> +
> +struct GTY(()) ipa_agg_value_set
Likewise, moreover...
> +{
> + /* Description of the individual item. */
> + vec<ipa_agg_value> items;
...if it is, this vector will not be handled by GC well.
> + /* True if the data was passed by reference (as opposed to by value). */
> + bool by_ref;
> +
> + /* Return true if OTHER describes same agg values. */
> + bool equal_to (const ipa_agg_value_set &other)
> {
> if (by_ref != other.by_ref)
> return false;
> - if (items != NULL && other.items == NULL)
> - return false;
> - if (!items)
> - return other.items == NULL;
> - if (items->length () != other.items->length ())
> + if (items.length () != other.items.length ())
> return false;
> - for (unsigned int i = 0; i < items->length (); i++)
> - if (!(*items)[i].equal_to ((*other.items)[i]))
> + for (unsigned int i = 0; i < items.length (); i++)
> + if (!items[i].equal_to (other.items[i]))
> return false;
> return true;
> }
> +
> + /* Return true if there is any value for aggregate. */
> + operator bool () const
> + {
> + return !items.is_empty ();
> + }
I do not know various C++ conventions well, but unless this is a really
really well established one, please don't use an operator but a normal
method. (My preference is to invert its meaning and call it is_empty
:-)
> +
> + ipa_agg_value_set copy () const
> + {
> + ipa_agg_value_set new_copy;
> +
> + new_copy.items = items.copy ();
> + new_copy.by_ref = by_ref;
> +
> + return new_copy;
> + }
> +
> + void release ()
> + {
> + items.release ();
> + }
> };
>
Thanks!
Martin
