On Wed, Apr 17, 2019 at 3:10 PM Jakub Jelinek <ja...@redhat.com> wrote: > > 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 <bin.ch...@linux.alibaba.com> > > > > PR tree-optimization/92078 > > * tree-ssa-loop-ivopts.c (comp_cost::operator +,-,+=,-+,/=,*=): Add > > checks for infinite_cost overflow. > > > > 2018-04-17 Bin Cheng <bin.ch...@linux.alibaba.com> > > > > 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. It's a bit complicated, but in general, 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)? Negative cost is kind of a result of booking cost cancellation at different place. For example, it mostly comes from in modeling auto increment/decrement addressing mode. To be specific, when IV's increment instruction can be merged into addressing mode, we cancel cost of IV increment operation in cand-use cost. Very likely 4 will be subtracted. In general, we wouldn't expect negative cost can go too big, so there is no -INFTY logic in ivopts at all. So this is the least invasive fix for the moment, I would consider capping bb_freq/loop_freq in the future which should rule out the overflow possibility in the first place. Thanks, bin > > Jakub
