[PATCH] D138475: [SCEVExpander] Support cost evaluation of several SCEVs with same budget

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri accepted this revision.
lebedev.ri added a comment.
This revision is now accepted and ready to land.

While i do think that the suggestion would be useful,
this is better than nothing



================
Comment at: llvm/include/llvm/Transforms/Utils/ScalarEvolutionExpander.h:226
+  bool isHighCostExpansion(ArrayRef<const SCEV *> Exprs, Loop *L,
+                           unsigned Budget, const TargetTransformInfo *TTI,
                            const Instruction *At) {
----------------
mkazantsev wrote:
> mkazantsev wrote:
> > lebedev.ri wrote:
> > > I was thinking `Budget` should be a non-const-ref,
> > > and we should just subtract the computed cost from it
> > > if we return false from this function.
> > > Would that be worse?
> > I think it should be fine, let's try it out.
> Let's leave it as is. The issue I'm seeing is that the estimate is done in terms of scaled budget, and there is no clear translation of scaled budget back into the normal one (division is lossy).
> 
> 
As far as i'm concerned `TargetTransformInfo::TCC_Basic` is always `1`,
so you could just assert that the division is lossless.


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

https://reviews.llvm.org/D138475