[CodeGen] Replace of PointerType::get(Type) with opaque version (NFC) #124771

junlarsen · 2025-01-28T15:45:31Z

Follow-up to #123569

junlarsen · 2025-01-28T15:46:41Z

clang/lib/CodeGen/CGBlocks.cpp

-  llvm::Type *UnsignedLongTy =
-    getTypes().ConvertType(getContext().UnsignedLongTy);
-
-  // struct __block_descriptor {
-  //   unsigned long reserved;
-  //   unsigned long block_size;
-  //
-  //   // later, the following will be added
-  //
-  //   struct {
-  //     void (*copyHelper)();
-  //     void (*copyHelper)();
-  //   } helpers;                // !!! optional
-  //
-  //   const char *signature;   // the block signature
-  //   const char *layout;      // reserved
-  // };


I do feel a bit guilty about removing this comment. However, I'm not experienced enough to know if it's significant and something we'd like to keep?

Perhaps move it somewhere?

I think it's okay to drop it, essentially the same comment also exists on buildBlockDescriptor.

llvmbot · 2025-01-28T15:51:31Z

@llvm/pr-subscribers-clang-codegen

@llvm/pr-subscribers-clang

Author: Mats Jun Larsen (junlarsen)

Changes

Follow-up to #123569

Full diff: https://github.com/llvm/llvm-project/pull/124771.diff

7 Files Affected:

(modified) clang/lib/CodeGen/Address.h (+4-4)
(modified) clang/lib/CodeGen/CGBlocks.cpp (+1-22)
(modified) clang/lib/CodeGen/CGDecl.cpp (+1-4)
(modified) clang/lib/CodeGen/CGDeclCXX.cpp (+2-2)
(modified) clang/lib/CodeGen/CGExpr.cpp (+2-2)
(modified) clang/lib/CodeGen/CGObjCMac.cpp (+1-1)
(modified) clang/lib/CodeGen/CodeGenModule.cpp (+3-3)

diff --git a/clang/lib/CodeGen/Address.h b/clang/lib/CodeGen/Address.h
index a18c7169af1eb9..852aa0e686fe3c 100644
--- a/clang/lib/CodeGen/Address.h
+++ b/clang/lib/CodeGen/Address.h
@@ -19,6 +19,7 @@
 #include "clang/AST/Type.h"
 #include "llvm/ADT/PointerIntPair.h"
 #include "llvm/IR/Constants.h"
+#include "llvm/IR/DerivedTypes.h"
 #include "llvm/Support/MathExtras.h"
 
 namespace clang {
@@ -197,10 +198,9 @@ class Address {
 
   /// Return the type of the pointer value.
   llvm::PointerType *getType() const {
-    return llvm::PointerType::get(
-        ElementType,
-        llvm::cast<llvm::PointerType>(Pointer.getPointer()->getType())
-            ->getAddressSpace());
+    auto AS = llvm::cast<llvm::PointerType>(Pointer.getPointer()->getType())
+                  ->getAddressSpace();
+    return llvm::PointerType::get(ElementType->getContext(), AS);
   }
 
   /// Return the type of the values stored in this address.
diff --git a/clang/lib/CodeGen/CGBlocks.cpp b/clang/lib/CodeGen/CGBlocks.cpp
index a7584a95c8ca7b..033528dbceb3bd 100644
--- a/clang/lib/CodeGen/CGBlocks.cpp
+++ b/clang/lib/CodeGen/CGBlocks.cpp
@@ -1097,31 +1097,10 @@ llvm::Type *CodeGenModule::getBlockDescriptorType() {
   if (BlockDescriptorType)
     return BlockDescriptorType;
 
-  llvm::Type *UnsignedLongTy =
-    getTypes().ConvertType(getContext().UnsignedLongTy);
-
-  // struct __block_descriptor {
-  //   unsigned long reserved;
-  //   unsigned long block_size;
-  //
-  //   // later, the following will be added
-  //
-  //   struct {
-  //     void (*copyHelper)();
-  //     void (*copyHelper)();
-  //   } helpers;                // !!! optional
-  //
-  //   const char *signature;   // the block signature
-  //   const char *layout;      // reserved
-  // };
-  BlockDescriptorType = llvm::StructType::create(
-      "struct.__block_descriptor", UnsignedLongTy, UnsignedLongTy);
-
-  // Now form a pointer to that.
   unsigned AddrSpace = 0;
   if (getLangOpts().OpenCL)
     AddrSpace = getContext().getTargetAddressSpace(LangAS::opencl_constant);
-  BlockDescriptorType = llvm::PointerType::get(BlockDescriptorType, AddrSpace);
+  BlockDescriptorType = llvm::PointerType::get(getLLVMContext(), AddrSpace);
   return BlockDescriptorType;
 }
 
diff --git a/clang/lib/CodeGen/CGDecl.cpp b/clang/lib/CodeGen/CGDecl.cpp
index ded905cdcc9f44..feb4ddbb52b2b7 100644
--- a/clang/lib/CodeGen/CGDecl.cpp
+++ b/clang/lib/CodeGen/CGDecl.cpp
@@ -2868,15 +2868,12 @@ void CodeGenModule::EmitOMPAllocateDecl(const OMPAllocateDecl *D) {
 
     // We can also keep the existing global if the address space is what we
     // expect it to be, if not, it is replaced.
-    QualType ASTTy = VD->getType();
     clang::LangAS GVAS = GetGlobalVarAddressSpace(VD);
     auto TargetAS = getContext().getTargetAddressSpace(GVAS);
     if (Entry->getType()->getAddressSpace() == TargetAS)
       continue;
 
-    // Make a new global with the correct type / address space.
-    llvm::Type *Ty = getTypes().ConvertTypeForMem(ASTTy);
-    llvm::PointerType *PTy = llvm::PointerType::get(Ty, TargetAS);
+    llvm::PointerType *PTy = llvm::PointerType::get(getLLVMContext(), TargetAS);
 
     // Replace all uses of the old global with a cast. Since we mutate the type
     // in place we neeed an intermediate that takes the spot of the old entry
diff --git a/clang/lib/CodeGen/CGDeclCXX.cpp b/clang/lib/CodeGen/CGDeclCXX.cpp
index 1c2fecea1a6ac2..5005f6b3cbd2d1 100644
--- a/clang/lib/CodeGen/CGDeclCXX.cpp
+++ b/clang/lib/CodeGen/CGDeclCXX.cpp
@@ -347,7 +347,7 @@ void CodeGenFunction::registerGlobalDtorWithAtExit(llvm::Constant *dtorStub) {
   // extern "C" int atexit(void (*f)(void));
   assert(dtorStub->getType() ==
              llvm::PointerType::get(
-                 llvm::FunctionType::get(CGM.VoidTy, false),
+                 CGM.getLLVMContext(),
                  dtorStub->getType()->getPointerAddressSpace()) &&
          "Argument to atexit has a wrong type.");
 
@@ -374,7 +374,7 @@ CodeGenFunction::unregisterGlobalDtorWithUnAtExit(llvm::Constant *dtorStub) {
   // extern "C" int unatexit(void (*f)(void));
   assert(dtorStub->getType() ==
              llvm::PointerType::get(
-                 llvm::FunctionType::get(CGM.VoidTy, false),
+                 CGM.getLLVMContext(),
                  dtorStub->getType()->getPointerAddressSpace()) &&
          "Argument to unatexit has a wrong type.");
 
diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index 9676e61cf322d9..d3b1468f21b953 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -872,7 +872,7 @@ void CodeGenFunction::EmitTypeCheck(TypeCheckKind TCK, SourceLocation Loc,
       llvm::Value *TypeHash =
           llvm::ConstantInt::get(Int64Ty, xxh3_64bits(Out.str()));
 
-      llvm::Type *VPtrTy = llvm::PointerType::get(IntPtrTy, 0);
+      llvm::Type *VPtrTy = llvm::PointerType::get(getLLVMContext(), 0);
       Address VPtrAddr(Ptr, IntPtrTy, getPointerAlign());
       llvm::Value *VPtrVal = GetVTablePtr(VPtrAddr, VPtrTy,
                                           Ty->getAsCXXRecordDecl(),
@@ -3054,7 +3054,7 @@ LValue CodeGenFunction::EmitDeclRefLValue(const DeclRefExpr *E) {
                                            getContext().getDeclAlign(VD));
         llvm::Type *VarTy = getTypes().ConvertTypeForMem(VD->getType());
         auto *PTy = llvm::PointerType::get(
-            VarTy, getTypes().getTargetAddressSpace(VD->getType()));
+            getLLVMContext(), getTypes().getTargetAddressSpace(VD->getType()));
         Addr = Builder.CreatePointerBitCastOrAddrSpaceCast(Addr, PTy, VarTy);
       } else {
         // Should we be using the alignment of the constant pointer we emitted?
diff --git a/clang/lib/CodeGen/CGObjCMac.cpp b/clang/lib/CodeGen/CGObjCMac.cpp
index dd900f9b32fb78..6c929a6431c0f0 100644
--- a/clang/lib/CodeGen/CGObjCMac.cpp
+++ b/clang/lib/CodeGen/CGObjCMac.cpp
@@ -5717,7 +5717,7 @@ ObjCCommonTypesHelper::ObjCCommonTypesHelper(CodeGen::CodeGenModule &cgm)
   IntTy = CGM.IntTy;
   LongTy = cast<llvm::IntegerType>(Types.ConvertType(Ctx.LongTy));
   Int8PtrTy = CGM.Int8PtrTy;
-  Int8PtrProgramASTy = llvm::PointerType::get(CGM.Int8Ty, ProgramAS);
+  Int8PtrProgramASTy = llvm::PointerType::get(CGM.getLLVMContext(), ProgramAS);
   Int8PtrPtrTy = CGM.Int8PtrPtrTy;
 
   // arm64 targets use "int" ivar offset variables. All others,
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index eb8d3ceeeba4c0..2c51de5bf7da16 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -4424,7 +4424,7 @@ void CodeGenModule::emitCPUDispatchDefinition(GlobalDecl GD) {
   GlobalDecl ResolverGD;
   if (getTarget().supportsIFunc()) {
     ResolverType = llvm::FunctionType::get(
-        llvm::PointerType::get(DeclTy,
+        llvm::PointerType::get(getLLVMContext(),
                                getTypes().getTargetAddressSpace(FD->getType())),
         false);
   }
@@ -4596,8 +4596,8 @@ llvm::Constant *CodeGenModule::GetOrCreateMultiVersionResolver(GlobalDecl GD) {
   // cpu_dispatch will be emitted in this translation unit.
   if (ShouldReturnIFunc) {
     unsigned AS = getTypes().getTargetAddressSpace(FD->getType());
-    llvm::Type *ResolverType =
-        llvm::FunctionType::get(llvm::PointerType::get(DeclTy, AS), false);
+    llvm::Type *ResolverType = llvm::FunctionType::get(
+        llvm::PointerType::get(getLLVMContext(), AS), false);
     llvm::Constant *Resolver = GetOrCreateLLVMFunction(
         MangledName + ".resolver", ResolverType, GlobalDecl{},
         /*ForVTable=*/false);

Follow-up to llvm#123569

github-actions · 2025-02-07T16:48:57Z

✅ With the latest revision this PR passed the C/C++ code formatter.

junlarsen · 2025-02-07T16:59:47Z

clang/lib/CodeGen/CGExpr.cpp

        auto *PTy = llvm::PointerType::get(
-            VarTy, getTypes().getTargetAddressSpace(VD->getType()));
+            getLLVMContext(), getTypes().getTargetAddressSpace(VD->getType()));
        Addr = Builder.CreatePointerBitCastOrAddrSpaceCast(Addr, PTy, VarTy);


Can the below cast be replaced with an AddrSpaceCast only? i.e CGBuilder::CreateAddrSpaceCast? I'm not too sure how the Clang codegen Address code behaves.

clang/lib/CodeGen/Address.h

nikic · 2025-02-08T08:44:58Z

clang/lib/CodeGen/CGBlocks.cpp

-  llvm::Type *UnsignedLongTy =
-    getTypes().ConvertType(getContext().UnsignedLongTy);
-
-  // struct __block_descriptor {
-  //   unsigned long reserved;
-  //   unsigned long block_size;
-  //
-  //   // later, the following will be added
-  //
-  //   struct {
-  //     void (*copyHelper)();
-  //     void (*copyHelper)();
-  //   } helpers;                // !!! optional
-  //
-  //   const char *signature;   // the block signature
-  //   const char *layout;      // reserved
-  // };


I think it's okay to drop it, essentially the same comment also exists on buildBlockDescriptor.

clang/lib/CodeGen/CGDeclCXX.cpp

nikic

LGTM

clang/lib/CodeGen/Address.h

…llvm#124771) Follow-up to llvm#123569

junlarsen commented Jan 28, 2025

View reviewed changes

junlarsen marked this pull request as ready for review January 28, 2025 15:50

llvmbot added clang Clang issues not falling into any other category clang:codegen IR generation bugs: mangling, exceptions, etc. labels Jan 28, 2025

junlarsen marked this pull request as draft January 28, 2025 16:12

junlarsen added 2 commits February 8, 2025 01:40

[CodeGen] Replace of PointerType::get(Type) with opaque version (NFC)

13d65a0

Follow-up to llvm#123569

Remove unnecessary pointer bitcast

9a21d42

junlarsen force-pushed the jun/clang-replace-uses-of-pointertype-get branch from 31752fb to 9a21d42 Compare February 7, 2025 16:45

Run clang format

ff0399f

junlarsen commented Feb 7, 2025

View reviewed changes

junlarsen marked this pull request as ready for review February 7, 2025 17:00

nikic reviewed Feb 8, 2025

View reviewed changes

Cast directly and simplify assertions

3dbdefb

nikic approved these changes Feb 8, 2025

View reviewed changes

clang/lib/CodeGen/Address.h Outdated Show resolved Hide resolved

Remove inserted includes

adf2362

junlarsen force-pushed the jun/clang-replace-uses-of-pointertype-get branch from 1934f7c to adf2362 Compare February 8, 2025 14:56

junlarsen merged commit e0fee55 into llvm:main Feb 8, 2025
5 of 8 checks passed

junlarsen deleted the jun/clang-replace-uses-of-pointertype-get branch February 8, 2025 15:13

Icohedron pushed a commit to Icohedron/llvm-project that referenced this pull request Feb 11, 2025

[CodeGen] Replace of PointerType::get(Type) with opaque version (NFC) (…

7351913

…llvm#124771) Follow-up to llvm#123569

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[CodeGen] Replace of PointerType::get(Type) with opaque version (NFC) #124771

[CodeGen] Replace of PointerType::get(Type) with opaque version (NFC) #124771

junlarsen commented Jan 28, 2025

Uh oh!

junlarsen Jan 28, 2025 •

edited

Loading

Uh oh!

nikic Feb 8, 2025

Uh oh!

llvmbot commented Jan 28, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Feb 7, 2025 •

edited

Loading

Uh oh!

junlarsen Feb 7, 2025 •

edited

Loading

Uh oh!

Uh oh!

nikic Feb 8, 2025

Uh oh!

Uh oh!

nikic left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

[CodeGen] Replace of PointerType::get(Type) with opaque version (NFC) #124771

[CodeGen] Replace of PointerType::get(Type) with opaque version (NFC) #124771

Conversation

junlarsen commented Jan 28, 2025

Uh oh!

junlarsen Jan 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nikic Feb 8, 2025

Choose a reason for hiding this comment

Uh oh!

llvmbot commented Jan 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Feb 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

junlarsen Feb 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nikic Feb 8, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nikic left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

junlarsen Jan 28, 2025 •

edited

Loading

llvmbot commented Jan 28, 2025 •

edited

Loading

github-actions bot commented Feb 7, 2025 •

edited

Loading

junlarsen Feb 7, 2025 •

edited

Loading