Hi,
On Thu, Nov 25 2021, Jan Hubicka wrote:
>>
>> gcc/ChangeLog:
>>
>> 2021-11-23 Martin Jambor <mjam...@suse.cz>
>>
>> PR ipa/103227
>> * ipa-prop.h (ipa_get_param): New overload. Move bits of the existing
>> one to the new one.
>> * ipa-param-manipulation.h (ipa_param_adjustments): New member
>> function get_updated_index_or_split.
>> * ipa-param-manipulation.c
>> (ipa_param_adjustments::get_updated_index_or_split): New function.
>> * ipa-prop.c (adjust_agg_replacement_values): Reimplement, add
>> capability to identify scalarized parameters and perform substitution
>> on them.
>> (ipcp_transform_function): Create descriptors earlier, handle new
>> return values of adjust_agg_replacement_values.
>>
>> gcc/testsuite/ChangeLog:
>>
>> 2021-11-23 Martin Jambor <mjam...@suse.cz>
>>
>> PR ipa/103227
>> * gcc.dg/ipa/pr103227-1.c: New test.
>> * gcc.dg/ipa/pr103227-3.c: Likewise.
>> * gcc.dg/ipa/pr103227-2.c: Likewise.
>> * gfortran.dg/pr53787.f90: Disable IPA-SRA.
>> ---
>> gcc/ipa-param-manipulation.c | 33 ++++++++++++
>> gcc/ipa-param-manipulation.h | 7 +++
>> gcc/ipa-prop.c | 73 +++++++++++++++++++--------
>> gcc/ipa-prop.h | 15 ++++--
>> gcc/testsuite/gcc.dg/ipa/pr103227-1.c | 29 +++++++++++
>> gcc/testsuite/gcc.dg/ipa/pr103227-2.c | 29 +++++++++++
>> gcc/testsuite/gcc.dg/ipa/pr103227-3.c | 52 +++++++++++++++++++
>> gcc/testsuite/gfortran.dg/pr53787.f90 | 2 +-
>> 8 files changed, 216 insertions(+), 24 deletions(-)
>> create mode 100644 gcc/testsuite/gcc.dg/ipa/pr103227-1.c
>> create mode 100644 gcc/testsuite/gcc.dg/ipa/pr103227-2.c
>> create mode 100644 gcc/testsuite/gcc.dg/ipa/pr103227-3.c
>>
>> diff --git a/gcc/ipa-param-manipulation.c b/gcc/ipa-param-manipulation.c
>> index cec1dba701f..479c20b3871 100644
>> --- a/gcc/ipa-param-manipulation.c
>> +++ b/gcc/ipa-param-manipulation.c
>> @@ -449,6 +449,39 @@ ipa_param_adjustments::get_updated_indices (vec<int>
>> *new_indices)
>> }
>> }
>>
>> +/* If a parameter with original INDEX has survived intact, return its new
>> + index. Otherwise return -1. In that case, if it has been split and
>> there
>> + is a new parameter representing a portion at unit OFFSET for which a
>> value
>> + of a TYPE can be substituted, store its new index into SPLIT_INDEX,
>> + otherwise store -1 there. */
>> +int
>> +ipa_param_adjustments::get_updated_index_or_split (int index,
>> + unsigned unit_offset,
>> + tree type, int *split_index)
>> +{
>> + unsigned adj_len = vec_safe_length (m_adj_params);
>> + for (unsigned i = 0; i < adj_len ; i++)
>
> In ipa-modref I precompute this to map so we do not need to walk all
> params, but the loop is probably not bad since functions do not have
> tens of thousdands parameters :)
The most I have seen is about 70 and those were big outliers.
I was thinking of precomputing it somehow but for one parameter there
can be up to param ipa-sra-max-replacements replacements (default 8 -
and there is another, by default stricter, limit for pointers). So it
would have to be a hash table or something like it.
>
> Can I use it in ipa-modref to discover what parameters was turned from
> by-reference to scalar, too?
IIUC, I don't think you directly can, also because for one parameter you
can have more scalar replacements and the interface needs an offset for
which to look. OTOH, if you only care about simple scalars passed by
reference, then passing zero as offset - and probably adding a flag to
check there are no replacements at other offsets - would work. (But
that information could also be easily pre-computed.)
>> + {
>> + ipa_adjusted_param *apm = &(*m_adj_params)[i];
>> + if (apm->base_index != index)
>> + continue;
>> + if (apm->op == IPA_PARAM_OP_COPY)
>> + return i;
>> + if (apm->op == IPA_PARAM_OP_SPLIT
>> + && apm->unit_offset == unit_offset)
>> + {
>> + if (useless_type_conversion_p (apm->type, type))
>> + *split_index = i;
>> + else
>> + *split_index = -1;
>> + return -1;
>> + }
>> + }
>> +
>> + *split_index = -1;
>> + return -1;
>> +}
>> +
>> /* Return the original index for the given new parameter index. Return a
>> negative number if not available. */
>>
>> diff --git a/gcc/ipa-param-manipulation.h b/gcc/ipa-param-manipulation.h
>> index 5adf8a22356..d1dad9fac73 100644
>> --- a/gcc/ipa-param-manipulation.h
>> +++ b/gcc/ipa-param-manipulation.h
>> @@ -236,6 +236,13 @@ public:
>> void get_surviving_params (vec<bool> *surviving_params);
>> /* Fill a vector with new indices of surviving original parameters. */
>> void get_updated_indices (vec<int> *new_indices);
>> + /* If a parameter with original INDEX has survived intact, return its new
>> + index. Otherwise return -1. In that case, if it has been split and
>> there
>> + is a new parameter representing a portion at UNIT_OFFSET for which a
>> value
>> + of a TYPE can be substituted, store its new index into SPLIT_INDEX,
>> + otherwise store -1 there. */
>> + int get_updated_index_or_split (int index, unsigned unit_offset, tree
>> type,
>> + int *split_index);
>> /* Return the original index for the given new parameter index. Return a
>> negative number if not available. */
>> int get_original_index (int newidx);
>> diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c
>> index e85df0971fc..a297f50e945 100644
>> --- a/gcc/ipa-prop.c
>> +++ b/gcc/ipa-prop.c
>> @@ -5578,32 +5578,55 @@ ipcp_read_transformation_summaries (void)
>> }
>>
>> /* Adjust the aggregate replacements in AGGVAL to reflect parameters
>> skipped in
>> - NODE. */
>> + NODE but also if any parameter was IPA-SRAed into a scalar go ahead with
>> + substitution of the default_definitions of that new param with the
>> + appropriate constant.
>>
>> -static void
>> -adjust_agg_replacement_values (struct cgraph_node *node,
>> - struct ipa_agg_replacement_value *aggval)
>> + Return two bools. the first it true if at least one item in AGGVAL still
>> + exists and function body walk should go ahead. The second is true if any
>> + values were already substituted for scalarized parameters and update_cfg
>> + shuld be run after replace_uses_by. */
>> +
>> +static std::pair<bool, bool>
> What a fancy C++ :)
Yeah :-) apparently I was spoiled by python this weekend.
>> +adjust_agg_replacement_values (cgraph_node *node,
>> + ipa_agg_replacement_value *aggval,
>> + const vec<ipa_param_descriptor, va_gc>
>> + &descriptors)
>
> Patch is OK.
Thanks a lot,
Martin
