[PATCH] D103465: [OpaquePtr] Track pointee types in Clang

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

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.



================
Comment at: clang/lib/CodeGen/Address.h:29-30
 public:
   Address(llvm::Value *pointer, CharUnits alignment)
-      : Pointer(pointer), Alignment(alignment) {
+      : Address(pointer, nullptr, alignment) {}
+  Address(llvm::Value *pointer, llvm::Type *PointeeType, CharUnits alignment)
----------------
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"))?


================
Comment at: clang/lib/CodeGen/Address.h:56-57
   ///
   /// 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 PointeeType; }
----------------
hey, someone wrote a handy comment here - could possibly delete this comment now that that reality has come to pass


================
Comment at: clang/lib/CodeGen/CGExpr.cpp:159
   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(),
----------------
Could potentially pull these sort of changes out as a separate patch/commit without pre-commit review, since they rely on the existing API.


================
Comment at: clang/lib/CodeGen/CGValue.h:230-231
 private:
   void Initialize(QualType Type, Qualifiers Quals, CharUnits Alignment,
                   LValueBaseInfo BaseInfo, TBAAAccessInfo TBAAInfo) {
     assert((!Alignment.isZero() || Type->isIncompleteType()) &&
----------------
Maybe this could use some assertions added to check the PointeeType is correct?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103465/new/

https://reviews.llvm.org/D103465