[TTI] Change new getVectorInstrCost overload to use const reference after D131114

[MaskRay]

Summary

A const reference is preferred over a non-null const pointer.
Type * is kept as is to match the other overload.

[mingmingl]

Thanks for the change! Mostly LGTM.

For my information, any particular reason that a const reference is preferred over a pointer here?
It's true that raw pointers makes ownership unclear, yet they are not uncommon in the codebase. https://llvm.org/docs/ProgrammersManual.html doesn't seem to mention this either.

[mingmingl]

A small heads-up, i'm having D128302 but I could wait for const reference change if we decide to proceed with this.

[davidxl]

lgtm

llvm/include/llvm/Analysis/TargetTransformInfo.h

InstructionCost getVectorInstrCost(unsigned Opcode, Type *Val,
                                      unsigned Index = -1) const;
 
   /// \return The expected cost of vector Insert and Extract.
   /// This is used when instruction is available, and implementation
   /// asserts 'I' is not nullptr.
   ///
   /// A typical suitable use case is cost estimation when vector instruction
   /// exists (e.g., from basic blocks during transformation).
-  InstructionCost getVectorInstrCost(const Instruction *I, Type *Val,
+  InstructionCost getVectorInstrCost(const Instruction &I, Type *Val,
                                      unsigned Index = -1) const;
 
   /// \return The cost of replication shuffle of \p VF elements typed \p EltTy
   /// \p ReplicationFactor times.
   ///
   /// For example, the mask for \p ReplicationFactor=3 and \p VF=4 is:
   ///   <0,0,0,1,1,1,2,2,2,3,3,3>
   InstructionCost getReplicationShuffleCost(Type *EltTy, int ReplicationFactor,
virtual InstructionCost getCFInstrCost(unsigned Opcode,
                                          const Instruction *I = nullptr) = 0;
   virtual InstructionCost getCmpSelInstrCost(unsigned Opcode, Type *ValTy,
                                              Type *CondTy,
                                              CmpInst::Predicate VecPred,
                                              TTI::TargetCostKind CostKind,
                                              const Instruction *I) = 0;
   virtual InstructionCost getVectorInstrCost(unsigned Opcode, Type *Val,
                                              unsigned Index) = 0;
-  virtual InstructionCost getVectorInstrCost(const Instruction *I, Type *Val,
+  virtual InstructionCost getVectorInstrCost(const Instruction &I, Type *Val,
                                              unsigned Index) = 0;
 
   virtual InstructionCost
   getReplicationShuffleCost(Type *EltTy, int ReplicationFactor, int VF,
                             const APInt &DemandedDstElts,
                             TTI::TargetCostKind CostKind) = 0;
 
   virtual InstructionCost getMemoryOpCost(unsigned Opcode, Type *Src,
InstructionCost getCmpSelInstrCost(unsigned Opcode, Type *ValTy, Type *CondTy,
                                      TTI::TargetCostKind CostKind,
                                      const Instruction *I) override {
     return Impl.getCmpSelInstrCost(Opcode, ValTy, CondTy, VecPred, CostKind, I);
   }
   InstructionCost getVectorInstrCost(unsigned Opcode, Type *Val,
                                      unsigned Index) override {
     return Impl.getVectorInstrCost(Opcode, Val, Index);
   }
-  InstructionCost getVectorInstrCost(const Instruction *I, Type *Val,
+  InstructionCost getVectorInstrCost(const Instruction &I, Type *Val,
                                      unsigned Index) override {
     return Impl.getVectorInstrCost(I, Val, Index);
   }
   InstructionCost
   getReplicationShuffleCost(Type *EltTy, int ReplicationFactor, int VF,
                             const APInt &DemandedDstElts,
                             TTI::TargetCostKind CostKind) override {
     return Impl.getReplicationShuffleCost(EltTy, ReplicationFactor, VF,

llvm/include/llvm/Analysis/TargetTransformInfoImpl.h

InstructionCost getCmpSelInstrCost(unsigned Opcode, Type *ValTy, Type *CondTy,
     return 1;
   }
 
   InstructionCost getVectorInstrCost(unsigned Opcode, Type *Val,
                                      unsigned Index) const {
     return 1;
   }
 
-  InstructionCost getVectorInstrCost(const Instruction *I, Type *Val,
+  InstructionCost getVectorInstrCost(const Instruction &I, Type *Val,
                                      unsigned Index) const {
     return 1;
   }
 
   unsigned getReplicationShuffleCost(Type *EltTy, int ReplicationFactor, int VF,
                                      const APInt &DemandedDstElts,
                                      TTI::TargetCostKind CostKind) {
     return 1;
InstructionCost getUserCost(const User *U, ArrayRef<const Value *> Operands,
     case Instruction::InsertElement: {
       auto *IE = dyn_cast<InsertElementInst>(U);
       if (!IE)
         return TTI::TCC_Basic; // FIXME
       unsigned Idx = -1;
       if (auto *CI = dyn_cast<ConstantInt>(IE->getOperand(2)))
         if (CI->getValue().getActiveBits() <= 32)
           Idx = CI->getZExtValue();
-      return TargetTTI->getVectorInstrCost(IE, Ty, Idx);
+      return TargetTTI->getVectorInstrCost(*IE, Ty, Idx);
     }
     case Instruction::ShuffleVector: {
       auto *Shuffle = dyn_cast<ShuffleVectorInst>(U);
       if (!Shuffle)
         return TTI::TCC_Basic; // FIXME
 
       auto *VecTy = cast<VectorType>(U->getType());
       auto *VecSrcTy = cast<VectorType>(U->getOperand(0)->getType());
case Instruction::ExtractElement: {
       auto *EEI = dyn_cast<ExtractElementInst>(U);
       if (!EEI)
         return TTI::TCC_Basic; // FIXME
       unsigned Idx = -1;
       if (auto *CI = dyn_cast<ConstantInt>(EEI->getOperand(1)))
         if (CI->getValue().getActiveBits() <= 32)
           Idx = CI->getZExtValue();
       Type *DstTy = U->getOperand(0)->getType();
-      return TargetTTI->getVectorInstrCost(EEI, DstTy, Idx);
+      return TargetTTI->getVectorInstrCost(*EEI, DstTy, Idx);
     }
     }
     // By default, just classify everything as 'basic'.
     return TTI::TCC_Basic;
   }
 
   InstructionCost getInstructionLatency(const Instruction *I) {
     SmallVector<const Value *, 4> Operands(I->operand_values());

llvm/include/llvm/CodeGen/BasicTTIImpl.h

public:
   InstructionCost getVectorInstrCost(unsigned Opcode, Type *Val,
                                      unsigned Index) {
     std::pair<InstructionCost, MVT> LT =
         getTLI()->getTypeLegalizationCost(DL, Val->getScalarType());
 
     return LT.first;
   }
 
-  InstructionCost getVectorInstrCost(const Instruction *I, Type *Val,
+  InstructionCost getVectorInstrCost(const Instruction &I, Type *Val,
                                      unsigned Index) {
-    return thisT()->getVectorInstrCost(I->getOpcode(), Val, Index);
+    return thisT()->getVectorInstrCost(I.getOpcode(), Val, Index);
   }
 
   InstructionCost getReplicationShuffleCost(Type *EltTy, int ReplicationFactor,
                                             int VF,
                                             const APInt &DemandedDstElts,
                                             TTI::TargetCostKind CostKind) {
     assert(DemandedDstElts.getBitWidth() == (unsigned)VF * ReplicationFactor &&
            "Unexpected size of DemandedDstElts.");

llvm/lib/Analysis/TargetTransformInfo.cpp

InstructionCost TargetTransformInfo::getVectorInstrCost(unsigned Opcode,
   // FIXME: Assert that Opcode is either InsertElement or ExtractElement.
   // This is mentioned in the interface description and respected by all
   // callers, but never asserted upon.
   InstructionCost Cost = TTIImpl->getVectorInstrCost(Opcode, Val, Index);
   assert(Cost >= 0 && "TTI should not produce negative costs!");
   return Cost;
 }
 
-InstructionCost TargetTransformInfo::getVectorInstrCost(const Instruction *I,
+InstructionCost TargetTransformInfo::getVectorInstrCost(const Instruction &I,
                                                         Type *Val,
                                                         unsigned Index) const {
-  assert((I != nullptr) && "Expect not-null instruction pointer");
   // FIXME: Assert that Opcode is either InsertElement or ExtractElement.
   // This is mentioned in the interface description and respected by all
   // callers, but never asserted upon.
   InstructionCost Cost = TTIImpl->getVectorInstrCost(I, Val, Index);
   assert(Cost >= 0 && "TTI should not produce negative costs!");
   return Cost;
 }
 

llvm/lib/CodeGen/CodeGenPrepare.cpp

if (!TLI.allowsMisalignedMemoryAccesses(
       // the extract with the store.
       return false;
     }
 
     // The scalar chain of computation has to pay for the transition
     // scalar to vector.
     // The vector chain has to account for the combining cost.
     InstructionCost ScalarCost =
-        TTI.getVectorInstrCost(Transition, PromotedType, Index);
+        TTI.getVectorInstrCost(*Transition, PromotedType, Index);
     InstructionCost VectorCost = StoreExtractCombineCost;
     enum TargetTransformInfo::TargetCostKind CostKind =
       TargetTransformInfo::TCK_RecipThroughput;
     for (const auto &Inst : InstsToBePromoted) {
       // Compute the cost.
       // By construction, all instructions being promoted are arithmetic ones.
       // Moreover, one argument is a constant that can be viewed as a splat
       // constant.

llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp

for (auto *V : VL) {
                                               EE->getVectorOperandType(), Idx);
           // Add back the cost of s|zext which is subtracted separately.
           Cost += TTIRef.getCastInstrCost(
               Ext->getOpcode(), Ext->getType(), EE->getType(),
               TTI::getCastContextHint(Ext), CostKind, Ext);
           continue;
         }
       }
-      Cost -= TTIRef.getVectorInstrCost(EE, EE->getVectorOperandType(), Idx);
+      Cost -= TTIRef.getVectorInstrCost(*EE, EE->getVectorOperandType(), Idx);
     }
     // Add a cost for subvector extracts/inserts if required.
     for (const auto &Data : ExtractVectorsTys) {
       auto *EEVTy = cast<FixedVectorType>(Data.first->getType());
       unsigned NumElts = VecTy->getNumElements();
       if (Data.second % NumElts == 0)
         continue;
       if (TTIRef.getNumberOfParts(EEVTy) > TTIRef.getNumberOfParts(VecTy)) {
case Instruction::ExtractElement: {
       // The common cost of removal ExtractElement/ExtractValue instructions +
       // the cost of shuffles, if required to resuffle the original vector.
       if (NeedToShuffleReuses) {
         unsigned Idx = 0;
         for (unsigned I : E->ReuseShuffleIndices) {
           if (ShuffleOrOp == Instruction::ExtractElement) {
             auto *EE = cast<ExtractElementInst>(VL[I]);
             CommonCost -= TTI->getVectorInstrCost(
-                EE, EE->getVectorOperandType(), *getExtractIndex(EE));
+                *EE, EE->getVectorOperandType(), *getExtractIndex(EE));
           } else {
             CommonCost -= TTI->getVectorInstrCost(Instruction::ExtractElement,
                                                   VecTy, Idx);
             ++Idx;
           }
         }
         Idx = EntryVF;
         for (Value *V : VL) {
           if (ShuffleOrOp == Instruction::ExtractElement) {
             auto *EE = cast<ExtractElementInst>(V);
             CommonCost += TTI->getVectorInstrCost(
-                EE, EE->getVectorOperandType(), *getExtractIndex(EE));
+                *EE, EE->getVectorOperandType(), *getExtractIndex(EE));
           } else {
             --Idx;
             CommonCost += TTI->getVectorInstrCost(Instruction::ExtractElement,
                                                   VecTy, Idx);
           }
         }
       }
       if (ShuffleOrOp == Instruction::ExtractValue) {

llvm/lib/Transforms/Vectorize/VectorCombine.cpp

ExtractElementInst *VectorCombine::getShuffleExtract(
   unsigned Index1 = Index1C->getZExtValue();
 
   // If the extract indexes are identical, no shuffle is needed.
   if (Index0 == Index1)
     return nullptr;
 
   Type *VecTy = Ext0->getVectorOperand()->getType();
   assert(VecTy == Ext1->getVectorOperand()->getType() && "Need matching types");
-  InstructionCost Cost0 = TTI.getVectorInstrCost(Ext0, VecTy, Index0);
-  InstructionCost Cost1 = TTI.getVectorInstrCost(Ext1, VecTy, Index1);
+  InstructionCost Cost0 = TTI.getVectorInstrCost(*Ext0, VecTy, Index0);
+  InstructionCost Cost1 = TTI.getVectorInstrCost(*Ext1, VecTy, Index1);
 
   // If both costs are invalid no shuffle is needed
   if (!Cost0.isValid() && !Cost1.isValid())
     return nullptr;
 
   // We are extracting from 2 different indexes, so one operand must be shuffled
   // before performing a vector operation and/or extract. The more expensive
   // extract will be replaced by a shuffle.
VectorOpCost = TTI.getCmpSelInstrCost(
         Opcode, VecTy, CmpInst::makeCmpResultType(VecTy), Pred);
   }
 
   // Get cost estimates for the extract elements. These costs will factor into
   // both sequences.
   unsigned Ext0Index = Ext0IndexC->getZExtValue();
   unsigned Ext1Index = Ext1IndexC->getZExtValue();
 
-  InstructionCost Extract0Cost = TTI.getVectorInstrCost(Ext0, VecTy, Ext0Index);
-  InstructionCost Extract1Cost = TTI.getVectorInstrCost(Ext1, VecTy, Ext1Index);
+  InstructionCost Extract0Cost =
+      TTI.getVectorInstrCost(*Ext0, VecTy, Ext0Index);
+  InstructionCost Extract1Cost =
+      TTI.getVectorInstrCost(*Ext1, VecTy, Ext1Index);
 
   // A more expensive extract will always be replaced by a splat shuffle.
   // For example, if Ext0 is more expensive:
   // opcode (extelt V0, Ext0), (ext V1, Ext1) -->
   // extelt (opcode (splat V0, Ext0), V1), Ext1
   // TODO: Evaluate whether that always results in lowest cost. Alternatively,
   //       check the cost of creating a broadcast shuffle and shuffling both
   //       operands to element 0.
bool VectorCombine::foldExtractedCmps(Instruction &I) {
   // binop i1 (cmp Pred (ext X, Index0), C0), (cmp Pred (ext X, Index1), C1)
   CmpInst::Predicate Pred = P0;
   unsigned CmpOpcode = CmpInst::isFPPredicate(Pred) ? Instruction::FCmp
                                                     : Instruction::ICmp;
   auto *VecTy = dyn_cast<FixedVectorType>(X->getType());
   if (!VecTy)
     return false;
 
-  InstructionCost OldCost = TTI.getVectorInstrCost(Ext0, VecTy, Index0);
-  OldCost += TTI.getVectorInstrCost(Ext1, VecTy, Index1);
+  InstructionCost OldCost = TTI.getVectorInstrCost(*Ext0, VecTy, Index0);
+  OldCost += TTI.getVectorInstrCost(*Ext1, VecTy, Index1);
   OldCost +=
       TTI.getCmpSelInstrCost(CmpOpcode, I0->getType(),
                              CmpInst::makeCmpResultType(I0->getType()), Pred) *
       2;
   OldCost += TTI.getArithmeticInstrCost(I.getOpcode(), I.getType());
 
   // The proposed vector pattern is:
   // vcmp = cmp Pred X, VecC
   // ext (binop vNi1 vcmp, (shuffle vcmp, Index1)), Index0
   int CheapIndex = ConvertToShuf == Ext0 ? Index1 : Index0;
   int ExpensiveIndex = ConvertToShuf == Ext0 ? Index0 : Index1;
   auto *CmpTy = cast<FixedVectorType>(CmpInst::makeCmpResultType(X->getType()));
   InstructionCost NewCost = TTI.getCmpSelInstrCost(
       CmpOpcode, X->getType(), CmpInst::makeCmpResultType(X->getType()), Pred);
   SmallVector<int, 32> ShufMask(VecTy->getNumElements(), UndefMaskElem);
   ShufMask[CheapIndex] = ExpensiveIndex;
   NewCost += TTI.getShuffleCost(TargetTransformInfo::SK_PermuteSingleSrc, CmpTy,
                                 ShufMask);
   NewCost += TTI.getArithmeticInstrCost(I.getOpcode(), CmpTy);
-  NewCost += TTI.getVectorInstrCost(Ext0, CmpTy, CheapIndex);
+  NewCost += TTI.getVectorInstrCost(*Ext0, CmpTy, CheapIndex);
 
   // Aggressively form vector ops if the cost is equal because the transform
   // may enable further optimization.
   // Codegen can reverse this transform (scalarize) if it was not profitable.
   if (OldCost < NewCost || !NewCost.isValid())
     return false;
 
   // Create a vector constant from the 2 scalar constants.