[TargetTransformInfo] assert on nullptr

[nickdesaulniers]

Summary

This was flagged in https://www.viva64.com/en/b/0629/ under "Snippet No.
38".

Add an assertion, since it's unlikely that this parameter is nullptr.

[fhahn]

I am pretty sure from the uses of getGEPCost that Ptr should never be null. It is also used without check later on. I think it would be better to drop the if (Ptr != nullptr) check.

[RKSimon]

In D62229#1511765, @fhahn wrote:

I am pretty sure from the uses of getGEPCost that Ptr should never be null. It is also used without check later on. I think it would be better to drop the if (Ptr != nullptr) check.

If we're going to do that we need to add an assertion instead

[nickdesaulniers]

• prefer assertion to release build checks

[nickdesaulniers]

• git-clang-format HEAD~

[RKSimon]

LGTM with a couple of optional minor cleanups

llvm/include/llvm/Analysis/TargetTransformInfoImpl.h

684 ↗

(On Diff #202532)

assert for a non-null PointeeType as well ?

690 ↗

(On Diff #202532)

auto *BaseGV = dyn_cast<GlobalValue>(Ptr->stripPointerCasts());

[fhahn]

Thanks for updating, LGTM, with Simon's suggestions.

[nickdesaulniers]

• implement @RKSimon's suggestions

[nickdesaulniers]

This comment was removed by nickdesaulniers.

[RKSimon]

cheers @nickdesaulniers LGTM again

llvm/trunk/include/llvm/Analysis/TargetTransformInfoImpl.h

unsigned getCallCost(const Function *F, ArrayRef<const Value *> Arguments,
     // will constant fold with these arguments.
     return static_cast<T *>(this)->getCallCost(F, Arguments.size(), U);
   }
 
   using BaseT::getGEPCost;
 
   int getGEPCost(Type *PointeeType, const Value *Ptr,
                  ArrayRef<const Value *> Operands) {
-    const GlobalValue *BaseGV = nullptr;
-    if (Ptr != nullptr) {
-      // TODO: will remove this when pointers have an opaque type.
-      assert(Ptr->getType()->getScalarType()->getPointerElementType() ==
-                 PointeeType &&
-             "explicit pointee type doesn't match operand's pointee type");
-      BaseGV = dyn_cast<GlobalValue>(Ptr->stripPointerCasts());
-    }
+    assert(PointeeType && Ptr && "can't get GEPCost of nullptr");
+    auto *BaseGV = dyn_cast<GlobalValue>(Ptr->stripPointerCasts());
     bool HasBaseReg = (BaseGV == nullptr);
 
     auto PtrSizeBits = DL.getPointerTypeSizeInBits(Ptr->getType());
     APInt BaseOffset(PtrSizeBits, 0);
     int64_t Scale = 0;
 
     auto GTI = gep_type_begin(PointeeType, Operands);
     Type *TargetType = nullptr;
for (auto I = Operands.begin(); I != Operands.end(); ++I, ++GTI) {
           if (Scale != 0)
             // No addressing mode takes two scale registers.
             return TTI::TCC_Basic;
           Scale = ElementSize;
         }
       }
     }
 
-    // Assumes the address space is 0 when Ptr is nullptr.
-    unsigned AS =
-        (Ptr == nullptr ? 0 : Ptr->getType()->getPointerAddressSpace());
-
     if (static_cast<T *>(this)->isLegalAddressingMode(
             TargetType, const_cast<GlobalValue *>(BaseGV),
-            BaseOffset.sextOrTrunc(64).getSExtValue(), HasBaseReg, Scale, AS))
+            BaseOffset.sextOrTrunc(64).getSExtValue(), HasBaseReg, Scale,
+            Ptr->getType()->getPointerAddressSpace()))
       return TTI::TCC_Free;
     return TTI::TCC_Basic;
   }
 
   unsigned getIntrinsicCost(Intrinsic::ID IID, Type *RetTy,
                             ArrayRef<Type *> ParamTys, const User *U) {
     switch (IID) {
     default: