[OpaquePtr] Create API to make a copy of a PointerType with some address space

[aeubanks]

Summary

Some existing places use getPointerElementType() to create a copy of a
pointer type with some new address space.

[dblaikie]

llvm/include/llvm/IR/DerivedTypes.h

667

Maybe the "with same pointee type" isn't the high priority aspect of this API? More "get with different address space" and it's assumed the rest of the type remains the same? (opaque to opaque, typed to typed)

Maybe "getWithAddressSpace" or something like that?

[aeubanks]

llvm/include/llvm/IR/DerivedTypes.h

667

There are already a couple PointerType::get() methods which construct a pointer type. This one is different in that it's sorta cloning an existing PointerType. I want to make that super clear via the name somehow. This could be changed to a non-static method to be clearer? IMO Something like getWithAddressSpace() is too similar to the existing methods.

And this should be removed after the opaque pointer transition is over.

[dblaikie]

llvm/include/llvm/IR/DerivedTypes.h

667

Yeah, happy for this to go away after the opaque pointer transition - when nothing significant from the old pointer type would be used to make the new pointer type anyway (since the only interesting property of the opaque pointer is its address space).

I don't too much mind it being a non-static member - it is different in that it's derived from the current pointer type so it can readily be a non-static member function.

[dblaikie]

Eh, either way - see what you mean about the existing name. And since it'll be relatively short-lived (maybe modify the comment to say "delete this after the opaque pointer transition" rather than the more passive "this is only useful ... " ?) the current name you've got seems OK.

[aeubanks]

add TODO

llvm/include/llvm/IR/DerivedTypes.h

public:
   }
 
   /// This constructs an opaque pointer to an object in the
   /// default address space (address space zero).
   static PointerType *getUnqual(LLVMContext &C) {
     return PointerType::get(C, 0);
   }
 
+  /// This constructs a pointer type with the same pointee type as input
+  /// PointerType (or opaque pointer is the input PointerType is opaque) and the
+  /// given address space. This is only useful during the opaque pointer
+  /// transition.
+  static PointerType *getWithSamePointeeType(PointerType *PT,
+                                             unsigned AddressSpace) {
+    if (PT->isOpaque())
+      return get(PT->getContext(), AddressSpace);
+    return get(PT->getElementType(), AddressSpace);
+  }
+
   Type *getElementType() const {
     assert(!isOpaque() && "Attempting to get element type of opaque pointer");
     return PointeeTy;
   }
 
   bool isOpaque() const { return !PointeeTy; }
 
   /// Return true if the specified type is valid as a element type.

llvm/lib/Analysis/ConstantFolding.cpp

Constant *StripPtrCastKeepAS(Constant *Ptr, Type *&ElemTy,
   }
 
   auto *NewPtrTy = cast<PointerType>(Ptr->getType());
 
   ElemTy = NewPtrTy->getPointerElementType();
 
   // Preserve the address space number of the pointer.
   if (NewPtrTy->getAddressSpace() != OldPtrTy->getAddressSpace()) {
-    NewPtrTy = ElemTy->getPointerTo(OldPtrTy->getAddressSpace());
-    Ptr = ConstantExpr::getPointerCast(Ptr, NewPtrTy);
+    Ptr = ConstantExpr::getPointerCast(
+        Ptr, PointerType::getWithSamePointeeType(NewPtrTy,
+                                                 OldPtrTy->getAddressSpace()));
   }
   return Ptr;
 }
 
 /// If we can symbolically evaluate the GEP constant expression, do so.
 Constant *SymbolicallyEvaluateGEP(const GEPOperator *GEP,
                                   ArrayRef<Constant *> Ops,
                                   const DataLayout &DL,

llvm/lib/Target/AMDGPU/AMDGPUPromoteAlloca.cpp

bool AMDGPUPromoteAllocaImpl::handleAlloca(AllocaInst &I, bool SufficientLDS) {
 
   SmallVector<IntrinsicInst *> DeferredIntrs;
 
   for (Value *V : WorkList) {
     CallInst *Call = dyn_cast<CallInst>(V);
     if (!Call) {
       if (ICmpInst *CI = dyn_cast<ICmpInst>(V)) {
         Value *Src0 = CI->getOperand(0);
-        Type *EltTy = Src0->getType()->getPointerElementType();
-        PointerType *NewTy = PointerType::get(EltTy, AMDGPUAS::LOCAL_ADDRESS);
+        PointerType *NewTy = PointerType::getWithSamePointeeType(
+            cast<PointerType>(Src0->getType()), AMDGPUAS::LOCAL_ADDRESS);
 
         if (isa<ConstantPointerNull>(CI->getOperand(0)))
           CI->setOperand(0, ConstantPointerNull::get(NewTy));
 
         if (isa<ConstantPointerNull>(CI->getOperand(1)))
           CI->setOperand(1, ConstantPointerNull::get(NewTy));
 
         continue;
       }
 
       // The operand's value should be corrected on its own and we don't want to
       // touch the users.
       if (isa<AddrSpaceCastInst>(V))
         continue;
 
-      Type *EltTy = V->getType()->getPointerElementType();
-      PointerType *NewTy = PointerType::get(EltTy, AMDGPUAS::LOCAL_ADDRESS);
+      PointerType *NewTy = PointerType::getWithSamePointeeType(
+          cast<PointerType>(V->getType()), AMDGPUAS::LOCAL_ADDRESS);
 
       // FIXME: It doesn't really make sense to try to do this for all
       // instructions.
       V->mutateType(NewTy);
 
       // Adjust the types of any constant operands.
       if (SelectInst *SI = dyn_cast<SelectInst>(V)) {
         if (isa<ConstantPointerNull>(SI->getOperand(1)))
for (Value *V : WorkList) {
     case Intrinsic::strip_invariant_group:
       Intr->eraseFromParent();
       // FIXME: I think the invariant marker should still theoretically apply,
       // but the intrinsics need to be changed to accept pointers with any
       // address space.
       continue;
     case Intrinsic::objectsize: {
       Value *Src = Intr->getOperand(0);
-      Type *SrcTy = Src->getType()->getPointerElementType();
-      Function *ObjectSize = Intrinsic::getDeclaration(Mod,
-        Intrinsic::objectsize,
-        { Intr->getType(), PointerType::get(SrcTy, AMDGPUAS::LOCAL_ADDRESS) }
-      );
+      Function *ObjectSize = Intrinsic::getDeclaration(
+          Mod, Intrinsic::objectsize,
+          {Intr->getType(),
+           PointerType::getWithSamePointeeType(
+               cast<PointerType>(Src->getType()), AMDGPUAS::LOCAL_ADDRESS)});
 
       CallInst *NewCall = Builder.CreateCall(
           ObjectSize,
           {Src, Intr->getOperand(1), Intr->getOperand(2), Intr->getOperand(3)});
       Intr->replaceAllUsesWith(NewCall);
       Intr->eraseFromParent();
       continue;
     }

llvm/lib/Target/NVPTX/NVPTXLowerArgs.cpp

auto CloneInstInParamAS = [](const IP &I) -> Value * {
     if (auto *GEP = dyn_cast<GetElementPtrInst>(I.OldInstruction)) {
       SmallVector<Value *, 4> Indices(GEP->indices());
       auto *NewGEP = GetElementPtrInst::Create(nullptr, I.NewParam, Indices,
                                                GEP->getName(), GEP);
       NewGEP->setIsInBounds(GEP->isInBounds());
       return NewGEP;
     }
     if (auto *BC = dyn_cast<BitCastInst>(I.OldInstruction)) {
-      auto *NewBCType = BC->getType()->getPointerElementType()->getPointerTo(
-          ADDRESS_SPACE_PARAM);
+      auto *NewBCType = PointerType::getWithSamePointeeType(
+          cast<PointerType>(BC->getType()), ADDRESS_SPACE_PARAM);
       return BitCastInst::Create(BC->getOpcode(), I.NewParam, NewBCType,
                                  BC->getName(), BC);
     }
     if (auto *ASC = dyn_cast<AddrSpaceCastInst>(I.OldInstruction)) {
       assert(ASC->getDestAddressSpace() == ADDRESS_SPACE_PARAM);
       (void)ASC;
       // Just pass through the argument, the old ASC is no longer needed.
       return I.NewParam;
void NVPTXLowerArgs::markPointerAsGlobal(Value *Ptr) {
   } else {
     // Insert right after Ptr if Ptr is an instruction.
     InsertPt = ++cast<Instruction>(Ptr)->getIterator();
     assert(InsertPt != InsertPt->getParent()->end() &&
            "We don't call this function with Ptr being a terminator.");
   }
 
   Instruction *PtrInGlobal = new AddrSpaceCastInst(
-      Ptr, PointerType::get(Ptr->getType()->getPointerElementType(),
+      Ptr,
+      PointerType::getWithSamePointeeType(cast<PointerType>(Ptr->getType()),
                                           ADDRESS_SPACE_GLOBAL),
       Ptr->getName(), &*InsertPt);
   Value *PtrInGeneric = new AddrSpaceCastInst(PtrInGlobal, Ptr->getType(),
                                               Ptr->getName(), &*InsertPt);
   // Replace with PtrInGeneric all uses of Ptr except PtrInGlobal.
   Ptr->replaceAllUsesWith(PtrInGeneric);
   PtrInGlobal->setOperand(0, Ptr);
 }
 

llvm/unittests/IR/TypesTest.cpp

 TEST(TypesTest, LayoutIdenticalEmptyStructs) {
   LLVMContext C;
 
   StructType *Foo = StructType::create(C, "Foo");
   StructType *Bar = StructType::create(C, "Bar");
   EXPECT_TRUE(Foo->isLayoutIdentical(Bar));
 }
 
+TEST(TypesTest, CopyPointerType) {
+  LLVMContext C;
+
+  PointerType *P1 = PointerType::get(C, 1);
+  EXPECT_TRUE(P1->isOpaque());
+  PointerType *P1C = PointerType::getWithSamePointeeType(P1, 1);
+  EXPECT_EQ(P1, P1C);
+  EXPECT_TRUE(P1C->isOpaque());
+  PointerType *P1C0 = PointerType::getWithSamePointeeType(P1, 0);
+  EXPECT_NE(P1, P1C0);
+  EXPECT_TRUE(P1C0->isOpaque());
+
+  Type *Int8 = Type::getInt8Ty(C);
+  PointerType *P2 = PointerType::get(Int8, 1);
+  EXPECT_FALSE(P2->isOpaque());
+  PointerType *P2C = PointerType::getWithSamePointeeType(P2, 1);
+  EXPECT_EQ(P2, P2C);
+  EXPECT_FALSE(P2C->isOpaque());
+  PointerType *P2C0 = PointerType::getWithSamePointeeType(P2, 0);
+  EXPECT_NE(P2, P2C0);
+  EXPECT_FALSE(P2C0->isOpaque());
+}
+
 }  // end anonymous namespace