Re: [PATCH] Fix UBSAN errors in ipa-cp.

["Martin Liška" <mliska@suse.cz>]

On 9/23/20 5:02 PM, Martin Jambor wrote:
> Hi,
> 
> On Wed, Sep 23 2020, Martin Liška wrote:
>> There's patch that does that.
>>
>> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>>
>> Ready to be installed?
>> Thanks,
>> Martin
>>  From ff5f78110684ed9aedde15d19e856b3acf649971 Mon Sep 17 00:00:00 2001
>> From: Martin Liska <mliska@suse.cz>
>> Date: Wed, 23 Sep 2020 15:10:43 +0200
>> Subject: [PATCH] Port IPA CP time and size to sreal
>>
>> gcc/ChangeLog:
>>
>> 	* ipa-cp.c (ipcp_lattice::print): Print sreal values
>> 	with to_double.
>> 	(incorporate_penalties): Change type to sreal.
>> 	(good_cloning_opportunity_p): Change args to sreal.
>> 	(get_max_overall_size): Work in sreal type.
>> 	(estimate_local_effects): Likewise.
>> 	(safe_add): Remove.
>> 	(value_topo_info::propagate_effects): Work in sreal type.
>> 	(ipcp_propagate_stage): Print sreal numbers.
>> 	(decide_about_value): Work in sreal type.
>> ---
>>   gcc/ipa-cp.c | 128 ++++++++++++++++++++++++---------------------------
>>   1 file changed, 59 insertions(+), 69 deletions(-)
>>
>> diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c
>> index b3e7d41ea10..9a79b5862f8 100644
>> @@ -3224,8 +3226,9 @@ incorporate_penalties (cgraph_node *node, ipa_node_params *info,
>>      potential new clone in FREQUENCIES.  */
>>   
>>   static bool
>> -good_cloning_opportunity_p (struct cgraph_node *node, int time_benefit,
>> -			    int freq_sum, profile_count count_sum, int size_cost)
>> +good_cloning_opportunity_p (struct cgraph_node *node, sreal time_benefit,
>> +			    sreal freq_sum, profile_count count_sum,
> 
> Is the change of type of freq_sum intentional?  Even with this patch,
> all the callers will keep passing their frequency sums as ints and so
> the implicit conversion seems a bit misleading.
> 
> I guess complete transition to sreals would include also switch from
> using cgraph_edge::frequency to sreal_frequency in
> get_info_about_necessary_edges and users of struct caller_statistics.

Hello.

All right, I'm leaving that to you Martin ;) You are much more familiar
with the code base.

Martin

> 
> I planned to work on conversions to sreals right after resolving various
> exchange2 issues and I can still do this bit afterwards, but the change
> in the parameter type looks like you wanted to but eventually did not?
> 
> Thanks,
> 
> Martin
> 
>