This is an archive of the discontinued LLVM Phabricator instance.

[SCEV] SCEVExpander::isHighCostExpansionHelper(): cost-model polynomial recurrence
ClosedPublic

Authored by lebedev.ri on Jan 30 2020, 1:38 PM.

Details

Summary

So, i wouldn't call this *obviously* correct,
but i think i got it right this time :)

Roughly, we have

Op0*x^0 + Op1*x^1 + Op2*x^2 ...

where Op_{n} * x^{n} is called term, and n the degree of term.

Due to the way they are stored internally in SCEVAddRecExpr,
i believe we can have Op_{n} to be 0, so we should not charge for those.

I think it is most straight-forward to count the cost in 4 steps:

  1. First, count it the same way we counted scAddExpr, but be sure to skip terms with zero constants. Much like with add expr we will have one less addition than number of terms.
  2. Each non-constant term (term degree >= 1) requires a multiplication between the Op_{n} and x^{n}. But again, only charge for it if it is required - Op_{n} must not be 0 (no term) or 1 (no multiplication needed), and obviously don't charge constant terms (x^0 == 1).
  3. We must charge for all the x^0..x^{poly_degree} themselves. Since x^{poly_degree} is x * x * ... * x, i.e. poly_degree x'es multiplied, for final poly_degree term we again require poly_degree-1 multiplications. Note that all the x^{0}..x^{poly_degree-1} will be computed for the free along the way there.
  4. And finally, the operands themselves.

Here, much like with add/mul exprs, we really don't look for preexisting instructions..

Diff Detail

Event Timeline

lebedev.ri created this revision.Jan 30 2020, 1:38 PM

NFC, reorder branches for simple further patch.

lebedev.ri edited the summary of this revision. (Show Details)Jan 30 2020, 2:29 PM

Hmm, though this is still a bit too pessimistic (high) cost if we the same operand (coeffient) more than once, consider:

C0 + C1*x + C1*x*x

currently will be counted as 2x add + 3x mul, but C1*x part can be reused,
so this is actually just 2x add + 2x mul:

C0 + C1*x*(1 + x)

I'm struggling to find a counter-example where such grouping would result in too optimistic (low) cost though.

Ping @reames / @mkazantsev
Please do indicate if there is any way i can help move this process along.
Thanks

mkazantsev accepted this revision.Feb 24 2020, 10:02 PM

LGTM

llvm/lib/Analysis/ScalarEvolutionExpander.cpp
2246

This variable name is somewhat misleading, it should clearly say that it only counts non-one coefficients.

2259

Maybe also assert that the last operand is not zero (trailing zeros should be thrown away on SCEV creation).

This revision is now accepted and ready to land.Feb 24 2020, 10:02 PM
lebedev.ri marked 3 inline comments as done.Feb 25 2020, 9:58 AM

LGTM

Thank you for the review.
Here i will also add a FIXME to investigate whether this is too pessimistic.

llvm/lib/Analysis/ScalarEvolutionExpander.cpp
2259

Good idea.

lebedev.ri marked an inline comment as done.

Rebased, review notes addressed.

Actually upload updated patch - fixes got committed into a followup commit locally.

This revision was automatically updated to reflect the committed changes.