[OpaquePtr] Track pointee types in Clang

[aeubanks]

Summary

With pointee types going away in llvm::PointerType, the frontend needs
to keep track of pointee types.

Add a PointeeType to Address and LValue.

[aeubanks]

This is definitely WIP, but I'd like to make sure that this is the right direction before continuing. And are there any other obvious places that also require this?

[dblaikie]

The changes look like the right direction to me - though I don't know/couldn't confirm whether more changes will be needed in other places.

clang/lib/CodeGen/Address.h

29–30

At some point will this include an assertion that 'pointer' isn't a PointerType? I guess some uses of PointerTyped values won't need to know their pointee type?

(or are all values in Address PointerTyped? (owing to them being "addresses"))?

56–57

hey, someone wrote a handy comment here - could possibly delete this comment now that that reality has come to pass

clang/lib/CodeGen/CGExpr.cpp

159

Could potentially pull these sort of changes out as a separate patch/commit without pre-commit review, since they rely on the existing API.

clang/lib/CodeGen/CGValue.h

230–231

Maybe this could use some assertions added to check the PointeeType is correct?

[nikic]

Maybe send a mail to cfe-dev to solicit some help with the opaque pointer migration on the clang side?

clang/lib/CodeGen/Address.h

29–30

Based on the unconditional cast in getType(), I'd assume that addresses are always pointer typed.

[craig.topper]

clang/lib/CodeGen/Address.h

31

Is PointeeType expected to be non-null when pointer is non-null?

58

Should this assert isValid() since it no longer goes through getType() and getPointer() which would have asserted previously?

[erichkeane]

clang/lib/CodeGen/Address.h

26

I think this will still end up being a problem when we try to look into the type for multi-dimension pointers. Should we have this just store the QualType of the value instead, so we can unpack it later? If we ever needed the llvm::Type of the Pointee, a convertTypeForMem is all that is standing in our way.

[jyknight]

clang/lib/CodeGen/Address.h

26

Do we do that? I didn't think Address was used that way.

29–30

This constructor seems odd. We shouldn't ever construct Address with a non-null pointer and a null PointeeType, should we?

31

That would be my expectation.

58

+1.

clang/lib/CodeGen/CGValue.h

230–231

I think Initialize should be modified to take Addr instead of Alignment, and move

R.V = Addr.getPointer();
R.PointeeType = Addr.getElementType();

into this function, from all the callers.

If that's done, I don't think a separate assert is useful.

clang/lib/CodeGen/Address.h

 #include "clang/AST/CharUnits.h"
 
 namespace clang {
 namespace CodeGen {
 
 /// An aligned address.
 class Address {
   llvm::Value *Pointer;
+  llvm::Type *PointeeType;
   CharUnits Alignment;
 public:
   Address(llvm::Value *pointer, CharUnits alignment)
-      : Pointer(pointer), Alignment(alignment) {
+      : Address(pointer, nullptr, alignment) {}
+  Address(llvm::Value *pointer, llvm::Type *PointeeType, CharUnits alignment)
+      : Pointer(pointer), PointeeType(PointeeType), Alignment(alignment) {
     assert((!alignment.isZero() || pointer == nullptr) &&
            "creating valid address with invalid alignment");
+    assert((pointer == nullptr ||
+            llvm::cast<llvm::PointerType>(pointer->getType())
+                ->isOpaqueOrPointeeTypeMatches(PointeeType)) &&
+           "pointee type should match pointer type's pointee type");
   }
 
   static Address invalid() { return Address(nullptr, CharUnits()); }
   bool isValid() const { return Pointer != nullptr; }
 
   llvm::Value *getPointer() const {
     assert(isValid());
     return Pointer;
   }
 
   /// Return the type of the pointer value.
   llvm::PointerType *getType() const {
     return llvm::cast<llvm::PointerType>(getPointer()->getType());
   }
 
   /// Return the type of the values stored in this address.
   ///
   /// When IR pointer types lose their element type, we should simply
   /// store it in Address instead for the convenience of writing code.
-  llvm::Type *getElementType() const {
-    return getType()->getElementType();
-  }
+  llvm::Type *getElementType() const { return PointeeType; }
 
   /// Return the address space that this address resides in.
   unsigned getAddressSpace() const {
     return getType()->getAddressSpace();
   }
 
   /// Return the IR name of the pointer value.
   llvm::StringRef getName() const {

clang/lib/CodeGen/CGBlocks.cpp

 
 /// Initialize the structural components of a __block variable, i.e.
 /// everything but the actual object.
 void CodeGenFunction::emitByrefStructureInit(const AutoVarEmission &emission) {
   // Find the address of the local.
   Address addr = emission.Addr;
 
   // That's an alloca of the byref structure type.
-  llvm::StructType *byrefType = cast<llvm::StructType>(
-    cast<llvm::PointerType>(addr.getPointer()->getType())->getElementType());
+  llvm::StructType *byrefType = cast<llvm::StructType>(addr.getElementType());
 
   unsigned nextHeaderIndex = 0;
   CharUnits nextHeaderOffset;
   auto storeHeaderField = [&](llvm::Value *value, CharUnits fieldSize,
                               const Twine &name) {
     auto fieldAddr = Builder.CreateStructGEP(addr, nextHeaderIndex, name);
     Builder.CreateStore(value, fieldAddr);
 

clang/lib/CodeGen/CGExpr.cpp

 /// CreateTempAlloca - This creates a alloca and inserts it into the entry
 /// block.
 Address CodeGenFunction::CreateTempAllocaWithoutCast(llvm::Type *Ty,
                                                      CharUnits Align,
                                                      const Twine &Name,
                                                      llvm::Value *ArraySize) {
   auto Alloca = CreateTempAlloca(Ty, Name, ArraySize);
   Alloca->setAlignment(Align.getAsAlign());
-  return Address(Alloca, Align);
+  return Address(Alloca, Ty, Align);
 }
 
 /// CreateTempAlloca - This creates a alloca and inserts it into the entry
 /// block. The alloca is casted to default address space if necessary.
 Address CodeGenFunction::CreateTempAlloca(llvm::Type *Ty, CharUnits Align,
                                           const Twine &Name,
                                           llvm::Value *ArraySize,
                                           Address *AllocaAddr) {
if (getASTAllocaAddressSpace() != LangAS::Default) {
     // builder.
     if (!ArraySize)
       Builder.SetInsertPoint(AllocaInsertPt);
     V = getTargetHooks().performAddrSpaceCast(
         *this, V, getASTAllocaAddressSpace(), LangAS::Default,
         Ty->getPointerTo(DestAddrSpace), /*non-null*/ true);
   }
 
-  return Address(V, Align);
+  return Address(V, Ty, Align);
 }
 
 /// CreateTempAlloca - This creates an alloca and inserts it into the entry
 /// block if \p ArraySize is nullptr, otherwise inserts it at the current
 /// insertion point of the builder.
 llvm::AllocaInst *CodeGenFunction::CreateTempAlloca(llvm::Type *Ty,
                                                     const Twine &Name,
                                                     llvm::Value *ArraySize) {

 }
 
 Address CodeGenFunction::CreateMemTemp(QualType Ty, CharUnits Align,
                                        const Twine &Name, Address *Alloca) {
   Address Result = CreateTempAlloca(ConvertTypeForMem(Ty), Align, Name,
                                     /*ArraySize=*/nullptr, Alloca);
 
   if (Ty->isConstantMatrixType()) {
-    auto *ArrayTy = cast<llvm::ArrayType>(Result.getType()->getElementType());
+    auto *ArrayTy = cast<llvm::ArrayType>(Result.getElementType());
     auto *VectorTy = llvm::FixedVectorType::get(ArrayTy->getElementType(),
                                                 ArrayTy->getNumElements());
 
     Result = Address(
         Builder.CreateBitCast(Result.getPointer(), VectorTy->getPointerTo()),
         Result.getAlignment());
   }
   return Result;
llvm::Value *CodeGenFunction::EmitFromMemory(llvm::Value *Value, QualType Ty) {
 
   return Value;
 }
 
 // Convert the pointer of \p Addr to a pointer to a vector (the value type of
 // MatrixType), if it points to a array (the memory type of MatrixType).
 static Address MaybeConvertMatrixAddress(Address Addr, CodeGenFunction &CGF,
                                          bool IsVector = true) {
-  auto *ArrayTy = dyn_cast<llvm::ArrayType>(
-      cast<llvm::PointerType>(Addr.getPointer()->getType())->getElementType());
+  auto *ArrayTy = dyn_cast<llvm::ArrayType>(Addr.getElementType());
   if (ArrayTy && IsVector) {
     auto *VectorTy = llvm::FixedVectorType::get(ArrayTy->getElementType(),
                                                 ArrayTy->getNumElements());
 
     return Address(CGF.Builder.CreateElementBitCast(Addr, VectorTy));
   }
   auto *VectorTy = dyn_cast<llvm::VectorType>(
-      cast<llvm::PointerType>(Addr.getPointer()->getType())->getElementType());
+      cast<llvm::PointerType>(Addr.getElementType()));
   if (VectorTy && !IsVector) {
     auto *ArrayTy = llvm::ArrayType::get(
         VectorTy->getElementType(),
         cast<llvm::FixedVectorType>(VectorTy)->getNumElements());
 
     return Address(CGF.Builder.CreateElementBitCast(Addr, ArrayTy));
   }
 

clang/lib/CodeGen/CGValue.h

enum {
     VectorElt,    // This is a vector element l-value (V[i]), use getVector*
     BitField,     // This is a bitfield l-value, use getBitfield*.
     ExtVectorElt, // This is an extended vector subset, use getExtVectorComp
     GlobalReg,    // This is a register l-value, use getGlobalReg()
     MatrixElt     // This is a matrix element, use getVector*
   } LVType;
 
   llvm::Value *V;
+  llvm::Type *PointeeType;
 
   union {
     // Index into a vector subscript: V[i]
     llvm::Value *VectorIdx;
 
     // ExtVector element subset: V.xyx
     llvm::Constant *VectorElts;
 
class LValue {
   bool Nontemporal : 1;
 
   LValueBaseInfo BaseInfo;
   TBAAAccessInfo TBAAInfo;
 
   Expr *BaseIvarExp;
 
 private:
   void Initialize(QualType Type, Qualifiers Quals, CharUnits Alignment,
                   LValueBaseInfo BaseInfo, TBAAAccessInfo TBAAInfo) {
     assert((!Alignment.isZero() || Type->isIncompleteType()) &&
            "initializing l-value with zero alignment!");
     this->Type = Type;
     this->Quals = Quals;
     const unsigned MaxAlign = 1U << 31;
     this->Alignment = Alignment.getQuantity() <= MaxAlign
                           ? Alignment.getQuantity()
                           : MaxAlign;
public:
   void setBaseInfo(LValueBaseInfo Info) { BaseInfo = Info; }
 
   // simple lvalue
   llvm::Value *getPointer(CodeGenFunction &CGF) const {
     assert(isSimple());
     return V;
   }
   Address getAddress(CodeGenFunction &CGF) const {
-    return Address(getPointer(CGF), getAlignment());
+    return Address(getPointer(CGF), PointeeType, getAlignment());
   }
   void setAddress(Address address) {
     assert(isSimple());
     V = address.getPointer();
     Alignment = address.getAlignment().getQuantity();
   }
 
   // vector elt lvalue
static LValue MakeAddr(Address address, QualType type, ASTContext &Context,
                          LValueBaseInfo BaseInfo, TBAAAccessInfo TBAAInfo) {
     Qualifiers qs = type.getQualifiers();
     qs.setObjCGCAttr(Context.getObjCGCAttrKind(type));
 
     LValue R;
     R.LVType = Simple;
     assert(address.getPointer()->getType()->isPointerTy());
     R.V = address.getPointer();
+    R.PointeeType = address.getElementType();
     R.Initialize(type, qs, address.getAlignment(), BaseInfo, TBAAInfo);
     return R;
   }
 
   static LValue MakeVectorElt(Address vecAddress, llvm::Value *Idx,
                               QualType type, LValueBaseInfo BaseInfo,
                               TBAAAccessInfo TBAAInfo) {
     LValue R;
     R.LVType = VectorElt;
     R.V = vecAddress.getPointer();
+    R.PointeeType = vecAddress.getElementType();
     R.VectorIdx = Idx;
     R.Initialize(type, type.getQualifiers(), vecAddress.getAlignment(),
                  BaseInfo, TBAAInfo);
     return R;
   }
 
   static LValue MakeExtVectorElt(Address vecAddress, llvm::Constant *Elts,
                                  QualType type, LValueBaseInfo BaseInfo,
                                  TBAAAccessInfo TBAAInfo) {
     LValue R;
     R.LVType = ExtVectorElt;
     R.V = vecAddress.getPointer();
+    R.PointeeType = vecAddress.getElementType();
     R.VectorElts = Elts;
     R.Initialize(type, type.getQualifiers(), vecAddress.getAlignment(),
                  BaseInfo, TBAAInfo);
     return R;
   }
 
   /// Create a new object to represent a bit-field access.
   ///
   /// \param Addr - The base address of the bit-field sequence this
   /// bit-field refers to.
   /// \param Info - The information describing how to perform the bit-field
   /// access.
   static LValue MakeBitfield(Address Addr, const CGBitFieldInfo &Info,
                              QualType type, LValueBaseInfo BaseInfo,
                              TBAAAccessInfo TBAAInfo) {
     LValue R;
     R.LVType = BitField;
     R.V = Addr.getPointer();
+    R.PointeeType = Addr.getElementType();
     R.BitFieldInfo = &Info;
     R.Initialize(type, type.getQualifiers(), Addr.getAlignment(), BaseInfo,
                  TBAAInfo);
     return R;
   }
 
   static LValue MakeGlobalReg(Address Reg, QualType type) {
     LValue R;
     R.LVType = GlobalReg;
     R.V = Reg.getPointer();
+    R.PointeeType = Reg.getElementType();
     R.Initialize(type, type.getQualifiers(), Reg.getAlignment(),
                  LValueBaseInfo(AlignmentSource::Decl), TBAAAccessInfo());
     return R;
   }
 
   static LValue MakeMatrixElt(Address matAddress, llvm::Value *Idx,
                               QualType type, LValueBaseInfo BaseInfo,
                               TBAAAccessInfo TBAAInfo) {
     LValue R;
     R.LVType = MatrixElt;
     R.V = matAddress.getPointer();
+    R.PointeeType = matAddress.getElementType();
     R.VectorIdx = Idx;
     R.Initialize(type, type.getQualifiers(), matAddress.getAlignment(),
                  BaseInfo, TBAAInfo);
     return R;
   }
 
   RValue asAggregateRValue(CodeGenFunction &CGF) const {
     return RValue::getAggregate(getAddress(CGF), isVolatileQualified());