This is an archive of the discontinued LLVM Phabricator instance.

[SCEVExpander] Support cost evaluation of several SCEVs with same budget
ClosedPublic

Authored by mkazantsev on Nov 21 2022, 10:45 PM.

Details

Summary

This is a follow-up from discussion in D138412. Sometimes we want to evaluate
the cost of expansion of several SCEVs together with same budget. For example,
if one of them is a bit above cheap limit, and the second one is free, then
we still want to expand. Checking each of them with "cheap" limit is a bit more
pessimistic.

Diff Detail

Event Timeline

mkazantsev created this revision.Nov 21 2022, 10:45 PM
Herald added a project: Restricted Project. · View Herald TranscriptNov 21 2022, 10:45 PM
mkazantsev requested review of this revision.Nov 21 2022, 10:45 PM
Herald added a project: Restricted Project. · View Herald TranscriptNov 21 2022, 10:45 PM
lebedev.ri added inline comments.Nov 22 2022, 5:08 AM
llvm/include/llvm/Transforms/Utils/ScalarEvolutionExpander.h
226

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?

(Thank you for looking into this!)

mkazantsev added inline comments.Dec 5 2022, 12:06 AM
llvm/include/llvm/Transforms/Utils/ScalarEvolutionExpander.h
226

I think it should be fine, let's try it out.

mkazantsev added inline comments.Dec 5 2022, 12:20 AM
llvm/include/llvm/Transforms/Utils/ScalarEvolutionExpander.h
226

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).

lebedev.ri accepted this revision.Dec 5 2022, 5:52 AM

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

llvm/include/llvm/Transforms/Utils/ScalarEvolutionExpander.h
226

As far as i'm concerned TargetTransformInfo::TCC_Basic is always 1,
so you could just assert that the division is lossless.

This revision is now accepted and ready to land.Dec 5 2022, 5:52 AM
This revision was landed with ongoing or failed builds.Dec 6 2022, 2:02 AM
This revision was automatically updated to reflect the committed changes.