NFC: Change getUserCost to return InstructionCost

[sdesmalen]

Summary

This patch migrates the TTI cost interfaces to return an InstructionCost.

See this patch for the introduction of the type: https://reviews.llvm.org/D91174
See this thread for context: http://lists.llvm.org/pipermail/llvm-dev/2020-November/146408.html

Depends on D97382

[paulwalker-arm]

llvm/include/llvm/Support/InstructionCost.h

213–221

Are these strictly necessary? I'm wondering if there's just the odd comparison somewhere whose operands can be switched.

Or is it the case that these are a temporary measure that will be removed once the current LHS becomes an InstructionCost? If so I think it's worth adding a //TODO: Remove me when...

llvm/lib/Target/ARM/ARMTargetTransformInfo.cpp

2155

More of an open question but is this initialisation required/preferred? Or to put another way, what is the expected meaning of InstructionCost()? Invalid? Zero?

[sdesmalen]

Removed operator== and operator!=.

[sdesmalen]

llvm/include/llvm/Support/InstructionCost.h

213–221

I've removed them, it was indeed the odd comparison in InlineCost.cpp.

llvm/lib/Target/ARM/ARMTargetTransformInfo.cpp

2155

The default InstructionCost() initialises with a valid 0. I would prefer to remove that constructor in favour of initializing it with an explicit value (when this is used for an accumulating cost value) or Invalid when used as 'uninitialized'. The only issue is that this requires some more changes, because it's used in a std::pair<> in e.g. LoopVectorize.cpp, where it needs a default constructor.

[ctetreau]

Does changing the -1's to invalid's constitute a functional change? Are they always checked for? If they are just blindly added to other costs, then they would result in valid costs becoming invalid costs.

llvm/lib/Target/ARM/ARMTargetTransformInfo.cpp

2155

I don't like the default constructor either. I think a default constructed InstructionCost should be invalid.

[dmgreen]

llvm/include/llvm/Analysis/TargetTransformInfoImpl.h

1029

This seems strange. Does it need to return an invalid cost? Or can it just return a cost of 1 for all cost kinds?

[sdesmalen]

Removed possibly functional changes (-1 -> Invalid) from this patch.

[sdesmalen]

In D97466#2590977, @ctetreau wrote:

Does changing the -1's to invalid's constitute a functional change? Are they always checked for? If they are just blindly added to other costs, then they would result in valid costs becoming invalid costs.

In TargetTransformInfo::getUserCost (in TargetTransformInfo.cpp) it has the following check:

InstructionCost Cost = TTIImpl->getUserCost(U, Operands, CostKind);
assert((CostKind == TTI::TCK_RecipThroughput || Cost >= 0) &&
       "TTI should not produce negative costs!");

By returning Invalid the assert no longer fails but instead Invalid is propagated, which you are right is a functional change.
I'll revert to returning -1 instead of Invalid, and will leave that to a separate (non-NFC) patch.

llvm/include/llvm/Analysis/TargetTransformInfoImpl.h

1029

For these patches, I haven't really tried to understand the reason behind why things have been cost-modelled the way they have, and have instead focussed on the interface-change alone. I agree returning -1 seems odd, I don't know why this cannot be costed.

[ctetreau]

Assuming everybody else is satisfied, LGTM

llvm/include/llvm/Analysis/TargetTransformInfo.h

InstructionCost getInstructionCost(const Instruction *I,
     case TCK_Latency:
       Cost = getInstructionLatency(I);
       break;
     case TCK_CodeSize:
     case TCK_SizeAndLatency:
       Cost = getUserCost(I, kind);
       break;
     }
-    if (Cost == -1)
-      Cost.setInvalid();
     return Cost;
   }
 
   /// Underlying constants for 'cost' values in this interface.
   ///
   /// Many APIs in this interface return a cost. This enum defines the
   /// fundamental values that should be used to interpret (and produce) those
   /// costs. The costs are returned as an int rather than a member of this
public:
   /// \p Operands is a list of operands which can be a result of transformations
   /// of the current operands. The number of the operands on the list must equal
   /// to the number of the current operands the IR user has. Their order on the
   /// list must be the same as the order of the current operands the IR user
   /// has.
   ///
   /// The returned cost is defined in terms of \c TargetCostConstants, see its
   /// comments for a detailed explanation of the cost values.
-  int getUserCost(const User *U, ArrayRef<const Value *> Operands,
+  InstructionCost getUserCost(const User *U, ArrayRef<const Value *> Operands,
                               TargetCostKind CostKind) const;
 
   /// This is a helper function which calls the two-argument getUserCost
   /// with \p Operands which are the current operands U has.
-  int getUserCost(const User *U, TargetCostKind CostKind) const {
+  InstructionCost getUserCost(const User *U, TargetCostKind CostKind) const {
     SmallVector<const Value *, 4> Operands(U->operand_values());
     return getUserCost(U, Operands, CostKind);
   }
 
   /// Return true if branch divergence exists.
   ///
   /// Branch divergence has a significantly negative impact on GPU performance
   /// when threads in the same wavefront take different paths due to conditional

 
   /// @}
 
   /// @}
 
 private:
   /// Estimate the latency of specified instruction.
   /// Returns 1 as the default value.
-  int getInstructionLatency(const Instruction *I) const;
+  InstructionCost getInstructionLatency(const Instruction *I) const;
 
   /// Returns the expected throughput cost of the instruction.
   /// Returns -1 if the cost is unknown.
-  int getInstructionThroughput(const Instruction *I) const;
+  InstructionCost getInstructionThroughput(const Instruction *I) const;
 
   /// The abstract base class used to type erase specific TTI
   /// implementations.
   class Concept;
 
   /// The template model for the base class which wraps a concrete
   /// implementation in a type erased interface.
   template <typename T> class Model;
public:
   virtual unsigned getInliningThresholdMultiplier() = 0;
   virtual unsigned adjustInliningThreshold(const CallBase *CB) = 0;
   virtual int getInlinerVectorBonusPercent() = 0;
   virtual int getMemcpyCost(const Instruction *I) = 0;
   virtual unsigned
   getEstimatedNumberOfCaseClusters(const SwitchInst &SI, unsigned &JTSize,
                                    ProfileSummaryInfo *PSI,
                                    BlockFrequencyInfo *BFI) = 0;
-  virtual int getUserCost(const User *U, ArrayRef<const Value *> Operands,
+  virtual InstructionCost getUserCost(const User *U,
+                                      ArrayRef<const Value *> Operands,
                                       TargetCostKind CostKind) = 0;
   virtual bool hasBranchDivergence() = 0;
   virtual bool useGPUDivergenceAnalysis() = 0;
   virtual bool isSourceOfDivergence(const Value *V) = 0;
   virtual bool isAlwaysUniform(const Value *V) = 0;
   virtual unsigned getFlatAddressSpace() = 0;
   virtual bool collectFlatAddressOperands(SmallVectorImpl<int> &OpIndexes,
                                           Intrinsic::ID IID) const = 0;
   virtual bool isNoopAddrSpaceCast(unsigned FromAS, unsigned ToAS) const = 0;
public:
   virtual bool preferInLoopReduction(unsigned Opcode, Type *Ty,
                                      ReductionFlags) const = 0;
   virtual bool preferPredicatedReductionSelect(unsigned Opcode, Type *Ty,
                                                ReductionFlags) const = 0;
   virtual bool shouldExpandReduction(const IntrinsicInst *II) const = 0;
   virtual unsigned getGISelRematGlobalCost() const = 0;
   virtual bool supportsScalableVectors() const = 0;
   virtual bool hasActiveVectorLength() const = 0;
-  virtual int getInstructionLatency(const Instruction *I) = 0;
+  virtual InstructionCost getInstructionLatency(const Instruction *I) = 0;
 };
 
 template <typename T>
 class TargetTransformInfo::Model final : public TargetTransformInfo::Concept {
   T Impl;
 
 public:
   Model(T Impl) : Impl(std::move(Impl)) {}
unsigned adjustInliningThreshold(const CallBase *CB) override {
     return Impl.adjustInliningThreshold(CB);
   }
   int getInlinerVectorBonusPercent() override {
     return Impl.getInlinerVectorBonusPercent();
   }
   int getMemcpyCost(const Instruction *I) override {
     return Impl.getMemcpyCost(I);
   }
-  int getUserCost(const User *U, ArrayRef<const Value *> Operands,
+  InstructionCost getUserCost(const User *U, ArrayRef<const Value *> Operands,
                               TargetCostKind CostKind) override {
     return Impl.getUserCost(U, Operands, CostKind);
   }
   bool hasBranchDivergence() override { return Impl.hasBranchDivergence(); }
   bool useGPUDivergenceAnalysis() override {
     return Impl.useGPUDivergenceAnalysis();
   }
   bool isSourceOfDivergence(const Value *V) override {
     return Impl.isSourceOfDivergence(V);
public:
   bool supportsScalableVectors() const override {
     return Impl.supportsScalableVectors();
   }
 
   bool hasActiveVectorLength() const override {
     return Impl.hasActiveVectorLength();
   }
 
-  int getInstructionLatency(const Instruction *I) override {
+  InstructionCost getInstructionLatency(const Instruction *I) override {
     return Impl.getInstructionLatency(I);
   }
 };
 
 template <typename T>
 TargetTransformInfo::TargetTransformInfo(T Impl)
     : TTIImpl(new Model<T>(Impl)) {}
 

llvm/include/llvm/Analysis/TargetTransformInfoImpl.h

int getGEPCost(Type *PointeeType, const Value *Ptr,
     if (static_cast<T *>(this)->isLegalAddressingMode(
             TargetType, const_cast<GlobalValue *>(BaseGV),
             BaseOffset.sextOrTrunc(64).getSExtValue(), HasBaseReg, Scale,
             Ptr->getType()->getPointerAddressSpace()))
       return TTI::TCC_Free;
     return TTI::TCC_Basic;
   }
 
-  int getUserCost(const User *U, ArrayRef<const Value *> Operands,
+  InstructionCost getUserCost(const User *U, ArrayRef<const Value *> Operands,
                               TTI::TargetCostKind CostKind) {
     auto *TargetTTI = static_cast<T *>(this);
     // Handle non-intrinsic calls, invokes, and callbr.
     // FIXME: Unlikely to be true for anything but CodeSize.
     auto *CB = dyn_cast<CallBase>(U);
     if (CB && !isa<IntrinsicInst>(U)) {
       if (const Function *F = CB->getCalledFunction()) {
         if (!TargetTTI->isLoweredToCall(F))
           return TTI::TCC_Basic; // Give a basic cost if it will be lowered
case Instruction::ShuffleVector: {
       auto *VecSrcTy = cast<VectorType>(U->getOperand(0)->getType());
 
       // TODO: Identify and add costs for insert subvector, etc.
       int SubIndex;
       if (Shuffle->isExtractSubvectorMask(SubIndex))
         return TargetTTI->getShuffleCost(TTI::SK_ExtractSubvector, VecSrcTy,
                                          SubIndex, VecTy);
       else if (Shuffle->changesLength())
-        return CostKind == TTI::TCK_RecipThroughput ? -1 : 1;
+        return CostKind == TTI::TCK_RecipThroughput
+                   ? InstructionCost::getInvalid()
+                   : 1;
       else if (Shuffle->isIdentity())
         return 0;
       else if (Shuffle->isReverse())
         return TargetTTI->getShuffleCost(TTI::SK_Reverse, VecTy, 0, nullptr);
       else if (Shuffle->isSelect())
         return TargetTTI->getShuffleCost(TTI::SK_Select, VecTy, 0, nullptr);
       else if (Shuffle->isTranspose())
         return TargetTTI->getShuffleCost(TTI::SK_Transpose, VecTy, 0, nullptr);
case Instruction::ExtractElement: {
       return TargetTTI->getVectorInstrCost(Opcode, U->getOperand(0)->getType(),
                                            Idx);
     }
     }
     // By default, just classify everything as 'basic'.
     return TTI::TCC_Basic;
   }
 
-  int getInstructionLatency(const Instruction *I) {
+  InstructionCost getInstructionLatency(const Instruction *I) {
     SmallVector<const Value *, 4> Operands(I->operand_values());
     if (getUserCost(I, Operands, TTI::TCK_Latency) == TTI::TCC_Free)
       return 0;
 
     if (isa<LoadInst>(I))
       return 4;
 
     Type *DstTy = I->getType();

llvm/include/llvm/CodeGen/BasicTTIImpl.h

Optional<Value *> simplifyDemandedVectorEltsIntrinsic(
       APInt &UndefElts2, APInt &UndefElts3,
       std::function<void(Instruction *, unsigned, APInt, APInt &)>
           SimplifyAndSetOp) {
     return BaseT::simplifyDemandedVectorEltsIntrinsic(
         IC, II, DemandedElts, UndefElts, UndefElts2, UndefElts3,
         SimplifyAndSetOp);
   }
 
-  int getInstructionLatency(const Instruction *I) {
+  InstructionCost getInstructionLatency(const Instruction *I) {
     if (isa<LoadInst>(I))
       return getST()->getSchedModel().DefaultLoadLatency;
 
     return BaseT::getInstructionLatency(I);
   }
 
   virtual Optional<unsigned>
   getCacheSize(TargetTransformInfo::CacheLevel Level) const {

llvm/include/llvm/Support/InstructionCost.h

public:
   bool operator>=(const CostType RHS) const {
     InstructionCost RHS2(RHS);
     return *this >= RHS2;
   }
 
   void print(raw_ostream &OS) const;
 };
 
+inline bool operator==(const InstructionCost::CostType LHS,
+                       const InstructionCost &RHS) {
+  return RHS == LHS;
+}
+
+inline bool operator!=(const InstructionCost::CostType LHS,
+                       const InstructionCost &RHS) {
+  return RHS != LHS;
+}
+
 inline InstructionCost operator+(const InstructionCost &LHS,
                                  const InstructionCost &RHS) {
   InstructionCost LHS2(LHS);
   LHS2 += RHS;
   return LHS2;
 }
 
 inline InstructionCost operator-(const InstructionCost &LHS,

llvm/lib/Analysis/TargetTransformInfo.cpp

 }
 
 unsigned TargetTransformInfo::getEstimatedNumberOfCaseClusters(
     const SwitchInst &SI, unsigned &JTSize, ProfileSummaryInfo *PSI,
     BlockFrequencyInfo *BFI) const {
   return TTIImpl->getEstimatedNumberOfCaseClusters(SI, JTSize, PSI, BFI);
 }
 
-int TargetTransformInfo::getUserCost(const User *U,
-                                     ArrayRef<const Value *> Operands,
-                                     enum TargetCostKind CostKind) const {
-  int Cost = TTIImpl->getUserCost(U, Operands, CostKind);
+InstructionCost
+TargetTransformInfo::getUserCost(const User *U,
+  InstructionCost Cost = TTIImpl->getUserCost(U, Operands, CostKind);
   assert((CostKind == TTI::TCK_RecipThroughput || Cost >= 0) &&
          "TTI should not produce negative costs!");
   return Cost;
 }
 
 bool TargetTransformInfo::hasBranchDivergence() const {
   return TTIImpl->hasBranchDivergence();
 }

 unsigned TargetTransformInfo::getGISelRematGlobalCost() const {
   return TTIImpl->getGISelRematGlobalCost();
 }
 
 bool TargetTransformInfo::supportsScalableVectors() const {
   return TTIImpl->supportsScalableVectors();
 }
 
-int TargetTransformInfo::getInstructionLatency(const Instruction *I) const {
+InstructionCost
+TargetTransformInfo::getInstructionLatency(const Instruction *I) const {
   return TTIImpl->getInstructionLatency(I);
 }
 
 static bool matchPairwiseShuffleMask(ShuffleVectorInst *SI, bool IsLeft,
                                      unsigned Level) {
   // We don't need a shuffle if we just want to have element 0 in position 0 of
   // the vector.
   if (!SI && Level == 0 && IsLeft)
TTI::matchVectorReduction(const ExtractElementInst *Root, unsigned &Opcode,
   if (RdxKind != TTI::ReductionKind::RK_None) {
     IsPairwise = false;
     return RdxKind;
   }
   IsPairwise = true;
   return matchPairwiseReduction(Root, Opcode, Ty);
 }
 
-int TargetTransformInfo::getInstructionThroughput(const Instruction *I) const {
+InstructionCost
+TargetTransformInfo::getInstructionThroughput(const Instruction *I) const {
   TTI::TargetCostKind CostKind = TTI::TCK_RecipThroughput;
 
   switch (I->getOpcode()) {
   case Instruction::GetElementPtr:
   case Instruction::Ret:
   case Instruction::PHI:
   case Instruction::Br:
   case Instruction::Add:
TargetTransformInfo::getInstructionThroughput(const Instruction *I) const {
   case Instruction::ExtractElement:
   case Instruction::InsertElement:
   case Instruction::ExtractValue:
   case Instruction::ShuffleVector:
   case Instruction::Call:
     return getUserCost(I, CostKind);
   default:
     // We don't have any information on this instruction.
-    return -1;
+    return InstructionCost::getInvalid();
   }
 }
 
 TargetTransformInfo::Concept::~Concept() {}
 
 TargetIRAnalysis::TargetIRAnalysis() : TTICallback(&getDefaultTTI) {}
 
 TargetIRAnalysis::TargetIRAnalysis(

llvm/lib/Target/ARM/ARMTargetTransformInfo.cpp

if (ST->hasBranchPredictor() && L->getNumBlocks() > 4)
     return;
 
   // Don't unroll vectorized loops, including the remainder loop
   if (getBooleanLoopAttribute(L, "llvm.loop.isvectorized"))
     return;
 
   // Scan the loop: don't unroll loops with calls as this could prevent
   // inlining.
-  unsigned Cost = 0;
+  InstructionCost Cost = 0;
   for (auto *BB : L->getBlocks()) {
     for (auto &I : *BB) {
       // Don't unroll vectorised loop. MVE does not benefit from it as much as
       // scalar code.
       if (I.getType()->isVectorTy())
         return;
 
       if (isa<CallInst>(I) || isa<InvokeInst>(I)) {

llvm/lib/Target/Hexagon/HexagonTargetTransformInfo.h

unsigned getCFInstrCost(unsigned Opcode, TTI::TargetCostKind CostKind) {
     return 1;
   }
 
   bool isLegalMaskedStore(Type *DataType, Align Alignment);
   bool isLegalMaskedLoad(Type *DataType, Align Alignment);
 
   /// @}
 
-  int getUserCost(const User *U, ArrayRef<const Value *> Operands,
+  InstructionCost getUserCost(const User *U, ArrayRef<const Value *> Operands,
                               TTI::TargetCostKind CostKind);
 
   // Hexagon specific decision to generate a lookup table.
   bool shouldBuildLookupTables() const;
 };
 
 } // end namespace llvm
 #endif // LLVM_LIB_TARGET_HEXAGON_HEXAGONTARGETTRANSFORMINFO_H

llvm/lib/Target/Hexagon/HexagonTargetTransformInfo.cpp

 unsigned HexagonTTIImpl::getPrefetchDistance() const {
   return ST.getL1PrefetchDistance();
 }
 
 unsigned HexagonTTIImpl::getCacheLineSize() const {
   return ST.getL1CacheLineSize();
 }
 
-int
-HexagonTTIImpl::getUserCost(const User *U,
+InstructionCost HexagonTTIImpl::getUserCost(const User *U,
                                             ArrayRef<const Value *> Operands,
                                             TTI::TargetCostKind CostKind) {
   auto isCastFoldedIntoLoad = [this](const CastInst *CI) -> bool {
     if (!CI->isIntegerCast())
       return false;
     // Only extensions from an integer type shorter than 32-bit to i32
     // can be folded into the load.
     const DataLayout &DL = getDataLayout();
     unsigned SBW = DL.getTypeSizeInBits(CI->getSrcTy());
     unsigned DBW = DL.getTypeSizeInBits(CI->getDestTy());

llvm/lib/Target/PowerPC/PPCTargetTransformInfo.h

int getIntImmCost(const APInt &Imm, Type *Ty,
                     TTI::TargetCostKind CostKind);
 
   int getIntImmCostInst(unsigned Opcode, unsigned Idx, const APInt &Imm,
                         Type *Ty, TTI::TargetCostKind CostKind,
                         Instruction *Inst = nullptr);
   int getIntImmCostIntrin(Intrinsic::ID IID, unsigned Idx, const APInt &Imm,
                           Type *Ty, TTI::TargetCostKind CostKind);
 
-  unsigned getUserCost(const User *U, ArrayRef<const Value *> Operands,
+  InstructionCost getUserCost(const User *U, ArrayRef<const Value *> Operands,
                               TTI::TargetCostKind CostKind);
 
   TTI::PopcntSupportKind getPopcntSupport(unsigned TyWidth);
   bool isHardwareLoopProfitable(Loop *L, ScalarEvolution &SE,
                                 AssumptionCache &AC,
                                 TargetLibraryInfo *LibInfo,
                                 HardwareLoopInfo &HWLoopInfo);
   bool canSaveCmp(Loop *L, BranchInst **BI, ScalarEvolution *SE, LoopInfo *LI,
                   DominatorTree *DT, AssumptionCache *AC,

llvm/lib/Target/PowerPC/PPCTargetTransformInfo.cpp

if (Idx == ImmIdx && Imm.getBitWidth() <= 64) {
 
     if (ShiftedFree && (Imm.getZExtValue() & 0xFFFF) == 0)
       return TTI::TCC_Free;
   }
 
   return PPCTTIImpl::getIntImmCost(Imm, Ty, CostKind);
 }
 
-unsigned
-PPCTTIImpl::getUserCost(const User *U, ArrayRef<const Value *> Operands,
+InstructionCost PPCTTIImpl::getUserCost(const User *U,
+                                        ArrayRef<const Value *> Operands,
                                         TTI::TargetCostKind CostKind) {
   // We already implement getCastInstrCost and getMemoryOpCost where we perform
   // the vector adjustment there.
   if (isa<CastInst>(U) || isa<LoadInst>(U) || isa<StoreInst>(U))
     return BaseT::getUserCost(U, Operands, CostKind);
 
   if (U->getType()->isVectorTy()) {
     // Instructions that need to be split should cost more.
     std::pair<int, MVT> LT = TLI->getTypeLegalizationCost(DL, U->getType());