ebevhan created this revision.
ebevhan added reviewers: Anastasia, rjmccall.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
This is an initial draft of the support for target-configurable
address spaces.
Original RFC: http://lists.llvm.org/pipermail/cfe-dev/2019-March/061541.html
After thinking about Anastasia's input on the RFC regarding
the superspace/subspace model, I decided to go a different
route with the design of the interface. Instead of having
accessors for implicit casts and explicit casts, I kept the
subspace accessor and added an accessor for overriding
the behavior of explicit casts only.
This lets us keep the overlap model and support the default
Embedded-C functionality while still letting targets
configure their behavior reasonably.
This patch does not address the issue with the accessors
on Qualifiers (isAddressSpaceSupersetOf, compatiblyIncludes),
because I don't know how to solve it without breaking a ton of
rather nice encapsulation. Either, every mention of compatiblyIncludes
must be replaced with a call to a corresponding ASTContext method,
Qualifiers must have a handle to ASTContext, or ASTContext must be
passed to every such call. This revision mentions the issue in a comment:
https://reviews.llvm.org/D49294
Repository:
rC Clang
https://reviews.llvm.org/D62574
Files:
include/clang/AST/ASTContext.h
include/clang/AST/Type.h
include/clang/Basic/TargetInfo.h
lib/AST/ASTContext.cpp
lib/Sema/SemaCast.cpp
lib/Sema/SemaExpr.cpp
lib/Sema/SemaInit.cpp
lib/Sema/SemaOverload.cpp
test/Sema/address_space_print_macro.c
test/Sema/address_spaces.c
Index: test/Sema/address_spaces.c
===================================================================
--- test/Sema/address_spaces.c
+++ test/Sema/address_spaces.c
@@ -71,7 +71,8 @@
// Clang extension doesn't forbid operations on pointers to different address spaces.
char* cmp(_AS1 char *x, _AS2 char *y) {
- return x < y ? x : y; // expected-error{{conditional operator with the second and third operands of type ('_AS1 char *' and '_AS2 char *') which are pointers to non-overlapping address spaces}}
+ return x < y ? x : y; // expected-error{{conditional operator with the second and third operands of type ('_AS1 char *' and '_AS2 char *') which are pointers to non-overlapping address spaces}} \
+ // expected-error{{comparison between ('_AS1 char *' and '_AS2 char *') which are pointers to non-overlapping address spaces}}
}
struct SomeStruct {
Index: test/Sema/address_space_print_macro.c
===================================================================
--- test/Sema/address_space_print_macro.c
+++ test/Sema/address_space_print_macro.c
@@ -14,7 +14,8 @@
}
char *cmp(AS1 char *x, AS2 char *y) {
- return x < y ? x : y; // expected-error{{conditional operator with the second and third operands of type ('AS1 char *' and 'AS2 char *') which are pointers to non-overlapping address spaces}}
+ return x < y ? x : y; // expected-error{{conditional operator with the second and third operands of type ('AS1 char *' and 'AS2 char *') which are pointers to non-overlapping address spaces}} \
+ // expected-error{{comparison between ('AS1 char *' and 'AS2 char *') which are pointers to non-overlapping address spaces}}
}
__attribute__((address_space(1))) char test_array[10];
Index: lib/Sema/SemaOverload.cpp
===================================================================
--- lib/Sema/SemaOverload.cpp
+++ lib/Sema/SemaOverload.cpp
@@ -3161,12 +3161,17 @@
= PreviousToQualsIncludeConst && ToQuals.hasConst();
}
- // Allows address space promotion by language rules implemented in
- // Type::Qualifiers::isAddressSpaceSupersetOf.
+ // FIXME: This should *really* be testing implicit conversions with
+ // 'isAddressSpaceSupersetOf' and explicit conversions with
+ // 'isExplicitAddrSpaceConversionLegal', but IsQualificationConversion isn't
+ // aware of whether the conversion is implicit or explicit. Add that?
+ //
+ // Also, I don't really understand why we do this. How does preventing
+ // qualification conversion for disjoint address spaces make address space
+ // casts *work*?
Qualifiers FromQuals = FromType.getQualifiers();
Qualifiers ToQuals = ToType.getQualifiers();
- if (!ToQuals.isAddressSpaceSupersetOf(FromQuals) &&
- !FromQuals.isAddressSpaceSupersetOf(ToQuals)) {
+ if (!Context.isAddressSpaceOverlapping(ToQuals, FromQuals)) {
return false;
}
@@ -5142,10 +5147,14 @@
return ICS;
}
+ // FIXME: hasAddressSpace is wrong; this check will be skipped if FromType is
+ // not qualified with an address space, but if there's no implicit conversion
+ // from Default to ImplicitParamType's AS, that's an error.
if (FromTypeCanon.getQualifiers().hasAddressSpace()) {
Qualifiers QualsImplicitParamType = ImplicitParamType.getQualifiers();
Qualifiers QualsFromType = FromTypeCanon.getQualifiers();
- if (!QualsImplicitParamType.isAddressSpaceSupersetOf(QualsFromType)) {
+ if (!S.Context.isAddressSpaceSupersetOf(QualsImplicitParamType,
+ QualsFromType)) {
ICS.setBad(BadConversionSequence::bad_qualifiers,
FromType, ImplicitParamType);
return ICS;
Index: lib/Sema/SemaInit.cpp
===================================================================
--- lib/Sema/SemaInit.cpp
+++ lib/Sema/SemaInit.cpp
@@ -4813,7 +4813,7 @@
unsigned T2CVRQuals = T2Quals.getCVRQualifiers();
if ((RefRelationship == Sema::Ref_Related &&
(T1CVRQuals | T2CVRQuals) != T1CVRQuals) ||
- !T1Quals.isAddressSpaceSupersetOf(T2Quals)) {
+ !S.Context.isAddressSpaceSupersetOf(T1Quals, T2Quals)) {
Sequence.SetFailed(InitializationSequence::FK_ReferenceInitDropsQualifiers);
return;
}
Index: lib/Sema/SemaExpr.cpp
===================================================================
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -6792,9 +6792,9 @@
// OpenCL v1.1 s6.5 - Conversion between pointers to distinct address
// spaces is disallowed.
- if (lhQual.isAddressSpaceSupersetOf(rhQual))
+ if (S.Context.isAddressSpaceSupersetOf(lhQual, rhQual))
ResultAddrSpace = LAddrSpace;
- else if (rhQual.isAddressSpaceSupersetOf(lhQual))
+ else if (S.Context.isAddressSpaceSupersetOf(rhQual, lhQual))
ResultAddrSpace = RAddrSpace;
else {
S.Diag(Loc, diag::err_typecheck_op_on_nonoverlapping_address_space_pointers)
@@ -7745,7 +7745,7 @@
if (!lhq.compatiblyIncludes(rhq)) {
// Treat address-space mismatches as fatal.
- if (!lhq.isAddressSpaceSupersetOf(rhq))
+ if (!S.Context.isAddressSpaceSupersetOf(lhq, rhq))
return Sema::IncompatiblePointerDiscardsQualifiers;
// It's okay to add or remove GC or lifetime qualifiers when converting to
@@ -9244,10 +9244,12 @@
if (isRHSPointer) RHSPointeeTy = RHSExpr->getType()->getPointeeType();
// if both are pointers check if operation is valid wrt address spaces
- if (S.getLangOpts().OpenCL && isLHSPointer && isRHSPointer) {
+ if (isLHSPointer && isRHSPointer) {
const PointerType *lhsPtr = LHSExpr->getType()->getAs<PointerType>();
const PointerType *rhsPtr = RHSExpr->getType()->getAs<PointerType>();
- if (!lhsPtr->isAddressSpaceOverlapping(*rhsPtr)) {
+ if (!S.Context.isAddressSpaceOverlapping(
+ lhsPtr->getPointeeType().getAddressSpace(),
+ rhsPtr->getPointeeType().getAddressSpace())) {
S.Diag(Loc,
diag::err_typecheck_op_on_nonoverlapping_address_space_pointers)
<< LHSExpr->getType() << RHSExpr->getType() << 1 /*arithmetic op*/
@@ -10582,10 +10584,11 @@
diagnoseDistinctPointerComparison(*this, Loc, LHS, RHS, /*isError*/false);
}
if (LCanPointeeTy != RCanPointeeTy) {
- // Treat NULL constant as a special case in OpenCL.
- if (getLangOpts().OpenCL && !LHSIsNull && !RHSIsNull) {
+ if (!LHSIsNull && !RHSIsNull) {
const PointerType *LHSPtr = LHSType->getAs<PointerType>();
- if (!LHSPtr->isAddressSpaceOverlapping(*RHSType->getAs<PointerType>())) {
+ if (!Context.isAddressSpaceOverlapping(
+ LHSPtr->getPointeeType().getAddressSpace(),
+ RHSType->getPointeeType().getAddressSpace())) {
Diag(Loc,
diag::err_typecheck_op_on_nonoverlapping_address_space_pointers)
<< LHSType << RHSType << 0 /* comparison */
Index: lib/Sema/SemaCast.cpp
===================================================================
--- lib/Sema/SemaCast.cpp
+++ lib/Sema/SemaCast.cpp
@@ -2215,9 +2215,13 @@
if (IsAddressSpaceConversion(SrcType, DestType)) {
Kind = CK_AddressSpaceConversion;
assert(SrcType->isPointerType() && DestType->isPointerType());
- if (!CStyle &&
- !DestType->getPointeeType().getQualifiers().isAddressSpaceSupersetOf(
- SrcType->getPointeeType().getQualifiers())) {
+ auto SrcQ = SrcType->getPointeeType().getQualifiers();
+ auto DestQ = DestType->getPointeeType().getQualifiers();
+ // Real reinterpret_casts can only do address space conversions which are
+ // 'implicit', such as subspace->superspace. For C-style casts, check if
+ // the cast is explicitly legal as well.
+ if (CStyle ? !Self.Context.isExplicitAddrSpaceConversionLegal(SrcQ, DestQ)
+ : !Self.Context.isAddressSpaceSupersetOf(DestQ, SrcQ)) {
SuccessResult = TC_Failed;
}
} else if (IsLValueCast) {
@@ -2286,14 +2290,7 @@
static TryCastResult TryAddressSpaceCast(Sema &Self, ExprResult &SrcExpr,
QualType DestType, bool CStyle,
- unsigned &msg) {
- if (!Self.getLangOpts().OpenCL)
- // FIXME: As compiler doesn't have any information about overlapping addr
- // spaces at the moment we have to be permissive here.
- return TC_NotApplicable;
- // Even though the logic below is general enough and can be applied to
- // non-OpenCL mode too, we fast-path above because no other languages
- // define overlapping address spaces currently.
+ unsigned &msg, CastKind &Kind) {
auto SrcType = SrcExpr.get()->getType();
auto SrcPtrType = SrcType->getAs<PointerType>();
if (!SrcPtrType)
@@ -2305,7 +2302,8 @@
auto DestPointeeType = DestPtrType->getPointeeType();
if (SrcPointeeType.getAddressSpace() == DestPointeeType.getAddressSpace())
return TC_NotApplicable;
- if (!DestPtrType->isAddressSpaceOverlapping(*SrcPtrType)) {
+ if (!Self.Context.isExplicitAddrSpaceConversionLegal(
+ SrcPointeeType.getQualifiers(), DestPointeeType.getQualifiers())) {
msg = diag::err_bad_cxx_cast_addr_space_mismatch;
return TC_Failed;
}
@@ -2313,10 +2311,12 @@
Self.Context.removeAddrSpaceQualType(SrcPointeeType.getCanonicalType());
auto DestPointeeTypeWithoutAS =
Self.Context.removeAddrSpaceQualType(DestPointeeType.getCanonicalType());
- return Self.Context.hasSameType(SrcPointeeTypeWithoutAS,
- DestPointeeTypeWithoutAS)
- ? TC_Success
- : TC_NotApplicable;
+ if (!Self.Context.hasSameType(SrcPointeeTypeWithoutAS,
+ DestPointeeTypeWithoutAS))
+ return TC_NotApplicable;
+
+ Kind = CK_AddressSpaceConversion;
+ return TC_Success;
}
void CastOperation::checkAddressSpaceCast(QualType SrcType, QualType DestType) {
@@ -2331,34 +2331,34 @@
// local int ** p;
// return (generic int **) p;
// warn even though local -> generic is permitted.
- if (Self.getLangOpts().OpenCL) {
- const Type *DestPtr, *SrcPtr;
- bool Nested = false;
- unsigned DiagID = diag::err_typecheck_incompatible_address_space;
- DestPtr = Self.getASTContext().getCanonicalType(DestType.getTypePtr()),
- SrcPtr = Self.getASTContext().getCanonicalType(SrcType.getTypePtr());
-
- while (isa<PointerType>(DestPtr) && isa<PointerType>(SrcPtr)) {
- const PointerType *DestPPtr = cast<PointerType>(DestPtr);
- const PointerType *SrcPPtr = cast<PointerType>(SrcPtr);
- QualType DestPPointee = DestPPtr->getPointeeType();
- QualType SrcPPointee = SrcPPtr->getPointeeType();
- if (Nested ? DestPPointee.getAddressSpace() !=
- SrcPPointee.getAddressSpace()
- : !DestPPtr->isAddressSpaceOverlapping(*SrcPPtr)) {
- Self.Diag(OpRange.getBegin(), DiagID)
- << SrcType << DestType << Sema::AA_Casting
- << SrcExpr.get()->getSourceRange();
- if (!Nested)
- SrcExpr = ExprError();
- return;
- }
-
- DestPtr = DestPPtr->getPointeeType().getTypePtr();
- SrcPtr = SrcPPtr->getPointeeType().getTypePtr();
- Nested = true;
- DiagID = diag::ext_nested_pointer_qualifier_mismatch;
+ const Type *DestPtr, *SrcPtr;
+ bool Nested = false;
+ unsigned DiagID = diag::err_typecheck_incompatible_address_space;
+ DestPtr = Self.getASTContext().getCanonicalType(DestType.getTypePtr()),
+ SrcPtr = Self.getASTContext().getCanonicalType(SrcType.getTypePtr());
+
+ while (isa<PointerType>(DestPtr) && isa<PointerType>(SrcPtr)) {
+ const PointerType *DestPPtr = cast<PointerType>(DestPtr);
+ const PointerType *SrcPPtr = cast<PointerType>(SrcPtr);
+ QualType DestPPointee = DestPPtr->getPointeeType();
+ QualType SrcPPointee = SrcPPtr->getPointeeType();
+ if (Nested ? DestPPointee.getAddressSpace() !=
+ SrcPPointee.getAddressSpace()
+ : !Self.Context.isExplicitAddrSpaceConversionLegal(
+ SrcPPointee.getAddressSpace(),
+ DestPPointee.getAddressSpace())) {
+ Self.Diag(OpRange.getBegin(), DiagID)
+ << SrcType << DestType << Sema::AA_Casting
+ << SrcExpr.get()->getSourceRange();
+ if (!Nested)
+ SrcExpr = ExprError();
+ return;
}
+
+ DestPtr = DestPPtr->getPointeeType().getTypePtr();
+ SrcPtr = SrcPPtr->getPointeeType().getTypePtr();
+ Nested = true;
+ DiagID = diag::ext_nested_pointer_qualifier_mismatch;
}
}
@@ -2447,7 +2447,8 @@
Sema::CheckedConversionKind CCK =
FunctionalStyle ? Sema::CCK_FunctionalCast : Sema::CCK_CStyleCast;
if (tcr == TC_NotApplicable) {
- tcr = TryAddressSpaceCast(Self, SrcExpr, DestType, /*CStyle*/ true, msg);
+ tcr = TryAddressSpaceCast(Self, SrcExpr, DestType, /*CStyle*/ true, msg,
+ Kind);
if (SrcExpr.isInvalid())
return;
if (tcr == TC_NotApplicable) {
Index: lib/AST/ASTContext.cpp
===================================================================
--- lib/AST/ASTContext.cpp
+++ lib/AST/ASTContext.cpp
@@ -8879,7 +8879,7 @@
Qualifiers RHSPteeQual = RHSPointee.getQualifiers();
// Blocks can't be an expression in a ternary operator (OpenCL v2.0
// 6.12.5) thus the following check is asymmetric.
- if (!LHSPteeQual.isAddressSpaceSupersetOf(RHSPteeQual))
+ if (!isAddressSpaceSupersetOf(LHSPteeQual, RHSPteeQual))
return {};
LHSPteeQual.removeAddressSpace();
RHSPteeQual.removeAddressSpace();
Index: include/clang/Basic/TargetInfo.h
===================================================================
--- include/clang/Basic/TargetInfo.h
+++ include/clang/Basic/TargetInfo.h
@@ -741,6 +741,22 @@
return UseAddrSpaceMapMangling;
}
+ /// Return true if address space A is a superspace of B.
+ /// By default, all target address spaces are disjoint.
+ virtual bool isAddressSpaceSupersetOf(LangAS A, LangAS B) const {
+ return false;
+ }
+
+ /// Return true if an explicit cast from address space From to To is legal.
+ /// This lets targets override the behavior when neither address space is a
+ /// true superset of the other, but the target still wants explicit casting
+ /// between them.
+ /// By default, explicit casting between all target address spaces is
+ /// permitted.
+ virtual bool isExplicitAddrSpaceConversionLegal(LangAS From, LangAS To) const{
+ return true;
+ }
+
///===---- Other target property query methods --------------------------===//
/// Appends the target-specific \#define values for this
Index: include/clang/AST/Type.h
===================================================================
--- include/clang/AST/Type.h
+++ include/clang/AST/Type.h
@@ -2561,22 +2561,6 @@
public:
QualType getPointeeType() const { return PointeeType; }
- /// Returns true if address spaces of pointers overlap.
- /// OpenCL v2.0 defines conversion rules for pointers to different
- /// address spaces (OpenCLC v2.0 s6.5.5) and notion of overlapping
- /// address spaces.
- /// CL1.1 or CL1.2:
- /// address spaces overlap iff they are they same.
- /// CL2.0 adds:
- /// __generic overlaps with any address space except for __constant.
- bool isAddressSpaceOverlapping(const PointerType &other) const {
- Qualifiers thisQuals = PointeeType.getQualifiers();
- Qualifiers otherQuals = other.getPointeeType().getQualifiers();
- // Address spaces overlap if at least one of them is a superset of another
- return thisQuals.isAddressSpaceSupersetOf(otherQuals) ||
- otherQuals.isAddressSpaceSupersetOf(thisQuals);
- }
-
bool isSugared() const { return false; }
QualType desugar() const { return QualType(this, 0); }
Index: include/clang/AST/ASTContext.h
===================================================================
--- include/clang/AST/ASTContext.h
+++ include/clang/AST/ASTContext.h
@@ -2528,6 +2528,60 @@
return AddrSpaceMapMangling || isTargetAddressSpace(AS);
}
+ /// Returns true if address space A is a superset of B.
+ bool isAddressSpaceSupersetOf(LangAS A, LangAS B) const {
+ // All address spaces are supersets of themselves.
+ if (A == B)
+ return true;
+
+ // OpenCL v2.0 defines conversion rules (OpenCLC v2.0 s6.5.5) and notion of
+ // overlapping address spaces.
+ // CL1.1 or CL1.2:
+ // every address space is a superset of itself.
+ // CL2.0 adds:
+ // __generic is a superset of any address space except for __constant.
+ if (A == LangAS::opencl_generic &&
+ B != LangAS::opencl_constant)
+ return true;
+
+ // Otherwise, ask the target.
+ return Target->isAddressSpaceSupersetOf(A, B);
+ }
+ bool isAddressSpaceSupersetOf(Qualifiers A, Qualifiers B) const {
+ return isAddressSpaceSupersetOf(A.getAddressSpace(), B.getAddressSpace());
+ }
+
+ /// Returns true if address space A overlaps with B.
+ bool isAddressSpaceOverlapping(LangAS A, LangAS B) const {
+ // A overlaps with B if either is a superset of the other.
+ return isAddressSpaceSupersetOf(A, B) ||
+ isAddressSpaceSupersetOf(B, A);
+ }
+ bool isAddressSpaceOverlapping(Qualifiers A, Qualifiers B) const {
+ return isAddressSpaceOverlapping(A.getAddressSpace(), B.getAddressSpace());
+ }
+
+ /// Returns true if an explicit cast from address space A to B is legal.
+ bool isExplicitAddrSpaceConversionLegal(LangAS From, LangAS To) const {
+ // If From and To overlap, the cast is legal.
+ if (isAddressSpaceOverlapping(From, To))
+ return true;
+
+ // Or, if either From or To are target address spaces, the target can
+ // decide whether or not to allow the cast regardless of overlap.
+ if (isTargetAddressSpace(From) || isTargetAddressSpace(To) ||
+ From == LangAS::Default || To == LangAS::Default)
+ return Target->isExplicitAddrSpaceConversionLegal(From, To);
+
+ // Otherwise, the cast is illegal.
+ return false;
+ }
+ bool isExplicitAddrSpaceConversionLegal(Qualifiers From,
+ Qualifiers To) const {
+ return isExplicitAddrSpaceConversionLegal(From.getAddressSpace(),
+ To.getAddressSpace());
+ }
+
private:
// Helper for integer ordering
unsigned getIntegerRank(const Type *T) const;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
