[PATCH] D138296: [clang] Avoid duplicating ProgramAddressSpace in TargetInfo. NFCI

2022-12-01 Thread Alexander Richardson via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rGf3a17d059509: [clang] Avoid duplicating ProgramAddressSpace 
in TargetInfo. NFCI (authored by arichardson).

Changed prior to commit:
  https://reviews.llvm.org/D138296?vs=476473=479387#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138296

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/Basic/TargetInfo.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/Basic/TargetInfo.cpp
  clang/lib/Basic/Targets/AVR.h
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGDeclCXX.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenTypes.cpp
  clang/lib/CodeGen/CodeGenTypes.h

Index: clang/lib/CodeGen/CodeGenTypes.h
===
--- clang/lib/CodeGen/CodeGenTypes.h
+++ clang/lib/CodeGen/CodeGenTypes.h
@@ -305,7 +305,7 @@
   bool isRecordBeingLaidOut(const Type *Ty) const {
 return RecordsBeingLaidOut.count(Ty);
   }
-
+  unsigned getTargetAddressSpace(QualType T) const;
 };
 
 }  // end namespace CodeGen
Index: clang/lib/CodeGen/CodeGenTypes.cpp
===
--- clang/lib/CodeGen/CodeGenTypes.cpp
+++ clang/lib/CodeGen/CodeGenTypes.cpp
@@ -655,7 +655,7 @@
 const ReferenceType *RTy = cast(Ty);
 QualType ETy = RTy->getPointeeType();
 llvm::Type *PointeeType = ConvertTypeForMem(ETy);
-unsigned AS = Context.getTargetAddressSpace(ETy);
+unsigned AS = getTargetAddressSpace(ETy);
 ResultType = llvm::PointerType::get(PointeeType, AS);
 break;
   }
@@ -665,7 +665,7 @@
 llvm::Type *PointeeType = ConvertTypeForMem(ETy);
 if (PointeeType->isVoidTy())
   PointeeType = llvm::Type::getInt8Ty(getLLVMContext());
-unsigned AS = Context.getTargetAddressSpace(ETy);
+unsigned AS = getTargetAddressSpace(ETy);
 ResultType = llvm::PointerType::get(PointeeType, AS);
 break;
   }
@@ -958,3 +958,13 @@
 bool CodeGenTypes::isZeroInitializable(const RecordDecl *RD) {
   return getCGRecordLayout(RD).isZeroInitializable();
 }
+
+unsigned CodeGenTypes::getTargetAddressSpace(QualType T) const {
+  // Return the address space for the type. If the type is a
+  // function type without an address space qualifier, the
+  // program address space is used. Otherwise, the target picks
+  // the best address space based on the type information
+  return T->isFunctionType() && !T.hasAddressSpace()
+ ? getDataLayout().getProgramAddressSpace()
+ : getContext().getTargetAddressSpace(T.getAddressSpace());
+}
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -3196,7 +3196,7 @@
   // See if there is already something with the target's name in the module.
   llvm::GlobalValue *Entry = GetGlobalValue(AA->getAliasee());
   if (Entry) {
-unsigned AS = getContext().getTargetAddressSpace(VD->getType());
+unsigned AS = getTypes().getTargetAddressSpace(VD->getType());
 auto Ptr = llvm::ConstantExpr::getBitCast(Entry, DeclTy->getPointerTo(AS));
 return ConstantAddress(Ptr, DeclTy, Alignment);
   }
@@ -3761,7 +3761,7 @@
   if (getTarget().supportsIFunc()) {
 ResolverType = llvm::FunctionType::get(
 llvm::PointerType::get(DeclTy,
-   Context.getTargetAddressSpace(FD->getType())),
+   getTypes().getTargetAddressSpace(FD->getType())),
 false);
   }
   else {
@@ -3899,8 +3899,8 @@
   // cpu_dispatch will be emitted in this translation unit.
   if (getTarget().supportsIFunc() && !FD->isCPUSpecificMultiVersion()) {
 llvm::Type *ResolverType = llvm::FunctionType::get(
-llvm::PointerType::get(
-DeclTy, getContext().getTargetAddressSpace(FD->getType())),
+llvm::PointerType::get(DeclTy,
+   getTypes().getTargetAddressSpace(FD->getType())),
 false);
 llvm::Constant *Resolver = GetOrCreateLLVMFunction(
 MangledName + ".resolver", ResolverType, GlobalDecl{},
Index: clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
===
--- clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
+++ clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
@@ -3133,7 +3133,7 @@
   const Type *NonQualTy = QC.strip(NativeParamType);
   QualType NativePointeeTy = cast(NonQualTy)->getPointeeType();
   unsigned NativePointeeAddrSpace =
-  CGF.getContext().getTargetAddressSpace(NativePointeeTy);
+  CGF.getTypes().getTargetAddressSpace(NativePointeeTy);
   QualType TargetTy = TargetParam->getType();
   llvm::Value *TargetAddr = CGF.EmitLoadOfScalar(
   LocalAddr,

[PATCH] D138296: [clang] Avoid duplicating ProgramAddressSpace in TargetInfo. NFCI

2022-12-01 Thread John McCall via Phabricator via cfe-commits 
rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.

I think this is fine.  Most of the patch is changing calls to 
`getTargetAddressSpace` to be internal to IRGen, which, as mentioned, I think 
is the right move.

I do think that if we're going to support multiple program address spaces 
(which seems to be a goal) that we'll eventually want an AST-level concept of 
the default program address space, but perpetuating the use of target ASes at 
the AST level isn't the right way to approach that, so this is still the right 
first step.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138296

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D138296: [clang] Avoid duplicating ProgramAddressSpace in TargetInfo. NFCI

2022-12-01 Thread Alexander Richardson via Phabricator via cfe-commits 
arichardson added a comment.

In D138296#3937599 , @rjmccall wrote:

> In D138296#3937486 , @arichardson 
> wrote:
>
>> In D138296#3937224 , @eandrews 
>> wrote:
>>
>>> Functionally this looks ok to me. However I am not sure if CodeGenTypes is 
>>> the 'right' place for this function to live, considering that other 
>>> functions with similar functionality are in ASTContext - including 
>>> overloads of getTargetAddressSpace( ). @erichkeane @aaron.ballman could you 
>>> please take a look?
>>
>> My view is that the parts that interact with LLVM IR should really live in 
>> CodeGen/ and not Basic/ or AST/. I will see how difficult it would be to 
>> move the remaining target (LLVM IR) address space handling code to CodeGen/
>
> Yeah, I don't think there's a good reason for some of the address-space stuff 
> that currently lives in Basic to be there instead of in CodeGen.  Basic/AST 
> need to understand what address spaces exist, their sizes, and what 
> relationships they have with each other, and that's it.

Thanks for looking at this - does this mean you are happy for me to commit this 
change?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138296

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D138296: [clang] Avoid duplicating ProgramAddressSpace in TargetInfo. NFCI

2022-11-18 Thread John McCall via Phabricator via cfe-commits 
rjmccall added a comment.

In D138296#3937486 , @arichardson 
wrote:

> In D138296#3937224 , @eandrews 
> wrote:
>
>> Functionally this looks ok to me. However I am not sure if CodeGenTypes is 
>> the 'right' place for this function to live, considering that other 
>> functions with similar functionality are in ASTContext - including overloads 
>> of getTargetAddressSpace( ). @erichkeane @aaron.ballman could you please 
>> take a look?
>
> My view is that the parts that interact with LLVM IR should really live in 
> CodeGen/ and not Basic/ or AST/. I will see how difficult it would be to move 
> the remaining target (LLVM IR) address space handling code to CodeGen/

Yeah, I don't think there's a good reason for some of the address-space stuff 
that currently lives in Basic to be there instead of in CodeGen.  Basic/AST 
need to understand what address spaces exist, their sizes, and what 
relationships they have with each other, and that's it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138296

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D138296: [clang] Avoid duplicating ProgramAddressSpace in TargetInfo. NFCI

2022-11-18 Thread Alexander Richardson via Phabricator via cfe-commits 
arichardson added a comment.

In D138296#3937224 , @eandrews wrote:

> Functionally this looks ok to me. However I am not sure if CodeGenTypes is 
> the 'right' place for this function to live, considering that other functions 
> with similar functionality are in ASTContext - including overloads of 
> getTargetAddressSpace( ). @erichkeane @aaron.ballman could you please take a 
> look?

My view is that the parts that interact with LLVM IR should really live in 
CodeGen/ and not Basic/ or AST/. I will see how difficult it would be to move 
the remaining target (LLVM IR) address space handling code to CodeGen/


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138296

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D138296: [clang] Avoid duplicating ProgramAddressSpace in TargetInfo. NFCI

2022-11-18 Thread Elizabeth Andrews via Phabricator via cfe-commits 
eandrews added subscribers: erichkeane, aaron.ballman.
eandrews added a comment.

Functionally this looks ok to me. However I am not sure if CodeGenTypes is the 
'right' place for this function to live, considering that other functions with 
similar functionality are in ASTContext - including overloads of 
getTargetAddressSpace( ). @erichkeane @aaron.ballman could you please take a 
look?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138296

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D138296: [clang] Avoid duplicating ProgramAddressSpace in TargetInfo. NFCI

2022-11-18 Thread Alexander Richardson via Phabricator via cfe-commits 
arichardson updated this revision to Diff 476473.
arichardson added a comment.

clang-format


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138296

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/Basic/TargetInfo.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/Basic/TargetInfo.cpp
  clang/lib/Basic/Targets/AVR.h
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGDeclCXX.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenTypes.cpp
  clang/lib/CodeGen/CodeGenTypes.h

Index: clang/lib/CodeGen/CodeGenTypes.h
===
--- clang/lib/CodeGen/CodeGenTypes.h
+++ clang/lib/CodeGen/CodeGenTypes.h
@@ -305,7 +305,7 @@
   bool isRecordBeingLaidOut(const Type *Ty) const {
 return RecordsBeingLaidOut.count(Ty);
   }
-
+  unsigned getTargetAddressSpace(QualType T) const;
 };
 
 }  // end namespace CodeGen
Index: clang/lib/CodeGen/CodeGenTypes.cpp
===
--- clang/lib/CodeGen/CodeGenTypes.cpp
+++ clang/lib/CodeGen/CodeGenTypes.cpp
@@ -655,7 +655,7 @@
 const ReferenceType *RTy = cast(Ty);
 QualType ETy = RTy->getPointeeType();
 llvm::Type *PointeeType = ConvertTypeForMem(ETy);
-unsigned AS = Context.getTargetAddressSpace(ETy);
+unsigned AS = getTargetAddressSpace(ETy);
 ResultType = llvm::PointerType::get(PointeeType, AS);
 break;
   }
@@ -665,7 +665,7 @@
 llvm::Type *PointeeType = ConvertTypeForMem(ETy);
 if (PointeeType->isVoidTy())
   PointeeType = llvm::Type::getInt8Ty(getLLVMContext());
-unsigned AS = Context.getTargetAddressSpace(ETy);
+unsigned AS = getTargetAddressSpace(ETy);
 ResultType = llvm::PointerType::get(PointeeType, AS);
 break;
   }
@@ -958,3 +958,13 @@
 bool CodeGenTypes::isZeroInitializable(const RecordDecl *RD) {
   return getCGRecordLayout(RD).isZeroInitializable();
 }
+
+unsigned CodeGenTypes::getTargetAddressSpace(QualType T) const {
+  // Return the address space for the type. If the type is a
+  // function type without an address space qualifier, the
+  // program address space is used. Otherwise, the target picks
+  // the best address space based on the type information
+  return T->isFunctionType() && !T.hasAddressSpace()
+ ? getDataLayout().getProgramAddressSpace()
+ : getContext().getTargetAddressSpace(T.getAddressSpace());
+}
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -3196,7 +3196,7 @@
   // See if there is already something with the target's name in the module.
   llvm::GlobalValue *Entry = GetGlobalValue(AA->getAliasee());
   if (Entry) {
-unsigned AS = getContext().getTargetAddressSpace(VD->getType());
+unsigned AS = getTypes().getTargetAddressSpace(VD->getType());
 auto Ptr = llvm::ConstantExpr::getBitCast(Entry, DeclTy->getPointerTo(AS));
 return ConstantAddress(Ptr, DeclTy, Alignment);
   }
@@ -3759,7 +3759,7 @@
   if (getTarget().supportsIFunc()) {
 ResolverType = llvm::FunctionType::get(
 llvm::PointerType::get(DeclTy,
-   Context.getTargetAddressSpace(FD->getType())),
+   getTypes().getTargetAddressSpace(FD->getType())),
 false);
   }
   else {
@@ -3897,8 +3897,8 @@
   // cpu_dispatch will be emitted in this translation unit.
   if (getTarget().supportsIFunc() && !FD->isCPUSpecificMultiVersion()) {
 llvm::Type *ResolverType = llvm::FunctionType::get(
-llvm::PointerType::get(
-DeclTy, getContext().getTargetAddressSpace(FD->getType())),
+llvm::PointerType::get(DeclTy,
+   getTypes().getTargetAddressSpace(FD->getType())),
 false);
 llvm::Constant *Resolver = GetOrCreateLLVMFunction(
 MangledName + ".resolver", ResolverType, GlobalDecl{},
Index: clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
===
--- clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
+++ clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
@@ -3128,7 +3128,7 @@
   const Type *NonQualTy = QC.strip(NativeParamType);
   QualType NativePointeeTy = cast(NonQualTy)->getPointeeType();
   unsigned NativePointeeAddrSpace =
-  CGF.getContext().getTargetAddressSpace(NativePointeeTy);
+  CGF.getTypes().getTargetAddressSpace(NativePointeeTy);
   QualType TargetTy = TargetParam->getType();
   llvm::Value *TargetAddr = CGF.EmitLoadOfScalar(
   LocalAddr, /*Volatile=*/false, TargetTy, SourceLocation());
Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++

[PATCH] D138296: [clang] Avoid duplicating ProgramAddressSpace in TargetInfo. NFCI

2022-11-18 Thread Alexander Richardson via Phabricator via cfe-commits 
arichardson created this revision.
arichardson added reviewers: eandrews, dylanmckay, bader, rjmccall.
Herald added subscribers: jrtc27, luismarques, s.egerton, Jim, PkmX, atanasyan, 
simoncook, kristof.beyls, sdardis.
Herald added a project: All.
arichardson requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This value was added to clang/Basic in D111566 
, but is only used during
codegen, where we can use the LLVM IR DataLayout instead. I noticed this
because the downstream CHERI targets would have to also set this value
for AArch64/RISC-V/MIPS. Instead of duplicating more information between
LLVM IR and Clang, this patch moves getTargetAddressSpace(QualType T) to
CodeGenTypes, where we can consult the DataLayout.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138296

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/Basic/TargetInfo.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/Basic/TargetInfo.cpp
  clang/lib/Basic/Targets/AVR.h
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGDeclCXX.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenTypes.cpp
  clang/lib/CodeGen/CodeGenTypes.h

Index: clang/lib/CodeGen/CodeGenTypes.h
===
--- clang/lib/CodeGen/CodeGenTypes.h
+++ clang/lib/CodeGen/CodeGenTypes.h
@@ -305,7 +305,7 @@
   bool isRecordBeingLaidOut(const Type *Ty) const {
 return RecordsBeingLaidOut.count(Ty);
   }
-
+  unsigned getTargetAddressSpace(QualType T) const;
 };
 
 }  // end namespace CodeGen
Index: clang/lib/CodeGen/CodeGenTypes.cpp
===
--- clang/lib/CodeGen/CodeGenTypes.cpp
+++ clang/lib/CodeGen/CodeGenTypes.cpp
@@ -655,7 +655,7 @@
 const ReferenceType *RTy = cast(Ty);
 QualType ETy = RTy->getPointeeType();
 llvm::Type *PointeeType = ConvertTypeForMem(ETy);
-unsigned AS = Context.getTargetAddressSpace(ETy);
+unsigned AS = getTargetAddressSpace(ETy);
 ResultType = llvm::PointerType::get(PointeeType, AS);
 break;
   }
@@ -665,7 +665,7 @@
 llvm::Type *PointeeType = ConvertTypeForMem(ETy);
 if (PointeeType->isVoidTy())
   PointeeType = llvm::Type::getInt8Ty(getLLVMContext());
-unsigned AS = Context.getTargetAddressSpace(ETy);
+unsigned AS = getTargetAddressSpace(ETy);
 ResultType = llvm::PointerType::get(PointeeType, AS);
 break;
   }
@@ -958,3 +958,13 @@
 bool CodeGenTypes::isZeroInitializable(const RecordDecl *RD) {
   return getCGRecordLayout(RD).isZeroInitializable();
 }
+
+unsigned CodeGenTypes::getTargetAddressSpace(QualType T) const {
+  // Return the address space for the type. If the type is a
+  // function type without an address space qualifier, the
+  // program address space is used. Otherwise, the target picks
+  // the best address space based on the type information
+  return T->isFunctionType() && !T.hasAddressSpace()
+ ? getDataLayout().getProgramAddressSpace()
+ : getContext().getTargetAddressSpace(T.getAddressSpace());
+}
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -3195,7 +3195,7 @@
   // See if there is already something with the target's name in the module.
   llvm::GlobalValue *Entry = GetGlobalValue(AA->getAliasee());
   if (Entry) {
-unsigned AS = getContext().getTargetAddressSpace(VD->getType());
+unsigned AS = getTypes().getTargetAddressSpace(VD->getType());
 auto Ptr = llvm::ConstantExpr::getBitCast(Entry, DeclTy->getPointerTo(AS));
 return ConstantAddress(Ptr, DeclTy, Alignment);
   }
@@ -3758,7 +3758,7 @@
   if (getTarget().supportsIFunc()) {
 ResolverType = llvm::FunctionType::get(
 llvm::PointerType::get(DeclTy,
-   Context.getTargetAddressSpace(FD->getType())),
+   getTypes().getTargetAddressSpace(FD->getType())),
 false);
   }
   else {
@@ -3897,7 +3897,7 @@
   if (getTarget().supportsIFunc() && !FD->isCPUSpecificMultiVersion()) {
 llvm::Type *ResolverType = llvm::FunctionType::get(
 llvm::PointerType::get(
-DeclTy, getContext().getTargetAddressSpace(FD->getType())),
+DeclTy, getTypes().getTargetAddressSpace(FD->getType())),
 false);
 llvm::Constant *Resolver = GetOrCreateLLVMFunction(
 MangledName + ".resolver", ResolverType, GlobalDecl{},
Index: clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
===
--- clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
+++ clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
@@ -3128,7 +3128,7 @@
   const Type *NonQualTy =