Skip to content

Commit 1e400ac

Browse files
committed
[Clang] Use TargetInfo when deciding is an address space is compatible
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 00f2989 commit 1e400ac

22 files changed

+355
-128
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->getTargetInfo()))
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.getTargetInfo());
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.getTargetInfo()))
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.getTargetInfo()) &&
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.getTargetInfo()))
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: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -164,14 +164,14 @@ class CanQual {
164164

165165
/// Determines whether this canonical type is more qualified than
166166
/// the @p Other canonical type.
167-
bool isMoreQualifiedThan(CanQual<T> Other) const {
168-
return Stored.isMoreQualifiedThan(Other.Stored);
167+
bool isMoreQualifiedThan(CanQual<T> Other, const TargetInfo &TI) const {
168+
return Stored.isMoreQualifiedThan(Other.Stored, TI);
169169
}
170170

171171
/// Determines whether this canonical type is at least as qualified as
172172
/// the @p Other canonical type.
173-
bool isAtLeastAsQualifiedAs(CanQual<T> Other) const {
174-
return Stored.isAtLeastAsQualifiedAs(Other.Stored);
173+
bool isAtLeastAsQualifiedAs(CanQual<T> Other, const TargetInfo &TI) const {
174+
return Stored.isAtLeastAsQualifiedAs(Other.Stored, TI);
175175
}
176176

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

clang/include/clang/AST/Type.h

Lines changed: 24 additions & 14 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,7 +699,8 @@ 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 TargetInfo &TI) {
701704
// Address spaces must match exactly.
702705
return A == B ||
703706
// Otherwise in OpenCLC v2.0 s6.5.5: every address space except
@@ -722,20 +725,24 @@ class Qualifiers {
722725
// to implicitly cast into the default address space.
723726
(A == LangAS::Default &&
724727
(B == LangAS::cuda_constant || B == LangAS::cuda_device ||
725-
B == LangAS::cuda_shared));
728+
B == LangAS::cuda_shared)) ||
729+
// Conversions from target specific address spaces may be legal
730+
// depending on the target information.
731+
TI.isAddressSpaceSupersetOf(A, B);
726732
}
727733

728734
/// Returns true if the address space in these qualifiers is equal to or
729735
/// a superset of the address space in the argument qualifiers.
730-
bool isAddressSpaceSupersetOf(Qualifiers other) const {
731-
return isAddressSpaceSupersetOf(getAddressSpace(), other.getAddressSpace());
736+
bool isAddressSpaceSupersetOf(Qualifiers other, const TargetInfo &TI) const {
737+
return isAddressSpaceSupersetOf(getAddressSpace(), other.getAddressSpace(),
738+
TI);
732739
}
733740

734741
/// Determines if these qualifiers compatibly include another set.
735742
/// Generally this answers the question of whether an object with the other
736743
/// qualifiers can be safely used as an object with these qualifiers.
737-
bool compatiblyIncludes(Qualifiers other) const {
738-
return isAddressSpaceSupersetOf(other) &&
744+
bool compatiblyIncludes(Qualifiers other, const TargetInfo &TI) const {
745+
return isAddressSpaceSupersetOf(other, TI) &&
739746
// ObjC GC qualifiers can match, be added, or be removed, but can't
740747
// be changed.
741748
(getObjCGCAttr() == other.getObjCGCAttr() || !hasObjCGCAttr() ||
@@ -1273,11 +1280,11 @@ class QualType {
12731280

12741281
/// Determine whether this type is more qualified than the other
12751282
/// given type, requiring exact equality for non-CVR qualifiers.
1276-
bool isMoreQualifiedThan(QualType Other) const;
1283+
bool isMoreQualifiedThan(QualType Other, const TargetInfo &TI) const;
12771284

12781285
/// Determine whether this type is at least as qualified as the other
12791286
/// given type, requiring exact equality for non-CVR qualifiers.
1280-
bool isAtLeastAsQualifiedAs(QualType Other) const;
1287+
bool isAtLeastAsQualifiedAs(QualType Other, const TargetInfo &TI) const;
12811288

12821289
QualType getNonReferenceType() const;
12831290

@@ -1425,11 +1432,12 @@ class QualType {
14251432
/// address spaces overlap iff they are they same.
14261433
/// OpenCL C v2.0 s6.5.5 adds:
14271434
/// __generic overlaps with any address space except for __constant.
1428-
bool isAddressSpaceOverlapping(QualType T) const {
1435+
bool isAddressSpaceOverlapping(QualType T, const TargetInfo &TI) const {
14291436
Qualifiers Q = getQualifiers();
14301437
Qualifiers TQ = T.getQualifiers();
14311438
// Address spaces overlap if at least one of them is a superset of another
1432-
return Q.isAddressSpaceSupersetOf(TQ) || TQ.isAddressSpaceSupersetOf(Q);
1439+
return Q.isAddressSpaceSupersetOf(TQ, TI) ||
1440+
TQ.isAddressSpaceSupersetOf(Q, TI);
14331441
}
14341442

14351443
/// Returns gc attribute of this type.
@@ -8112,24 +8120,26 @@ inline FunctionType::ExtInfo getFunctionExtInfo(QualType t) {
81128120
/// is more qualified than "const int", "volatile int", and
81138121
/// "int". However, it is not more qualified than "const volatile
81148122
/// int".
8115-
inline bool QualType::isMoreQualifiedThan(QualType other) const {
8123+
inline bool QualType::isMoreQualifiedThan(QualType other,
8124+
const TargetInfo &TI) const {
81168125
Qualifiers MyQuals = getQualifiers();
81178126
Qualifiers OtherQuals = other.getQualifiers();
8118-
return (MyQuals != OtherQuals && MyQuals.compatiblyIncludes(OtherQuals));
8127+
return (MyQuals != OtherQuals && MyQuals.compatiblyIncludes(OtherQuals, TI));
81198128
}
81208129

81218130
/// Determine whether this type is at last
81228131
/// as qualified as the Other type. For example, "const volatile
81238132
/// int" is at least as qualified as "const int", "volatile int",
81248133
/// "int", and "const volatile int".
8125-
inline bool QualType::isAtLeastAsQualifiedAs(QualType other) const {
8134+
inline bool QualType::isAtLeastAsQualifiedAs(QualType other,
8135+
const TargetInfo &TI) const {
81268136
Qualifiers OtherQuals = other.getQualifiers();
81278137

81288138
// Ignore __unaligned qualifier if this type is a void.
81298139
if (getUnqualifiedType()->isVoidType())
81308140
OtherQuals.removeUnaligned();
81318141

8132-
return getQualifiers().compatiblyIncludes(OtherQuals);
8142+
return getQualifiers().compatiblyIncludes(OtherQuals, TI);
81338143
}
81348144

81358145
/// 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, getTargetInfo()))
1140911409
return {};
1141011410
LHSPteeQual.removeAddressSpace();
1141111411
RHSPteeQual.removeAddressSpace();

clang/lib/Basic/Targets/AMDGPU.h

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,19 @@ 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+
isTargetAddressSpace(B) &&
124+
toTargetAddressSpace(B) <= llvm::AMDGPUAS::PRIVATE_ADDRESS;
125+
}
126+
114127
uint64_t getMaxPointerWidth() const override {
115128
return getTriple().getArch() == llvm::Triple::amdgcn ? 64 : 32;
116129
}

clang/lib/Basic/Targets/NVPTX.h

Lines changed: 36 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -17,34 +17,34 @@
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

2324
namespace clang {
2425
namespace targets {
2526

2627
static const unsigned NVPTXAddrSpaceMap[] = {
27-
0, // Default
28-
1, // opencl_global
29-
3, // opencl_local
30-
4, // opencl_constant
31-
0, // opencl_private
32-
// FIXME: generic has to be added to the target
33-
0, // opencl_generic
34-
1, // opencl_global_device
35-
1, // opencl_global_host
36-
1, // cuda_device
37-
4, // cuda_constant
38-
3, // cuda_shared
39-
1, // sycl_global
40-
1, // sycl_global_device
41-
1, // sycl_global_host
42-
3, // sycl_local
43-
0, // sycl_private
44-
0, // ptr32_sptr
45-
0, // ptr32_uptr
46-
0, // ptr64
47-
0, // hlsl_groupshared
28+
llvm::NVPTXAS::ADDRESS_SPACE_GENERIC, // Default
29+
llvm::NVPTXAS::ADDRESS_SPACE_GLOBAL, // opencl_global
30+
llvm::NVPTXAS::ADDRESS_SPACE_SHARED, // opencl_local
31+
llvm::NVPTXAS::ADDRESS_SPACE_CONST, // opencl_constant
32+
llvm::NVPTXAS::ADDRESS_SPACE_GENERIC, // opencl_private
33+
llvm::NVPTXAS::ADDRESS_SPACE_GENERIC, // opencl_generic
34+
llvm::NVPTXAS::ADDRESS_SPACE_GLOBAL, // opencl_global_device
35+
llvm::NVPTXAS::ADDRESS_SPACE_GLOBAL, // opencl_global_host
36+
llvm::NVPTXAS::ADDRESS_SPACE_GLOBAL, // cuda_device
37+
llvm::NVPTXAS::ADDRESS_SPACE_CONST, // cuda_constant
38+
llvm::NVPTXAS::ADDRESS_SPACE_SHARED, // cuda_shared
39+
llvm::NVPTXAS::ADDRESS_SPACE_GLOBAL, // sycl_global
40+
llvm::NVPTXAS::ADDRESS_SPACE_GLOBAL, // sycl_global_device
41+
llvm::NVPTXAS::ADDRESS_SPACE_GLOBAL, // sycl_global_host
42+
llvm::NVPTXAS::ADDRESS_SPACE_SHARED, // sycl_local
43+
llvm::NVPTXAS::ADDRESS_SPACE_GENERIC, // sycl_private
44+
llvm::NVPTXAS::ADDRESS_SPACE_GENERIC, // ptr32_sptr
45+
llvm::NVPTXAS::ADDRESS_SPACE_GENERIC, // ptr32_uptr
46+
llvm::NVPTXAS::ADDRESS_SPACE_GENERIC, // ptr64
47+
llvm::NVPTXAS::ADDRESS_SPACE_GENERIC, // hlsl_groupshared
4848
// Wasm address space values for this target are dummy values,
4949
// as it is only enabled for Wasm targets.
5050
20, // wasm_funcref
@@ -89,6 +89,21 @@ class LLVM_LIBRARY_VISIBILITY NVPTXTargetInfo : public TargetInfo {
8989

9090
bool hasFeature(StringRef Feature) const override;
9191

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

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

clang/lib/Sema/SemaARM.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -907,7 +907,8 @@ 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,
911+
getASTContext().getTargetInfo())) {
911912
CastNeeded = CK_BitCast;
912913
Diag(DRE->getBeginLoc(), diag::ext_typecheck_convert_discards_qualifiers)
913914
<< PointerArg->getType() << Context.getPointerType(AddrType)

0 commit comments

Comments
 (0)