Page MenuHomePhabricator

[SCEV] SCEVExpander::isHighCostExpansionHelper(): begin cost modelling - model cast cost
ClosedPublic

Authored by lebedev.ri on Jan 30 2020, 8:14 AM.

Details

Summary

This is not a NFC, although it does not change any of the existing tests.
I'm not really sure if we should have specific tests for the cost modelling itself.

This is the first patch that actually makes SCEVExpander::isHighCostExpansionHelper()
account for the cost of the SCEV expression, and consider the budget available,
by modelling cast expressions.

I believe the logic itself is "pretty obviously correct" - from budget,
we need to subtract the cost of the cast expression from inner type Op->getType()
to the S->getType() type, and recurse into the expression we are casting.

Diff Detail

Event Timeline

lebedev.ri created this revision.Jan 30 2020, 8:14 AM
xbolva00 added inline comments.
llvm/include/llvm/Analysis/ScalarEvolutionExpander.h
177

Budget

mkazantsev added inline comments.Feb 4 2020, 7:32 PM
llvm/include/llvm/Analysis/ScalarEvolutionExpander.h
176–177

evaluate -> evaluated?

194

You already check this inside the helper, why do it twice?

llvm/lib/Analysis/ScalarEvolutionExpander.cpp
2160

Does it really need to be opcode, or we can pass SCEVType to getOperationCost?

lebedev.ri marked 2 inline comments as done.Feb 5 2020, 12:13 AM
lebedev.ri added inline comments.
llvm/lib/Analysis/ScalarEvolutionExpander.cpp
2160

TargetTransformInfo::getOperationCost() has no such SCEVType overload.
it can only be called with either opcode or preexisting existing IR instruction.

lebedev.ri updated this revision to Diff 242588.Feb 5 2020, 6:26 AM
lebedev.ri marked 4 inline comments as done.

Addressed review notes.

This revision is now accepted and ready to land.Feb 18 2020, 10:54 PM

LGTM

Thank you for the review!

Rebased, NFC.

This revision was automatically updated to reflect the committed changes.