Page MenuHomePhabricator

[NFCI] Move cost estimation from TargetLowering to TargetTransformInfo.
Needs ReviewPublic

Authored by dfukalov on Jan 19 2022, 2:02 PM.

Details

Summary

TragetLowering had two last InstructionCost related getTypeLegalizationCost()
and getScalingFactorCost() members, but all other costs are processed in TTI.

E.g. it is not comfortable to use other TTI members in these two functions
overrided in a target.

Minor refactoring: getTypeLegalizationCost() now doesn't need DataLayout
parameter - it was always passed from TTI.

Diff Detail

Event Timeline

dfukalov created this revision.Jan 19 2022, 2:02 PM
dfukalov added inline comments.Jan 19 2022, 2:10 PM
llvm/include/llvm/CodeGen/BasicTTIImpl.h
339

I'm not sure I should make it virtual (as it was in TLI) and mark it override in targets...

343–347

Perhaps TTI::getScalingFactorCost should be unified to TargetLoweringBase::AddrMode AM parameter.

dfukalov published this revision for review.Jan 19 2022, 2:10 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 19 2022, 2:10 PM

Does anyone else have any comments?

llvm/include/llvm/CodeGen/BasicTTIImpl.h
339

The TTI classes use a CRTP layout - you shouldn't need virtual classes

343–347

Not sure about whether that's useful - LoopStrengthReduction seems to be the main user of this, and it has its own data structures. We're going to end up creating the AddrMode structure somewhere, it might as well be here.

@dfukalov reverse ping - what happened with this patch?

Herald added a project: Restricted Project. · View Herald TranscriptTue, Aug 2, 5:33 AM
dfukalov updated this revision to Diff 452097.Fri, Aug 12, 12:38 AM

Rebased.
I thought we should wait for additional opinions on the patch...

Consolidating all cost-query related functions in TTI makes sense to me.

llvm/include/llvm/CodeGen/BasicTTIImpl.h
342

this could be done independently?

dfukalov added inline comments.Fri, Aug 12, 1:13 AM
llvm/include/llvm/CodeGen/BasicTTIImpl.h
342

Actually I added const since the removed TargetLoweringBase::getScalingFactorCost() was const.

dfukalov updated this revision to Diff 452623.Mon, Aug 15, 3:56 AM

Addressed comment.

RKSimon added inline comments.Mon, Aug 15, 4:17 AM
llvm/include/llvm/CodeGen/TargetLowering.h
954

doxygen comment / description?

dfukalov added inline comments.Mon, Aug 15, 7:23 AM
llvm/include/llvm/CodeGen/TargetLowering.h
954

I Just moved the declaration from private to public section to allow access it from BasicTTIImplBase<T>::getTypeLegalizationCost().
There was no description in previous place. The function returns pair of values, which are actually mainly used by getTypeAction() (uses first element of pair) and getTypeToTransformTo() (uses second element). These interfaces have descriptions (related to first and second element of pair respectively.
So, should I copy+combine these descriptions or move them? Or perhaps place references here or there (after move)?

RKSimon added inline comments.Mon, Aug 15, 7:37 AM
llvm/include/llvm/CodeGen/TargetLowering.h
954

Yeah, a copy + combine + tweak should be OK - we just need a summary of what is expected. Cheers.

dfukalov updated this revision to Diff 452800.Mon, Aug 15, 1:56 PM

Added comment/description for getTypeConversion()