[CostModel] Remove getExtCost

[samparker]

Summary

This has not been implemented by any backends, which appear to cover the functionality through getCastInstrCost. Sink what there is in the default implementation into BasicTTI.

[RKSimon]

llvm/include/llvm/CodeGen/BasicTTIImpl.h

724

This doesn't look as effective as the getTLI()->isExtLoad call in getExtCost - missing one use checks etc.

[samparker]

llvm/include/llvm/CodeGen/BasicTTIImpl.h

724

Thanks, I hadn't done that intentionally. I've uploaded that change in D78937 because it stops this patch from looking like an NFC. I'm not sure whether the vectorization changes are good/expected.

[samparker]

Ping.

[samparker]

After turning on asserts, two inlining tests were failing for AArch64 and PowerPC. I've now updated their implementations of getCastInstrCost to pass the Instruction to the default implementation. I've also done the same for the ARM backend.

[dmgreen]

It may be better to separate out the "add I to getCastInstrCost" change, which is something I agree with but seems to deserve it's own patch to get the details correct.

llvm/test/Analysis/CostModel/ARM/cast.ll

1669 ↗

(On Diff #261824)

I don't think this should be free for Neon, as far as I understand. It doesn't fold the extend into the load like it would for MVE or scalar.

[samparker]

Yeah, fair enough.

[samparker]

Spun out ARM changes into D79561.

[samparker]

AArch64 change: D79562.

[samparker]

Rebased on top of the arm and aarch64 changes.

[samparker]

Rebased with the new aarch64 tests.

[samparker]

ping.

[spatel]

LGTM

[dmgreen]

llvm/include/llvm/CodeGen/BasicTTIImpl.h

717

I don't believe this will be correct from the vectorizer. It can pass a context instruction that has different types to the final IR due to it truncating at the same time it vectorizes. It is generally unsound to rely on I being accurate.

[samparker]

llvm/include/llvm/CodeGen/BasicTTIImpl.h

717

I'm sure there's going to be many cases like that, and the vectorizer can always choose to not pass the instruction.

[dmgreen]

llvm/include/llvm/CodeGen/BasicTTIImpl.h

717

Hmm. You may be correct there in the end. I was kind of hoping that we could keep the context instruction but only look at opcodes. I don't think even that will remain truly correct for very long though.

I believe that adding this would at least cause inaccuracies in the costmodelling over what we have right now, possibly regressions in some cases. Not passing I through from the vectorizer (and any other place where we couldn't trust the context) would seem to need a different change to me though.

llvm/test/Analysis/CostModel/AArch64/cast.ll

30 ↗

(On Diff #263402)

Also, and I may be forgetting something because I thought these looked fine the last time I looked, but why would a sext i32->i64 be free? Would it not need a sxtw?

[samparker]

llvm/test/Analysis/CostModel/AArch64/cast.ll

30 ↗

(On Diff #263402)

We would, this confused me too. AArch64 looks at all the users of the cast and in the case of no users in returns 0! So it completely breaks these tests. I've haven't given enough context to this patch, because I've also added another cast test with users and that hasn't changed.

[samparker]

I believe that adding this would at least cause inaccuracies in the costmodelling over what we have right now, possibly regressions in some cases.

Don't forget that this is just doing what it did before though and hopefully anyone who is really concerned about performance won't be relying on BasicTTI to get it. There's also D78937 which may help too.

llvm/include/llvm/Analysis/TargetTransformInfo.h

enum TargetCostConstants {
     TCC_Expensive = 4 ///< The cost of a 'div' instruction on x86.
   };
 
   /// Estimate the cost of a GEP operation when lowered.
   int getGEPCost(Type *PointeeType, const Value *Ptr,
                  ArrayRef<const Value *> Operands,
                  TargetCostKind CostKind = TCK_SizeAndLatency) const;
 
-  /// Estimate the cost of a EXT operation when lowered.
-  int getExtCost(const Instruction *I, const Value *Src) const;
-
   /// \returns A value by which our inlining threshold should be multiplied.
   /// This is primarily used to bump up the inlining threshold wholesale on
   /// targets where calls are unusually expensive.
   ///
   /// TODO: This is a rather blunt instrument.  Perhaps altering the costs of
   /// individual classes of instructions would be better.
   unsigned getInliningThresholdMultiplier() const;
 

 
 class TargetTransformInfo::Concept {
 public:
   virtual ~Concept() = 0;
   virtual const DataLayout &getDataLayout() const = 0;
   virtual int getGEPCost(Type *PointeeType, const Value *Ptr,
                          ArrayRef<const Value *> Operands,
                          TTI::TargetCostKind CostKind) = 0;
-  virtual int getExtCost(const Instruction *I, const Value *Src) = 0;
   virtual unsigned getInliningThresholdMultiplier() = 0;
   virtual int getInlinerVectorBonusPercent() = 0;
   virtual int getIntrinsicCost(Intrinsic::ID IID, Type *RetTy,
                                ArrayRef<Type *> ParamTys, const User *U,
                                enum TargetCostKind CostKind) = 0;
   virtual int getIntrinsicCost(Intrinsic::ID IID, Type *RetTy,
                                ArrayRef<const Value *> Arguments,
                                const User *U,
const DataLayout &getDataLayout() const override {
     return Impl.getDataLayout();
   }
 
   int getGEPCost(Type *PointeeType, const Value *Ptr,
                  ArrayRef<const Value *> Operands,
                  enum TargetTransformInfo::TargetCostKind CostKind) override {
     return Impl.getGEPCost(PointeeType, Ptr, Operands);
   }
-  int getExtCost(const Instruction *I, const Value *Src) override {
-    return Impl.getExtCost(I, Src);
-  }
   unsigned getInliningThresholdMultiplier() override {
     return Impl.getInliningThresholdMultiplier();
   }
   int getInlinerVectorBonusPercent() override {
     return Impl.getInlinerVectorBonusPercent();
   }
   int getIntrinsicCost(Intrinsic::ID IID, Type *RetTy,
                        ArrayRef<Type *> ParamTys,

llvm/include/llvm/Analysis/TargetTransformInfoImpl.h

unsigned getEstimatedNumberOfCaseClusters(const SwitchInst &SI,
                                             ProfileSummaryInfo *PSI,
                                             BlockFrequencyInfo *BFI) {
     (void)PSI;
     (void)BFI;
     JTSize = 0;
     return SI.getNumCases();
   }
 
-  int getExtCost(const Instruction *I, const Value *Src) {
-    return TTI::TCC_Basic;
-  }
-
   unsigned getInliningThresholdMultiplier() { return 1; }
 
   int getInlinerVectorBonusPercent() { return 150; }
 
   unsigned getMemcpyCost(const Instruction *I) { return TTI::TCC_Expensive; }
 
   bool hasBranchDivergence() { return false; }
 
case Instruction::Trunc:
       break;
     case Instruction::BitCast:
       if (getCastInstrCost(Opcode, Ty, OpTy, CostKind, I) == TTI::TCC_Free)
         return TTI::TCC_Free;
       break;
     case Instruction::FPExt:
     case Instruction::SExt:
     case Instruction::ZExt:
-      if (I && TargetTTI->getExtCost(I, Operands.back()) == TTI::TCC_Free)
+      if (TargetTTI->getCastInstrCost(Opcode, Ty, OpTy, CostKind, I) == TTI::TCC_Free)
         return TTI::TCC_Free;
       break;
     }
     // By default, just classify everything as 'basic'.
     return TTI::TCC_Basic;
   }
 
   int getInstructionLatency(const Instruction *I) {

llvm/include/llvm/CodeGen/BasicTTIImpl.h

bool isTypeLegal(Type *Ty) {
     return getTLI()->isTypeLegal(VT);
   }
 
   int getGEPCost(Type *PointeeType, const Value *Ptr,
                  ArrayRef<const Value *> Operands) {
     return BaseT::getGEPCost(PointeeType, Ptr, Operands);
   }
 
-  int getExtCost(const Instruction *I, const Value *Src) {
-    if (getTLI()->isExtFree(I))
-      return TargetTransformInfo::TCC_Free;
-
-    if (isa<ZExtInst>(I) || isa<SExtInst>(I))
-      if (const LoadInst *LI = dyn_cast<LoadInst>(Src))
-        if (getTLI()->isExtLoad(LI, I, DL))
-          return TargetTransformInfo::TCC_Free;
-
-    return TargetTransformInfo::TCC_Basic;
-  }
-
   unsigned getIntrinsicCost(Intrinsic::ID IID, Type *RetTy,
                             ArrayRef<const Value *> Arguments, const User *U,
                             TTI::TargetCostKind CostKind) {
     return BaseT::getIntrinsicCost(IID, RetTy, Arguments, U, CostKind);
   }
 
   unsigned getIntrinsicCost(Intrinsic::ID IID, Type *RetTy,
                             ArrayRef<Type *> ParamTys, const User *U,
case Instruction::Trunc:
       if (TLI->isTruncateFree(SrcLT.second, DstLT.second))
         return 0;
       LLVM_FALLTHROUGH;
     case Instruction::BitCast:
       // Bitcast between types that are legalized to the same type are free.
       if (SrcLT.first == DstLT.first && SrcSize == DstSize)
         return 0;
       break;
+    case Instruction::FPExt:
+      if (I && getTLI()->isExtFree(I))
+        return TargetTransformInfo::TCC_Free;
+      break;
     case Instruction::ZExt:
       if (TLI->isZExtFree(SrcLT.second, DstLT.second))
         return 0;
       LLVM_FALLTHROUGH;
     case Instruction::SExt: {
+      if (I && getTLI()->isExtFree(I))
+        return TargetTransformInfo::TCC_Free;
+
       // If this is a zext/sext of a load, return 0 if the corresponding
       // extending load exists on target.
       if (I && isa<LoadInst>(I->getOperand(0))) {
         EVT ExtVT = EVT::getEVT(Dst);
         EVT LoadVT = EVT::getEVT(Src);
         unsigned LType =
           ((Opcode == Instruction::ZExt) ? ISD::ZEXTLOAD : ISD::SEXTLOAD);
         if (TLI->isLoadExtLegal(LType, ExtVT, LoadVT))
           return 0;
       }
       break;
     }
     case Instruction::AddrSpaceCast:
       if (TLI->isFreeAddrSpaceCast(Src->getPointerAddressSpace(),
                                    Dst->getPointerAddressSpace()))
         return 0;

llvm/lib/Analysis/TargetTransformInfo.cpp

 }
 
 int TargetTransformInfo::getGEPCost(Type *PointeeType, const Value *Ptr,
                                     ArrayRef<const Value *> Operands,
                                     TTI::TargetCostKind CostKind) const {
   return TTIImpl->getGEPCost(PointeeType, Ptr, Operands, CostKind);
 }
 
-int TargetTransformInfo::getExtCost(const Instruction *I,
-                                    const Value *Src) const {
-  return TTIImpl->getExtCost(I, Src);
-}
-
 int TargetTransformInfo::getIntrinsicCost(Intrinsic::ID IID, Type *RetTy,
                                           ArrayRef<const Value *> Arguments,
                                           const User *U,
                                           TTI::TargetCostKind CostKind) const {
   int Cost = TTIImpl->getIntrinsicCost(IID, RetTy, Arguments, U, CostKind);
   assert(Cost >= 0 && "TTI should not produce negative costs!");
   return Cost;
 }

llvm/lib/Target/PowerPC/PPCTargetTransformInfo.cpp

                               nullptr);
 }
 
 int PPCTTIImpl::getCastInstrCost(unsigned Opcode, Type *Dst, Type *Src,
                                  TTI::TargetCostKind CostKind,
                                  const Instruction *I) {
   assert(TLI->InstructionOpcodeToISD(Opcode) && "Invalid opcode");
 
-  int Cost = BaseT::getCastInstrCost(Opcode, Dst, Src, CostKind);
+  int Cost = BaseT::getCastInstrCost(Opcode, Dst, Src, CostKind, I);
   return vectorCostAdjustment(Cost, Opcode, Dst, Src);
 }
 
 int PPCTTIImpl::getCmpSelInstrCost(unsigned Opcode, Type *ValTy, Type *CondTy,
                                    TTI::TargetCostKind CostKind,
                                    const Instruction *I) {
   int Cost = BaseT::getCmpSelInstrCost(Opcode, ValTy, CondTy, CostKind, I);
   return vectorCostAdjustment(Cost, Opcode, ValTy, nullptr);