Skip to content

[NFC][SYCL] Fix some uses of stringref& and qualtype& #3504

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Apr 8, 2021
Merged

[NFC][SYCL] Fix some uses of stringref& and qualtype& #3504

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d409f95
[NFC][SYCL] Fix some uses of stringref& and qualtype&
Apr 7, 2021
e190e73
Better clean up DeclContextDesc uses, add a 'make' function to ensure…
Apr 7, 2021
bdcb819
Aaron's suggestion to remove llvm::
Apr 7, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
133 changes: 68 additions & 65 deletions clang/lib/Sema/SemaSYCL.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,38 +68,49 @@ namespace {
/// Various utilities.
class Util {
public:
using DeclContextDesc = std::pair<clang::Decl::Kind, StringRef>;
using DeclContextDesc = std::pair<Decl::Kind, StringRef>;

template <size_t N>
static constexpr DeclContextDesc MakeDeclContextDesc(Decl::Kind K,
const char (&Str)[N]) {
return DeclContextDesc{K, llvm::StringLiteral{Str}};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clang doesn't compile this code:

/home/runner/work/llvm/llvm/src/clang/lib/Sema/SemaSYCL.cpp:76:31: error: no matching constructor for initialization of 'llvm::StringLiteral'
    return DeclContextDesc{K, llvm::StringLiteral{Str}};

Copy link
Contributor Author

@erichkeane erichkeane Apr 9, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see! I'll push a fixing patch for this ASAP.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh... well something strange is going on with this @bader. As far as I can tell, that code should be completely valid, and it is odd it only happens in that single configuration. I can't reproduce it in other configs either (nor does googling the important note give any hint).

That said, I've fixed it in this commit here: #3520

}

static constexpr DeclContextDesc MakeDeclContextDesc(Decl::Kind K,
StringRef SR) {
return DeclContextDesc{K, SR};
}

/// Checks whether given clang type is a full specialization of the SYCL
/// accessor class.
static bool isSyclAccessorType(const QualType &Ty);
static bool isSyclAccessorType(QualType Ty);

/// Checks whether given clang type is a full specialization of the SYCL
/// sampler class.
static bool isSyclSamplerType(const QualType &Ty);
static bool isSyclSamplerType(QualType Ty);

/// Checks whether given clang type is a full specialization of the SYCL
/// stream class.
static bool isSyclStreamType(const QualType &Ty);
static bool isSyclStreamType(QualType Ty);

/// Checks whether given clang type is a full specialization of the SYCL
/// half class.
static bool isSyclHalfType(const QualType &Ty);
static bool isSyclHalfType(QualType Ty);

/// Checks whether given clang type is a full specialization of the SYCL
/// accessor_property_list class.
static bool isAccessorPropertyListType(const QualType &Ty);
static bool isAccessorPropertyListType(QualType Ty);

/// Checks whether given clang type is a full specialization of the SYCL
/// buffer_location class.
static bool isSyclBufferLocationType(const QualType &Ty);
static bool isSyclBufferLocationType(QualType Ty);

/// Checks whether given clang type is a standard SYCL API class with given
/// name.
/// \param Ty the clang type being checked
/// \param Name the class name checked against
/// \param Tmpl whether the class is template instantiation or simple record
static bool isSyclType(const QualType &Ty, StringRef Name, bool Tmpl = false);
static bool isSyclType(QualType Ty, StringRef Name, bool Tmpl = false);

/// Checks whether given function is a standard SYCL API function with given
/// name.
Expand All @@ -109,11 +120,11 @@ class Util {

/// Checks whether given clang type is a full specialization of the SYCL
/// specialization constant class.
static bool isSyclSpecConstantType(const QualType &Ty);
static bool isSyclSpecConstantType(QualType Ty);

/// Checks whether given clang type is a full specialization of the SYCL
/// kernel_handler class.
static bool isSyclKernelHandlerType(const QualType &Ty);
static bool isSyclKernelHandlerType(QualType Ty);

// Checks declaration context hierarchy.
/// \param DC the context of the item to be checked.
Expand All @@ -127,7 +138,7 @@ class Util {
/// \param Ty the clang type being checked
/// \param Scopes the declaration scopes leading from the type to the
/// translation unit (excluding the latter)
static bool matchQualifiedTypeName(const QualType &Ty,
static bool matchQualifiedTypeName(QualType Ty,
ArrayRef<Util::DeclContextDesc> Scopes);
};

Expand Down Expand Up @@ -1321,7 +1332,7 @@ class SyclKernelFieldChecker : public SyclKernelFieldHandler {
DiagnosticsEngine &Diag;
// Check whether the object should be disallowed from being copied to kernel.
// Return true if not copyable, false if copyable.
bool checkNotCopyableToKernel(const FieldDecl *FD, const QualType &FieldTy) {
bool checkNotCopyableToKernel(const FieldDecl *FD, QualType FieldTy) {
if (FieldTy->isArrayType()) {
if (const auto *CAT =
SemaRef.getASTContext().getAsConstantArrayType(FieldTy)) {
Expand Down Expand Up @@ -4305,70 +4316,62 @@ bool SYCLIntegrationFooter::emit(raw_ostream &O) {
// Utility class methods
// -----------------------------------------------------------------------------

bool Util::isSyclAccessorType(const QualType &Ty) {
bool Util::isSyclAccessorType(QualType Ty) {
return isSyclType(Ty, "accessor", true /*Tmpl*/);
}

bool Util::isSyclSamplerType(const QualType &Ty) {
return isSyclType(Ty, "sampler");
}
bool Util::isSyclSamplerType(QualType Ty) { return isSyclType(Ty, "sampler"); }

bool Util::isSyclStreamType(const QualType &Ty) {
return isSyclType(Ty, "stream");
}
bool Util::isSyclStreamType(QualType Ty) { return isSyclType(Ty, "stream"); }

bool Util::isSyclHalfType(const QualType &Ty) {
const StringRef &Name = "half";
bool Util::isSyclHalfType(QualType Ty) {
std::array<DeclContextDesc, 5> Scopes = {
Util::DeclContextDesc{clang::Decl::Kind::Namespace, "cl"},
Util::DeclContextDesc{clang::Decl::Kind::Namespace, "sycl"},
Util::DeclContextDesc{clang::Decl::Kind::Namespace, "detail"},
Util::DeclContextDesc{clang::Decl::Kind::Namespace, "half_impl"},
Util::DeclContextDesc{Decl::Kind::CXXRecord, Name}};
Util::MakeDeclContextDesc(Decl::Kind::Namespace, "cl"),
Util::MakeDeclContextDesc(Decl::Kind::Namespace, "sycl"),
Util::MakeDeclContextDesc(Decl::Kind::Namespace, "detail"),
Util::MakeDeclContextDesc(Decl::Kind::Namespace, "half_impl"),
Util::MakeDeclContextDesc(Decl::Kind::CXXRecord, "half")};
return matchQualifiedTypeName(Ty, Scopes);
}

bool Util::isSyclSpecConstantType(const QualType &Ty) {
const StringRef &Name = "spec_constant";
bool Util::isSyclSpecConstantType(QualType Ty) {
std::array<DeclContextDesc, 5> Scopes = {
Util::DeclContextDesc{clang::Decl::Kind::Namespace, "cl"},
Util::DeclContextDesc{clang::Decl::Kind::Namespace, "sycl"},
Util::DeclContextDesc{clang::Decl::Kind::Namespace, "ONEAPI"},
Util::DeclContextDesc{clang::Decl::Kind::Namespace, "experimental"},
Util::DeclContextDesc{Decl::Kind::ClassTemplateSpecialization, Name}};
Util::MakeDeclContextDesc(Decl::Kind::Namespace, "cl"),
Util::MakeDeclContextDesc(Decl::Kind::Namespace, "sycl"),
Util::MakeDeclContextDesc(Decl::Kind::Namespace, "ONEAPI"),
Util::MakeDeclContextDesc(Decl::Kind::Namespace, "experimental"),
Util::MakeDeclContextDesc(Decl::Kind::ClassTemplateSpecialization,
"spec_constant")};
return matchQualifiedTypeName(Ty, Scopes);
}

bool Util::isSyclKernelHandlerType(const QualType &Ty) {
const StringRef &Name = "kernel_handler";
bool Util::isSyclKernelHandlerType(QualType Ty) {
std::array<DeclContextDesc, 3> Scopes = {
Util::DeclContextDesc{clang::Decl::Kind::Namespace, "cl"},
Util::DeclContextDesc{clang::Decl::Kind::Namespace, "sycl"},
Util::DeclContextDesc{Decl::Kind::CXXRecord, Name}};
Util::MakeDeclContextDesc(Decl::Kind::Namespace, "cl"),
Util::MakeDeclContextDesc(Decl::Kind::Namespace, "sycl"),
Util::MakeDeclContextDesc(Decl::Kind::CXXRecord, "kernel_handler")};
return matchQualifiedTypeName(Ty, Scopes);
}

bool Util::isSyclBufferLocationType(const QualType &Ty) {
const StringRef &PropertyName = "buffer_location";
const StringRef &InstanceName = "instance";
bool Util::isSyclBufferLocationType(QualType Ty) {
std::array<DeclContextDesc, 6> Scopes = {
Util::DeclContextDesc{Decl::Kind::Namespace, "cl"},
Util::DeclContextDesc{Decl::Kind::Namespace, "sycl"},
Util::DeclContextDesc{Decl::Kind::Namespace, "INTEL"},
Util::DeclContextDesc{Decl::Kind::Namespace, "property"},
Util::DeclContextDesc{Decl::Kind::CXXRecord, PropertyName},
Util::DeclContextDesc{Decl::Kind::ClassTemplateSpecialization,
InstanceName}};
Util::MakeDeclContextDesc(Decl::Kind::Namespace, "cl"),
Util::MakeDeclContextDesc(Decl::Kind::Namespace, "sycl"),
Util::MakeDeclContextDesc(Decl::Kind::Namespace, "INTEL"),
Util::MakeDeclContextDesc(Decl::Kind::Namespace, "property"),
Util::MakeDeclContextDesc(Decl::Kind::CXXRecord, "buffer_location"),
Util::MakeDeclContextDesc(Decl::Kind::ClassTemplateSpecialization,
"instance")};
return matchQualifiedTypeName(Ty, Scopes);
}

bool Util::isSyclType(const QualType &Ty, StringRef Name, bool Tmpl) {
bool Util::isSyclType(QualType Ty, StringRef Name, bool Tmpl) {
Decl::Kind ClassDeclKind =
Tmpl ? Decl::Kind::ClassTemplateSpecialization : Decl::Kind::CXXRecord;
std::array<DeclContextDesc, 3> Scopes = {
Util::DeclContextDesc{clang::Decl::Kind::Namespace, "cl"},
Util::DeclContextDesc{clang::Decl::Kind::Namespace, "sycl"},
Util::DeclContextDesc{ClassDeclKind, Name}};
Util::MakeDeclContextDesc(Decl::Kind::Namespace, "cl"),
Util::MakeDeclContextDesc(Decl::Kind::Namespace, "sycl"),
Util::MakeDeclContextDesc(ClassDeclKind, Name)};
return matchQualifiedTypeName(Ty, Scopes);
}

Expand All @@ -4382,18 +4385,18 @@ bool Util::isSyclFunction(const FunctionDecl *FD, StringRef Name) {
return false;

std::array<DeclContextDesc, 2> Scopes = {
Util::DeclContextDesc{clang::Decl::Kind::Namespace, "cl"},
Util::DeclContextDesc{clang::Decl::Kind::Namespace, "sycl"}};
Util::MakeDeclContextDesc(Decl::Kind::Namespace, "cl"),
Util::MakeDeclContextDesc(Decl::Kind::Namespace, "sycl")};
return matchContext(DC, Scopes);
}

bool Util::isAccessorPropertyListType(const QualType &Ty) {
const StringRef &Name = "accessor_property_list";
bool Util::isAccessorPropertyListType(QualType Ty) {
std::array<DeclContextDesc, 4> Scopes = {
Util::DeclContextDesc{clang::Decl::Kind::Namespace, "cl"},
Util::DeclContextDesc{clang::Decl::Kind::Namespace, "sycl"},
Util::DeclContextDesc{clang::Decl::Kind::Namespace, "ONEAPI"},
Util::DeclContextDesc{Decl::Kind::ClassTemplateSpecialization, Name}};
Util::MakeDeclContextDesc(Decl::Kind::Namespace, "cl"),
Util::MakeDeclContextDesc(Decl::Kind::Namespace, "sycl"),
Util::MakeDeclContextDesc(Decl::Kind::Namespace, "ONEAPI"),
Util::MakeDeclContextDesc(Decl::Kind::ClassTemplateSpecialization,
"accessor_property_list")};
return matchQualifiedTypeName(Ty, Scopes);
}

Expand All @@ -4405,17 +4408,17 @@ bool Util::matchContext(const DeclContext *Ctx,
StringRef Name = "";

for (const auto &Scope : llvm::reverse(Scopes)) {
clang::Decl::Kind DK = Ctx->getDeclKind();
Decl::Kind DK = Ctx->getDeclKind();
if (DK != Scope.first)
return false;

switch (DK) {
case clang::Decl::Kind::ClassTemplateSpecialization:
case Decl::Kind::ClassTemplateSpecialization:
// ClassTemplateSpecializationDecl inherits from CXXRecordDecl
case clang::Decl::Kind::CXXRecord:
case Decl::Kind::CXXRecord:
Name = cast<CXXRecordDecl>(Ctx)->getName();
break;
case clang::Decl::Kind::Namespace:
case Decl::Kind::Namespace:
Name = cast<NamespaceDecl>(Ctx)->getName();
break;
default:
Expand All @@ -4428,7 +4431,7 @@ bool Util::matchContext(const DeclContext *Ctx,
return Ctx->isTranslationUnit();
}

bool Util::matchQualifiedTypeName(const QualType &Ty,
bool Util::matchQualifiedTypeName(QualType Ty,
ArrayRef<Util::DeclContextDesc> Scopes) {
const CXXRecordDecl *RecTy = Ty->getAsCXXRecordDecl();

Expand Down