> Hi,
>
> gcc/ChangeLog:
>
> 2021-06-29 Martin Jambor <mjam...@suse.cz>
>
> * cgraph.h (ipa_replace_map): New field force_load_ref.
> * ipa-prop.h (ipa_param_descriptor): Reduce precision of move_cost,
> aded new flag load_dereferenced, adjusted comments.
> (ipa_get_param_dereferenced): New function.
> (ipa_set_param_dereferenced): Likewise.
> * cgraphclones.c (cgraph_node::create_virtual_clone): Follow it.
> * ipa-cp.c: Include gimple.h.
> (ipcp_discover_new_direct_edges): Take into account dereferenced flag.
> (get_replacement_map): New parameter force_load_ref, set the
> appropriate flag in ipa_replace_map if set.
> (struct symbol_and_index_together): New type.
> (adjust_references_in_act_callers): New function.
> (adjust_references_in_caller): Likewise.
> (create_specialized_node): When appropriate, call
> adjust_references_in_caller and force only load references.
> * ipa-prop.c (load_from_dereferenced_name): New function.
> (ipa_analyze_controlled_uses): Also detect loads from a
> dereference, harden testing of call statements.
> (ipa_write_node_info): Stream the dereferenced flag.
> (ipa_read_node_info): Likewise.
> (ipa_set_jf_constant): Also create refdesc when jump function
> references a variable.
> (cgraph_node_for_jfunc): Rename to symtab_node_for_jfunc, work
> also on references of variables and return a symtab_node. Adjust
> all callers.
> (propagate_controlled_uses): Also remove references to VAR_DECLs.
>
> gcc/testsuite/ChangeLog:
>
> 2021-06-29 Martin Jambor <mjam...@suse.cz>
>
> * gcc.dg/ipa/remref-3.c: New test.
> * gcc.dg/ipa/remref-4.c: Likewise.
> * gcc.dg/ipa/remref-5.c: Likewise.
> * gcc.dg/ipa/remref-6.c: Likewise.
> ---
> gcc/cgraph.h | 3 +
> gcc/cgraphclones.c | 10 +-
> gcc/ipa-cp.c | 146 ++++++++++++++++++++++--
> gcc/ipa-prop.c | 166 ++++++++++++++++++++++------
> gcc/ipa-prop.h | 27 ++++-
> gcc/testsuite/gcc.dg/ipa/remref-3.c | 23 ++++
> gcc/testsuite/gcc.dg/ipa/remref-4.c | 31 ++++++
> gcc/testsuite/gcc.dg/ipa/remref-5.c | 38 +++++++
> gcc/testsuite/gcc.dg/ipa/remref-6.c | 24 ++++
> 9 files changed, 419 insertions(+), 49 deletions(-)
> create mode 100644 gcc/testsuite/gcc.dg/ipa/remref-3.c
> create mode 100644 gcc/testsuite/gcc.dg/ipa/remref-4.c
> create mode 100644 gcc/testsuite/gcc.dg/ipa/remref-5.c
> create mode 100644 gcc/testsuite/gcc.dg/ipa/remref-6.c
>
> diff --git a/gcc/cgraph.h b/gcc/cgraph.h
> index 9f4338fdf87..0fc20cd4517 100644
> --- a/gcc/cgraph.h
> +++ b/gcc/cgraph.h
> @@ -700,6 +700,9 @@ struct GTY(()) ipa_replace_map
> tree new_tree;
> /* Parameter number to replace, when old_tree is NULL. */
> int parm_num;
> + /* Set if the newly added reference should not be an address one, but a
> load
> + one from the operand of the ADDR_EXPR in NEW_TREE. */
So this is in case where parameter p is used only as *p?
I think the comment should be expanded to explain the situation or in a
year I will not know why we need such a flag :)
> @@ -4320,7 +4322,8 @@ gather_edges_for_value (ipcp_value<valtype> *val,
> cgraph_node *dest,
> Return it or NULL if for some reason it cannot be created. */
>
> static struct ipa_replace_map *
> -get_replacement_map (class ipa_node_params *info, tree value, int parm_num)
> +get_replacement_map (class ipa_node_params *info, tree value, int parm_num,
> + bool force_load_ref)
You want to comment the parameter here too..
> +/* At INDEX of a function being called by CS there is an ADDR_EXPR of a
> + variable which is only dereferenced and which is represented by SYMBOL.
> See
> + if we can remove ADDR reference in callers assosiated witht the call. */
> +
> +static void
> +adjust_references_in_caller (cgraph_edge *cs, symtab_node *symbol, int index)
> +{
> + ipa_edge_args *args = ipa_edge_args_sum->get (cs);
> + ipa_jump_func *jfunc = ipa_get_ith_jump_func (args, index);
> + if (jfunc->type == IPA_JF_CONST)
> + {
> + ipa_ref *to_del = cs->caller->find_reference (symbol, cs->call_stmt,
> + cs->lto_stmt_uid);
> + if (!to_del)
> + return;
> + to_del->remove_reference ();
> + if (dump_file)
> + fprintf (dump_file, " Removed a reference from %s to %s.\n",
> + cs->caller->dump_name (), symbol->dump_name ());
> + return;
> + }
> +
> + if (jfunc->type != IPA_JF_PASS_THROUGH
> + || ipa_get_jf_pass_through_operation (jfunc) != NOP_EXPR)
> + return;
> +
> + int fidx = ipa_get_jf_pass_through_formal_id (jfunc);
> + cgraph_node *caller = cs->caller;
> + ipa_node_params *caller_info = ipa_node_params_sum->get (caller);
> + /* TODO: This consistency check may be too big and not really
> + that useful. Consider removing it. */
> + tree cst;
> + if (caller_info->ipcp_orig_node)
> + cst = caller_info->known_csts[fidx];
> + else
> + {
> + ipcp_lattice<tree> *lat = ipa_get_scalar_lat (caller_info, fidx);
> + gcc_assert (lat->is_single_const ());
> + cst = lat->values->value;
> + }
> + gcc_assert (TREE_CODE (cst) == ADDR_EXPR
> + && symtab_node::get (TREE_OPERAND (cst, 0)) == symbol);
> +
> + int cuses = ipa_get_controlled_uses (caller_info, fidx);
> + if (cuses == IPA_UNDESCRIBED_USE)
> + return;
> + gcc_assert (cuses > 0);
> + cuses--;
> + ipa_set_controlled_uses (caller_info, fidx, cuses);
> + if (cuses)
> + return;
> +
> + if (caller_info->ipcp_orig_node)
> + {
> + /* Cloning machinery has created a reference here, we need to either
> + remove it or change it to a read one. */
> + ipa_ref *to_del = caller->find_reference (symbol, NULL, 0);
> + if (to_del && to_del->use == IPA_REF_ADDR)
> + {
> + to_del->remove_reference ();
> + if (dump_file)
> + fprintf (dump_file, " Removed a reference from %s to %s.\n",
> + cs->caller->dump_name (), symbol->dump_name ());
> + if (ipa_get_param_load_dereferenced (caller_info, fidx))
> + {
> + caller->create_reference (symbol, IPA_REF_LOAD, NULL);
> + if (dump_file)
> + fprintf (dump_file,
> + " ...and replaced it with LOAD one.\n");
> + }
> + }
> + }
> +
> + symbol_and_index_together pack;
> + pack.symbol = symbol;
> + pack.index = fidx;
> + caller->call_for_symbol_thunks_and_aliases
> (adjust_references_in_act_callers,
> + &pack, true);
> +}
> +
> +
> /* Return true if we would like to remove a parameter from NODE when cloning
> it
> with KNOWN_CSTS scalar constants. */
>
> @@ -4595,15 +4708,28 @@ create_specialized_node (struct cgraph_node *node,
> for (i = 0; i < count; i++)
> {
> tree t = known_csts[i];
> - if (t)
> - {
> - struct ipa_replace_map *replace_map;
> + if (!t)
> + continue;
>
> - gcc_checking_assert (TREE_CODE (t) != TREE_BINFO);
> - replace_map = get_replacement_map (info, t, i);
> - if (replace_map)
> - vec_safe_push (replace_trees, replace_map);
> + gcc_checking_assert (TREE_CODE (t) != TREE_BINFO);
> +
> + bool load_ref = false;
> + symtab_node *ref_symbol;
> + if (TREE_CODE (t) == ADDR_EXPR
> + && TREE_CODE (TREE_OPERAND (t, 0)) == VAR_DECL
Can't you see more complicate expressions like &var[10]?
So you want to find the base term here I think.
> +/* Return true EXPR is a load from a dereference of SSA_NAME NAME. */
> +
> +static bool
> +load_from_dereferenced_name (tree expr, tree name)
> +{
> + tree base = get_base_address (expr);
> + return (TREE_CODE (base) == MEM_REF
> + && TREE_OPERAND (base, 0) == name);
> +}
Similarly here.
> /* The parameter is used. */
> unsigned used : 1;
> unsigned used_by_ipa_predicates : 1;
> unsigned used_by_indirect_call : 1;
> unsigned used_by_polymorphic_call : 1;
> + /* Set to true when in addition to being used in call statements, the
> + parameterr has also been used for loads (but not for wtires, does not
> + escape, etc.). */
> + unsigned load_dereferenced : 1;
Some typos here :)
(and also I think comment should explain why we need the flag)
The patch looks reasonable to me,
Honza
