On Wed, Apr 17, 2019 at 02:13:12PM +0800, bin.cheng wrote:
> Hi,
> As discussed in PR90078, this patch checks possible infinite_cost overflow in
> ivopts.
> Also as discussed, overflow happens mostly because of cost scaling wrto
> bb_freq/loop_freq.
> For the moment, we only implement capping in comp_cost operators, while in
> next
> stage1, we may instead implement capping in get_scaled_computation_cost_at
> with
> more supporting benchmark data.
>
> BTW, I think switching costs around comparison between infinite_cost is
> unnecessary
> since there will be no overflow in integer after capping with infinite_cost.
>
> Bootstrap and test on x86_64, is it OK?
>
> Thanks,
> bin
>
> 2019-04-17 Bin Cheng <[email protected]>
>
> PR tree-optimization/92078
> * tree-ssa-loop-ivopts.c (comp_cost::operator +,-,+=,-+,/=,*=): Add
> checks for infinite_cost overflow.
>
> 2018-04-17 Bin Cheng <[email protected]>
>
> PR tree-optimization/92078
> * gcc/testsuite/g++.dg/tree-ssa/pr90078.C: New test.
--- a/gcc/tree-ssa-loop-ivopts.c
+++ b/gcc/tree-ssa-loop-ivopts.c
@@ -243,6 +243,9 @@ operator+ (comp_cost cost1, comp_cost cost2)
if (cost1.infinite_cost_p () || cost2.infinite_cost_p ())
return infinite_cost;
+ if (cost1.cost + cost2.cost >= infinite_cost.cost)
+ return infinite_cost;
As
#define INFTY 10000000
what is the reason to keep the previous condition as well?
I mean, if cost1.cost == INFTY or cost2.cost == INFTY,
cost1.cost + cost2.cost >= INFTY too.
Unless costs can go negative.
@@ -256,6 +259,8 @@ operator- (comp_cost cost1, comp_cost cost2)
return infinite_cost;
gcc_assert (!cost2.infinite_cost_p ());
+ if (cost1.cost - cost2.cost >= infinite_cost.cost)
+ return infinite_cost;
Unless costs can be negative, when you first bail out
for cost1.cost == INFTY, then cost1.cost - cost2.cost won't
be INFTY (but could get negative). So shouldn't there be a guard against
that instead? Or, if costs can be negative, shouldn't there be also
guards that it doesn't grow too negative (say smaller than -INFTY)?
Jakub
