>> Hi, >> >> On Mon, Aug 31 2020, Feng Xue OS wrote: >> > This patch is to fix a bug that cost that is used to evaluate clone >> > candidate >> > becomes negative due to integer overflow. >> > >> > Feng >> > --- >> > 2020-08-31 Feng Xue <f...@os.amperecomputing.com> >> > >> > gcc/ >> > PR tree-optimization/96806 >> >> the component is "ipa," please change that when you commit the patch. >> >> > * ipa-cp.c (decide_about_value): Use safe_add to avoid cost >> > addition >> > overflow. >> >> assuming you have bootstrapped and tested it, it is OK for both trunk >> and all affected release branches. > >I have already added caps on things that come from profile counts so >things do not overflow, but I think in longer run we want to simply use >sreals here.. >> > && !good_cloning_opportunity_p (node, >> > - val->local_time_benefit >> > - + val->prop_time_benefit, >> > + safe_add (val->local_time_benefit, >> > + val->prop_time_benefit), >> > freq_sum, count_sum, >> > - val->local_size_cost >> > - + val->prop_size_cost)) >> > + safe_add (val->local_size_cost, >> > + val->prop_size_cost))) > >Is it also size cost that may overflow? That seem bit odd ;) >
Yes. prop_size_cost accumulates all callees' size_cost. And since there are two recursive calls, this value increases exponentially as 2's power, and easily exceeds value space of integer. It is actually a defect of cost computation for recursive cloning. But I think we need a complete consideration on how to adjust cost model for recursive cloning, including profile estimation, threshold, size_cost... And a quick fix is to add a cap here to avoid overflow. Feng >Honza >> > return false; >> > >> > if (dump_file) >> >> [...] >
