Use DL.getIndexType() in Value::getPointerAlignment()

[arichardson]

Summary

In the out-of-tree CHERI and Arm Morello backends pointers are 128 or 64
bits, but only 64/32 bits of those hold the address information (our
DataLayout strings use "-pf200:128:128:128:64"/"-pf200:64:64:64:32").
I recently merged https://reviews.llvm.org/D73131 and started seeing
assertions in various tests due to this i8 addrspace(200)* -> i128
ptrtoint.

[nikic]

It's not really clear why using the index type for this would be correct. The index type is used for GEP index and casting a pointer to the index type doesn't make a whole lot of sense to me. Do you have any LangRef wording or other usages that would show that this is a sensible thing to do?

[lebedev.ri]

In D99660#2661435, @nikic wrote:

It's not really clear why using the index type for this would be correct. The index type is used for GEP index and casting a pointer to the index type doesn't make a whole lot of sense to me. Do you have any LangRef wording or other usages that would show that this is a sensible thing to do?

+1, i'm not convinced that index type is the right fix here.
I would think that you need to either unbreak the invariant that decltype(sizeof(ptr))==intptrtype
or make that addressspace non-integral, thus preventing int<->ptr casts.

[arichardson]

In D99660#2661453, @lebedev.ri wrote:

In D99660#2661435, @nikic wrote:

It's not really clear why using the index type for this would be correct. The index type is used for GEP index and casting a pointer to the index type doesn't make a whole lot of sense to me. Do you have any LangRef wording or other usages that would show that this is a sensible thing to do?

+1, i'm not convinced that index type is the right fix here.
I would think that you need to either unbreak the invariant that decltype(sizeof(ptr))==intptrtype
or make that addressspace non-integral, thus preventing int<->ptr casts.

We use something similar to non-integral pointers, but our fork pre-dates that work so we use DL.isFatPointer() instead.
I have been meaning make use of the non-integral pointer work for a long time but it's too strict for our purposes:
For CHERI pointers, inttoptr should be avoided, but ptrtoint is perfectly fine (you just get the address part and not the additional metadata).

My understanding of D42123 was that the index width is the number of bits that are affected by pointer arithmetic (i.e. the range of the pointer, 64 bits for our 128-bit capabilities).
If you disagree with that interpretation I guess I can to add another DataLayout field to pointer types that specifies the address range?

[arichardson]

Use new APIs

[lebedev.ri]

Can you use this new API in some more places?
I guess this looks fine as a cleanup.

[lebedev.ri]

In D99660#4049580, @lebedev.ri wrote:

Can you use this new API in some more places?
I guess this looks fine as a cleanup.

(marking as reviewed)

llvm/include/llvm/IR/DataLayout.h

public:
   /// Returns an integer type with size at least as big as that of a
   /// pointer in the given address space.
   IntegerType *getIntPtrType(LLVMContext &C, unsigned AddressSpace = 0) const;
 
   /// Returns an integer (vector of integer) type with size at least as
   /// big as that of a pointer of the given pointer (vector of pointer) type.
   Type *getIntPtrType(Type *) const;
 
+
+  /// Returns an integer type with size equal to the integral range of a pointer
+  /// in the given address space. This can be used to obtain an integer that
+  /// can be used for the type of a ptrtoint instruction.
+  IntegerType *getPtrToIntResultType(LLVMContext &C,
+                                     unsigned AddressSpace) const;
+
+  /// Returns an integer (or vector of integer) type with size at least as
+  /// big as that of the pointer range for the given pointer (or vector of
+  /// pointers). This can be used to obtain an integer (or vector of integer)
+  /// that can be used for the type of a ptrtoint instruction.
+  Type *getPtrToIntResultType(Type *) const;
+
   /// Returns the smallest integer type with size at least as big as
   /// Width bits.
   Type *getSmallestLegalIntType(LLVMContext &C, unsigned Width = 0) const;
 
   /// Returns the largest legal integer type, or null if none are set.
   Type *getLargestLegalIntType(LLVMContext &C) const {
     unsigned LargestSize = getLargestLegalIntTypeSizeInBits();
     return (LargestSize == 0) ? nullptr : Type::getIntNTy(C, LargestSize);

llvm/lib/IR/DataLayout.cpp

assert(Ty->isPtrOrPtrVectorTy() &&
          "Expected a pointer or pointer vector type.");
   unsigned NumBits = getPointerTypeSizeInBits(Ty);
   IntegerType *IntTy = IntegerType::get(Ty->getContext(), NumBits);
   if (VectorType *VecTy = dyn_cast<VectorType>(Ty))
     return VectorType::get(IntTy, VecTy);
   return IntTy;
 }
 
+IntegerType *DataLayout::getPtrToIntResultType(LLVMContext &C,
+                                               unsigned AddressSpace) const {
+  return IntegerType::get(C, getPointerIntegralSizeInBits(AddressSpace));
+}
+
+Type *DataLayout::getPtrToIntResultType(Type *Ty) const {
+  assert(Ty->isPtrOrPtrVectorTy() &&
+         "Expected a pointer or pointer vector type.");
+  unsigned NumBits =
+      getPointerIntegralSizeInBits(cast<PointerType>(Ty)->getAddressSpace());
+  IntegerType *IntTy = IntegerType::get(Ty->getContext(), NumBits);
+  if (VectorType *VecTy = dyn_cast<VectorType>(Ty))
+    return VectorType::get(IntTy, VecTy);
+  return IntTy;
+}
+
 Type *DataLayout::getSmallestLegalIntType(LLVMContext &C, unsigned Width) const {
   for (unsigned LegalIntWidth : LegalIntWidths)
     if (Width <= LegalIntWidth)
       return Type::getIntNTy(C, LegalIntWidth);
   return nullptr;
 }
 
 unsigned DataLayout::getLargestLegalIntTypeSizeInBits() const {

llvm/lib/IR/Value.cpp

if (MDNode *MD = LI->getMetadata(LLVMContext::MD_align)) {
       ConstantInt *CI = mdconst::extract<ConstantInt>(MD->getOperand(0));
       return Align(CI->getLimitedValue());
     }
   } else if (auto *CstPtr = dyn_cast<Constant>(this)) {
     // Strip pointer casts to avoid creating unnecessary ptrtoint expression
     // if the only "reduction" is combining a bitcast + ptrtoint.
     CstPtr = CstPtr->stripPointerCasts();
     if (auto *CstInt = dyn_cast_or_null<ConstantInt>(ConstantExpr::getPtrToInt(
-            const_cast<Constant *>(CstPtr), DL.getIntPtrType(getType()),
+            const_cast<Constant *>(CstPtr), DL.getPtrToIntResultType(getType()),
             /*OnlyIfReduced=*/true))) {
       size_t TrailingZeros = CstInt->getValue().countTrailingZeros();
       // While the actual alignment may be large, elsewhere we have
       // an arbitrary upper alignmet limit, so let's clamp to it.
       return Align(TrailingZeros < Value::MaxAlignmentExponent
                        ? uint64_t(1) << TrailingZeros
                        : Value::MaximumAlignment);
     }