Skip to content

Commit b9d678d

Browse files
authored
[Clang] Use TargetInfo when deciding if an address space is compatible (#115777)
Summary: Address spaces are used in several embedded and GPU targets to describe accesses to different types of memory. Currently we use the address space enumerations to control which address spaces are considered supersets of eachother, however this is also a target level property as described by the C standard's passing mentions. This patch allows the address space checks to use the target information to decide if a pointer conversion is legal. For AMDGPU and NVPTX, all supported address spaces can be converted to the default address space. More semantic checks can be added on top of this, for now I'm mainly looking to get more standard semantics working for C/C++. Right now the address space conversions must all be done explicitly in C/C++ unlike the offloading languages which define their own custom address spaces that just map to the same target specific ones anyway. The main question is if this behavior is a function of the target or the language.
1 parent 40afff7 commit b9d678d

25 files changed

+383
-129
lines changed

clang-tools-extra/clang-tidy/bugprone/VirtualNearMissCheck.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ static bool checkOverridingFunctionReturnType(const ASTContext *Context,
112112

113113
// The class type D should have the same cv-qualification as or less
114114
// cv-qualification than the class type B.
115-
if (DTy.isMoreQualifiedThan(BTy))
115+
if (DTy.isMoreQualifiedThan(BTy, *Context))
116116
return false;
117117

118118
return true;

clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -299,10 +299,11 @@ static bool applyDiceHeuristic(StringRef Arg, StringRef Param,
299299

300300
/// Checks if ArgType binds to ParamType regarding reference-ness and
301301
/// cv-qualifiers.
302-
static bool areRefAndQualCompatible(QualType ArgType, QualType ParamType) {
302+
static bool areRefAndQualCompatible(QualType ArgType, QualType ParamType,
303+
const ASTContext &Ctx) {
303304
return !ParamType->isReferenceType() ||
304305
ParamType.getNonReferenceType().isAtLeastAsQualifiedAs(
305-
ArgType.getNonReferenceType());
306+
ArgType.getNonReferenceType(), Ctx);
306307
}
307308

308309
static bool isPointerOrArray(QualType TypeToCheck) {
@@ -311,12 +312,12 @@ static bool isPointerOrArray(QualType TypeToCheck) {
311312

312313
/// Checks whether ArgType is an array type identical to ParamType's array type.
313314
/// Enforces array elements' qualifier compatibility as well.
314-
static bool isCompatibleWithArrayReference(QualType ArgType,
315-
QualType ParamType) {
315+
static bool isCompatibleWithArrayReference(QualType ArgType, QualType ParamType,
316+
const ASTContext &Ctx) {
316317
if (!ArgType->isArrayType())
317318
return false;
318319
// Here, qualifiers belong to the elements of the arrays.
319-
if (!ParamType.isAtLeastAsQualifiedAs(ArgType))
320+
if (!ParamType.isAtLeastAsQualifiedAs(ArgType, Ctx))
320321
return false;
321322

322323
return ParamType.getUnqualifiedType() == ArgType.getUnqualifiedType();
@@ -342,12 +343,13 @@ static QualType convertToPointeeOrArrayElementQualType(QualType TypeToConvert) {
342343
/// every * in ParamType to the right of that cv-qualifier, except the last
343344
/// one, must also be const-qualified.
344345
static bool arePointersStillQualCompatible(QualType ArgType, QualType ParamType,
345-
bool &IsParamContinuouslyConst) {
346+
bool &IsParamContinuouslyConst,
347+
const ASTContext &Ctx) {
346348
// The types are compatible, if the parameter is at least as qualified as the
347349
// argument, and if it is more qualified, it has to be const on upper pointer
348350
// levels.
349351
bool AreTypesQualCompatible =
350-
ParamType.isAtLeastAsQualifiedAs(ArgType) &&
352+
ParamType.isAtLeastAsQualifiedAs(ArgType, Ctx) &&
351353
(!ParamType.hasQualifiers() || IsParamContinuouslyConst);
352354
// Check whether the parameter's constness continues at the current pointer
353355
// level.
@@ -359,9 +361,10 @@ static bool arePointersStillQualCompatible(QualType ArgType, QualType ParamType,
359361
/// Checks whether multilevel pointers are compatible in terms of levels,
360362
/// qualifiers and pointee type.
361363
static bool arePointerTypesCompatible(QualType ArgType, QualType ParamType,
362-
bool IsParamContinuouslyConst) {
364+
bool IsParamContinuouslyConst,
365+
const ASTContext &Ctx) {
363366
if (!arePointersStillQualCompatible(ArgType, ParamType,
364-
IsParamContinuouslyConst))
367+
IsParamContinuouslyConst, Ctx))
365368
return false;
366369

367370
do {
@@ -372,7 +375,7 @@ static bool arePointerTypesCompatible(QualType ArgType, QualType ParamType,
372375
// Check whether cv-qualifiers permit compatibility on
373376
// current level.
374377
if (!arePointersStillQualCompatible(ArgType, ParamType,
375-
IsParamContinuouslyConst))
378+
IsParamContinuouslyConst, Ctx))
376379
return false;
377380

378381
if (ParamType.getUnqualifiedType() == ArgType.getUnqualifiedType())
@@ -396,7 +399,7 @@ static bool areTypesCompatible(QualType ArgType, QualType ParamType,
396399
return true;
397400

398401
// Check for constness and reference compatibility.
399-
if (!areRefAndQualCompatible(ArgType, ParamType))
402+
if (!areRefAndQualCompatible(ArgType, ParamType, Ctx))
400403
return false;
401404

402405
bool IsParamReference = ParamType->isReferenceType();
@@ -434,7 +437,7 @@ static bool areTypesCompatible(QualType ArgType, QualType ParamType,
434437
// When ParamType is an array reference, ArgType has to be of the same-sized
435438
// array-type with cv-compatible element type.
436439
if (IsParamReference && ParamType->isArrayType())
437-
return isCompatibleWithArrayReference(ArgType, ParamType);
440+
return isCompatibleWithArrayReference(ArgType, ParamType, Ctx);
438441

439442
bool IsParamContinuouslyConst =
440443
!IsParamReference || ParamType.getNonReferenceType().isConstQualified();
@@ -444,7 +447,7 @@ static bool areTypesCompatible(QualType ArgType, QualType ParamType,
444447
ParamType = convertToPointeeOrArrayElementQualType(ParamType);
445448

446449
// Check qualifier compatibility on the next level.
447-
if (!ParamType.isAtLeastAsQualifiedAs(ArgType))
450+
if (!ParamType.isAtLeastAsQualifiedAs(ArgType, Ctx))
448451
return false;
449452

450453
if (ParamType.getUnqualifiedType() == ArgType.getUnqualifiedType())
@@ -472,8 +475,8 @@ static bool areTypesCompatible(QualType ArgType, QualType ParamType,
472475
if (!(ParamType->isAnyPointerType() && ArgType->isAnyPointerType()))
473476
return false;
474477

475-
return arePointerTypesCompatible(ArgType, ParamType,
476-
IsParamContinuouslyConst);
478+
return arePointerTypesCompatible(ArgType, ParamType, IsParamContinuouslyConst,
479+
Ctx);
477480
}
478481

479482
static bool isOverloadedUnaryOrBinarySymbolOperator(const FunctionDecl *FD) {

clang/include/clang/AST/CanonicalType.h

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ namespace clang {
3030

3131
template<typename T> class CanProxy;
3232
template<typename T> struct CanProxyAdaptor;
33+
class ASTContext;
3334
class CXXRecordDecl;
3435
class EnumDecl;
3536
class Expr;
@@ -164,14 +165,14 @@ class CanQual {
164165

165166
/// Determines whether this canonical type is more qualified than
166167
/// the @p Other canonical type.
167-
bool isMoreQualifiedThan(CanQual<T> Other) const {
168-
return Stored.isMoreQualifiedThan(Other.Stored);
168+
bool isMoreQualifiedThan(CanQual<T> Other, const ASTContext &Ctx) const {
169+
return Stored.isMoreQualifiedThan(Other.Stored, Ctx);
169170
}
170171

171172
/// Determines whether this canonical type is at least as qualified as
172173
/// the @p Other canonical type.
173-
bool isAtLeastAsQualifiedAs(CanQual<T> Other) const {
174-
return Stored.isAtLeastAsQualifiedAs(Other.Stored);
174+
bool isAtLeastAsQualifiedAs(CanQual<T> Other, const ASTContext &Ctx) const {
175+
return Stored.isAtLeastAsQualifiedAs(Other.Stored, Ctx);
175176
}
176177

177178
/// If the canonical type is a reference type, returns the type that

clang/include/clang/AST/Type.h

Lines changed: 24 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
#include "clang/Basic/PointerAuthOptions.h"
3232
#include "clang/Basic/SourceLocation.h"
3333
#include "clang/Basic/Specifiers.h"
34+
#include "clang/Basic/TargetInfo.h"
3435
#include "clang/Basic/Visibility.h"
3536
#include "llvm/ADT/APInt.h"
3637
#include "llvm/ADT/APSInt.h"
@@ -323,6 +324,7 @@ class PointerAuthQualifier {
323324
/// * Objective C: the GC attributes (none, weak, or strong)
324325
class Qualifiers {
325326
public:
327+
Qualifiers() = default;
326328
enum TQ : uint64_t {
327329
// NOTE: These flags must be kept in sync with DeclSpec::TQ.
328330
Const = 0x1,
@@ -697,45 +699,27 @@ class Qualifiers {
697699
/// every address space is a superset of itself.
698700
/// CL2.0 adds:
699701
/// __generic is a superset of any address space except for __constant.
700-
static bool isAddressSpaceSupersetOf(LangAS A, LangAS B) {
702+
static bool isAddressSpaceSupersetOf(LangAS A, LangAS B,
703+
const ASTContext &Ctx) {
701704
// Address spaces must match exactly.
702-
return A == B ||
703-
// Otherwise in OpenCLC v2.0 s6.5.5: every address space except
704-
// for __constant can be used as __generic.
705-
(A == LangAS::opencl_generic && B != LangAS::opencl_constant) ||
706-
// We also define global_device and global_host address spaces,
707-
// to distinguish global pointers allocated on host from pointers
708-
// allocated on device, which are a subset of __global.
709-
(A == LangAS::opencl_global && (B == LangAS::opencl_global_device ||
710-
B == LangAS::opencl_global_host)) ||
711-
(A == LangAS::sycl_global && (B == LangAS::sycl_global_device ||
712-
B == LangAS::sycl_global_host)) ||
713-
// Consider pointer size address spaces to be equivalent to default.
714-
((isPtrSizeAddressSpace(A) || A == LangAS::Default) &&
715-
(isPtrSizeAddressSpace(B) || B == LangAS::Default)) ||
716-
// Default is a superset of SYCL address spaces.
717-
(A == LangAS::Default &&
718-
(B == LangAS::sycl_private || B == LangAS::sycl_local ||
719-
B == LangAS::sycl_global || B == LangAS::sycl_global_device ||
720-
B == LangAS::sycl_global_host)) ||
721-
// In HIP device compilation, any cuda address space is allowed
722-
// to implicitly cast into the default address space.
723-
(A == LangAS::Default &&
724-
(B == LangAS::cuda_constant || B == LangAS::cuda_device ||
725-
B == LangAS::cuda_shared));
705+
return A == B || isTargetAddressSpaceSupersetOf(A, B, Ctx);
726706
}
727707

708+
static bool isTargetAddressSpaceSupersetOf(LangAS A, LangAS B,
709+
const ASTContext &Ctx);
710+
728711
/// Returns true if the address space in these qualifiers is equal to or
729712
/// a superset of the address space in the argument qualifiers.
730-
bool isAddressSpaceSupersetOf(Qualifiers other) const {
731-
return isAddressSpaceSupersetOf(getAddressSpace(), other.getAddressSpace());
713+
bool isAddressSpaceSupersetOf(Qualifiers other, const ASTContext &Ctx) const {
714+
return isAddressSpaceSupersetOf(getAddressSpace(), other.getAddressSpace(),
715+
Ctx);
732716
}
733717

734718
/// Determines if these qualifiers compatibly include another set.
735719
/// Generally this answers the question of whether an object with the other
736720
/// qualifiers can be safely used as an object with these qualifiers.
737-
bool compatiblyIncludes(Qualifiers other) const {
738-
return isAddressSpaceSupersetOf(other) &&
721+
bool compatiblyIncludes(Qualifiers other, const ASTContext &Ctx) const {
722+
return isAddressSpaceSupersetOf(other, Ctx) &&
739723
// ObjC GC qualifiers can match, be added, or be removed, but can't
740724
// be changed.
741725
(getObjCGCAttr() == other.getObjCGCAttr() || !hasObjCGCAttr() ||
@@ -1273,11 +1257,11 @@ class QualType {
12731257

12741258
/// Determine whether this type is more qualified than the other
12751259
/// given type, requiring exact equality for non-CVR qualifiers.
1276-
bool isMoreQualifiedThan(QualType Other) const;
1260+
bool isMoreQualifiedThan(QualType Other, const ASTContext &Ctx) const;
12771261

12781262
/// Determine whether this type is at least as qualified as the other
12791263
/// given type, requiring exact equality for non-CVR qualifiers.
1280-
bool isAtLeastAsQualifiedAs(QualType Other) const;
1264+
bool isAtLeastAsQualifiedAs(QualType Other, const ASTContext &Ctx) const;
12811265

12821266
QualType getNonReferenceType() const;
12831267

@@ -1425,11 +1409,12 @@ class QualType {
14251409
/// address spaces overlap iff they are they same.
14261410
/// OpenCL C v2.0 s6.5.5 adds:
14271411
/// __generic overlaps with any address space except for __constant.
1428-
bool isAddressSpaceOverlapping(QualType T) const {
1412+
bool isAddressSpaceOverlapping(QualType T, const ASTContext &Ctx) const {
14291413
Qualifiers Q = getQualifiers();
14301414
Qualifiers TQ = T.getQualifiers();
14311415
// Address spaces overlap if at least one of them is a superset of another
1432-
return Q.isAddressSpaceSupersetOf(TQ) || TQ.isAddressSpaceSupersetOf(Q);
1416+
return Q.isAddressSpaceSupersetOf(TQ, Ctx) ||
1417+
TQ.isAddressSpaceSupersetOf(Q, Ctx);
14331418
}
14341419

14351420
/// Returns gc attribute of this type.
@@ -8112,24 +8097,26 @@ inline FunctionType::ExtInfo getFunctionExtInfo(QualType t) {
81128097
/// is more qualified than "const int", "volatile int", and
81138098
/// "int". However, it is not more qualified than "const volatile
81148099
/// int".
8115-
inline bool QualType::isMoreQualifiedThan(QualType other) const {
8100+
inline bool QualType::isMoreQualifiedThan(QualType other,
8101+
const ASTContext &Ctx) const {
81168102
Qualifiers MyQuals = getQualifiers();
81178103
Qualifiers OtherQuals = other.getQualifiers();
8118-
return (MyQuals != OtherQuals && MyQuals.compatiblyIncludes(OtherQuals));
8104+
return (MyQuals != OtherQuals && MyQuals.compatiblyIncludes(OtherQuals, Ctx));
81198105
}
81208106

81218107
/// Determine whether this type is at last
81228108
/// as qualified as the Other type. For example, "const volatile
81238109
/// int" is at least as qualified as "const int", "volatile int",
81248110
/// "int", and "const volatile int".
8125-
inline bool QualType::isAtLeastAsQualifiedAs(QualType other) const {
8111+
inline bool QualType::isAtLeastAsQualifiedAs(QualType other,
8112+
const ASTContext &Ctx) const {
81268113
Qualifiers OtherQuals = other.getQualifiers();
81278114

81288115
// Ignore __unaligned qualifier if this type is a void.
81298116
if (getUnqualifiedType()->isVoidType())
81308117
OtherQuals.removeUnaligned();
81318118

8132-
return getQualifiers().compatiblyIncludes(OtherQuals);
8119+
return getQualifiers().compatiblyIncludes(OtherQuals, Ctx);
81338120
}
81348121

81358122
/// If Type is a reference type (e.g., const

clang/include/clang/Basic/TargetInfo.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -486,6 +486,13 @@ class TargetInfo : public TransferrableTargetInfo,
486486
/// \param AddrSpace address space of pointee in source language.
487487
virtual uint64_t getNullPointerValue(LangAS AddrSpace) const { return 0; }
488488

489+
/// Returns true if an address space can be safely converted to another.
490+
/// \param A address space of target in source language.
491+
/// \param B address space of source in source language.
492+
virtual bool isAddressSpaceSupersetOf(LangAS A, LangAS B) const {
493+
return A == B;
494+
}
495+
489496
/// Return the size of '_Bool' and C++ 'bool' for this target, in bits.
490497
unsigned getBoolWidth() const { return BoolWidth; }
491498

clang/lib/AST/ASTContext.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11405,7 +11405,7 @@ QualType ASTContext::mergeTypes(QualType LHS, QualType RHS, bool OfBlockPointer,
1140511405
Qualifiers RHSPteeQual = RHSPointee.getQualifiers();
1140611406
// Blocks can't be an expression in a ternary operator (OpenCL v2.0
1140711407
// 6.12.5) thus the following check is asymmetric.
11408-
if (!LHSPteeQual.isAddressSpaceSupersetOf(RHSPteeQual))
11408+
if (!LHSPteeQual.isAddressSpaceSupersetOf(RHSPteeQual, *this))
1140911409
return {};
1141011410
LHSPteeQual.removeAddressSpace();
1141111411
RHSPteeQual.removeAddressSpace();

clang/lib/AST/Type.cpp

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,36 @@ bool Qualifiers::isStrictSupersetOf(Qualifiers Other) const {
7373
(hasObjCLifetime() && !Other.hasObjCLifetime()));
7474
}
7575

76+
bool Qualifiers::isTargetAddressSpaceSupersetOf(LangAS A, LangAS B,
77+
const ASTContext &Ctx) {
78+
// In OpenCLC v2.0 s6.5.5: every address space except for __constant can be
79+
// used as __generic.
80+
return (A == LangAS::opencl_generic && B != LangAS::opencl_constant) ||
81+
// We also define global_device and global_host address spaces,
82+
// to distinguish global pointers allocated on host from pointers
83+
// allocated on device, which are a subset of __global.
84+
(A == LangAS::opencl_global && (B == LangAS::opencl_global_device ||
85+
B == LangAS::opencl_global_host)) ||
86+
(A == LangAS::sycl_global &&
87+
(B == LangAS::sycl_global_device || B == LangAS::sycl_global_host)) ||
88+
// Consider pointer size address spaces to be equivalent to default.
89+
((isPtrSizeAddressSpace(A) || A == LangAS::Default) &&
90+
(isPtrSizeAddressSpace(B) || B == LangAS::Default)) ||
91+
// Default is a superset of SYCL address spaces.
92+
(A == LangAS::Default &&
93+
(B == LangAS::sycl_private || B == LangAS::sycl_local ||
94+
B == LangAS::sycl_global || B == LangAS::sycl_global_device ||
95+
B == LangAS::sycl_global_host)) ||
96+
// In HIP device compilation, any cuda address space is allowed
97+
// to implicitly cast into the default address space.
98+
(A == LangAS::Default &&
99+
(B == LangAS::cuda_constant || B == LangAS::cuda_device ||
100+
B == LangAS::cuda_shared)) ||
101+
// Conversions from target specific address spaces may be legal
102+
// depending on the target information.
103+
Ctx.getTargetInfo().isAddressSpaceSupersetOf(A, B);
104+
}
105+
76106
const IdentifierInfo* QualType::getBaseTypeIdentifier() const {
77107
const Type* ty = getTypePtr();
78108
NamedDecl *ND = nullptr;

clang/lib/Basic/Targets/AMDGPU.h

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,18 @@ class LLVM_LIBRARY_VISIBILITY AMDGPUTargetInfo final : public TargetInfo {
111111
return getPointerWidthV(AddrSpace);
112112
}
113113

114+
virtual bool isAddressSpaceSupersetOf(LangAS A, LangAS B) const override {
115+
// The flat address space AS(0) is a superset of all the other address
116+
// spaces used by the backend target.
117+
return A == B ||
118+
((A == LangAS::Default ||
119+
(isTargetAddressSpace(A) &&
120+
toTargetAddressSpace(A) == llvm::AMDGPUAS::FLAT_ADDRESS)) &&
121+
isTargetAddressSpace(B) &&
122+
toTargetAddressSpace(B) >= llvm::AMDGPUAS::FLAT_ADDRESS &&
123+
toTargetAddressSpace(B) <= llvm::AMDGPUAS::PRIVATE_ADDRESS);
124+
}
125+
114126
uint64_t getMaxPointerWidth() const override {
115127
return getTriple().getArch() == llvm::Triple::amdgcn ? 64 : 32;
116128
}

clang/lib/Basic/Targets/NVPTX.h

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
#include "clang/Basic/TargetInfo.h"
1818
#include "clang/Basic/TargetOptions.h"
1919
#include "llvm/Support/Compiler.h"
20+
#include "llvm/Support/NVPTXAddrSpace.h"
2021
#include "llvm/TargetParser/Triple.h"
2122
#include <optional>
2223

@@ -89,6 +90,20 @@ class LLVM_LIBRARY_VISIBILITY NVPTXTargetInfo : public TargetInfo {
8990

9091
bool hasFeature(StringRef Feature) const override;
9192

93+
virtual bool isAddressSpaceSupersetOf(LangAS A, LangAS B) const override {
94+
// The generic address space AS(0) is a superset of all the other address
95+
// spaces used by the backend target.
96+
return A == B ||
97+
((A == LangAS::Default ||
98+
(isTargetAddressSpace(A) &&
99+
toTargetAddressSpace(A) ==
100+
llvm::NVPTXAS::ADDRESS_SPACE_GENERIC)) &&
101+
isTargetAddressSpace(B) &&
102+
toTargetAddressSpace(B) >= llvm::NVPTXAS::ADDRESS_SPACE_GENERIC &&
103+
toTargetAddressSpace(B) <= llvm::NVPTXAS::ADDRESS_SPACE_LOCAL &&
104+
toTargetAddressSpace(B) != 2);
105+
}
106+
92107
ArrayRef<const char *> getGCCRegNames() const override;
93108

94109
ArrayRef<TargetInfo::GCCRegAlias> getGCCRegAliases() const override {

clang/lib/Sema/SemaARM.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -907,7 +907,7 @@ bool SemaARM::CheckARMBuiltinExclusiveCall(unsigned BuiltinID,
907907

908908
// Issue a warning if the cast is dodgy.
909909
CastKind CastNeeded = CK_NoOp;
910-
if (!AddrType.isAtLeastAsQualifiedAs(ValType)) {
910+
if (!AddrType.isAtLeastAsQualifiedAs(ValType, getASTContext())) {
911911
CastNeeded = CK_BitCast;
912912
Diag(DRE->getBeginLoc(), diag::ext_typecheck_convert_discards_qualifiers)
913913
<< PointerArg->getType() << Context.getPointerType(AddrType)

0 commit comments

Comments
 (0)