[PATCH] D117723: [NFCI] Move cost estimation from TargetLowering to TargetTransformInfo.

[Simon Pilgrim via Phabricator via llvm-commits]

RKSimon added reviewers: fhahn, dmgreen.
RKSimon added a comment.

Does anyone else have any comments?



================
Comment at: llvm/include/llvm/CodeGen/BasicTTIImpl.h:339
 
   InstructionCost getScalingFactorCost(Type *Ty, GlobalValue *BaseGV,
                                        int64_t BaseOffset, bool HasBaseReg,
----------------
dfukalov wrote:
> I'm not sure I should make it `virtual` (as it was in TLI) and mark it override in targets...
The TTI classes use a CRTP layout - you shouldn't need virtual classes


================
Comment at: llvm/include/llvm/CodeGen/BasicTTIImpl.h:343-347
     TargetLoweringBase::AddrMode AM;
     AM.BaseGV = BaseGV;
     AM.BaseOffs = BaseOffset;
     AM.HasBaseReg = HasBaseReg;
     AM.Scale = Scale;
----------------
dfukalov wrote:
> Perhaps TTI::getScalingFactorCost should be unified to TargetLoweringBase::AddrMode AM parameter.
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.  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117723