[PATCH] D62563: Fix incorrect expand of non-linear addrecs

[Artur Pilipenko via Phabricator via llvm-commits]

apilipenko added a comment.

In D62563#1521477 <https://reviews.llvm.org/D62563#1521477>, @reames wrote:

> Artur, nice find.  In terms of staging complexity, have you considered how impactful it would be to simply refuse to generate the value at the given iteration in this case?  evaluateAtIteration is allowed to return SCEVCouldNotCompute.  I'm tempted to first introduce a bailout for a quick correctness fix - maybe along side your assert to see if we're missing any other cases - and then spend more time considering your full fix.


Yes, evaluateAtIteration can return SCEVCouldNotCompute, but it doesn't seem like SCEVExpander is ready for that. What we can do instead is to use expandAddRecExprLiterally for non-affine addrecs.

> One observation: I really don't think we want to be emitting an i9.  We probably want to be rounding up to the nearest legal type.  For i65, I doubt we want to be generating it at all.  It's probably best to simply bail out in that case.

evaluateAtIteration is already emitting i9 types for high order binomial coefficients. See the debug output in the review description. But I guess you are right, that we don't want to introduce canonical IVs of non-legal types. If we need to bail out for some types the the bail out would be expandAddRecExprLiterally.

In general I like the idea of using expandAddRecExprLiterally for non-affine addrecs, I'm going to run some performance experiments with this approach to see the impact.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62563/new/

https://reviews.llvm.org/D62563