Skip to content

Commit cb66bf2

Browse files
author
Erich Keane
committed
Replace 'magic static' with a member variable for SCYL kernel names
I discovered when merging the __builtin_sycl_unique_stable_name into my downstream that it is actually possible for the cc1 invocation to have more than 1 Sema instance, if you pass it multiple input files, each gets its own Sema instance and thus ASTContext instance. The result was that the call to Filter the SYCL kernels was using an ItaniumMangleContext stored via a 'magic static', so it had an invalid reference to ASTContext when processing the 2nd failure. The failure is unfortunately flakey/transient, but the test that fails was added anyway. The magic-static was switched to a unique_ptr member variable in ASTContext that is initialized when needed.
1 parent 0d5219f commit cb66bf2

File tree

3 files changed

+35
-10
lines changed

3 files changed

+35
-10
lines changed

clang/include/clang/AST/ASTContext.h

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,7 @@ class DynTypedNode;
103103
class DynTypedNodeList;
104104
class Expr;
105105
class GlobalDecl;
106+
class ItaniumMangleContext;
106107
class MangleContext;
107108
class MangleNumberingContext;
108109
class MaterializeTemporaryExpr;
@@ -3166,7 +3167,7 @@ OPT_LIST(V)
31663167

31673168
void AddSYCLKernelNamingDecl(const CXXRecordDecl *RD);
31683169
bool IsSYCLKernelNamingDecl(const NamedDecl *RD) const;
3169-
unsigned GetSYCLKernelNamingIndex(const NamedDecl *RD) const;
3170+
unsigned GetSYCLKernelNamingIndex(const NamedDecl *RD);
31703171
/// A SourceLocation to store whether we have evaluated a kernel name already,
31713172
/// and where it happened. If so, we need to diagnose an illegal use of the
31723173
/// builtin.
@@ -3185,6 +3186,10 @@ OPT_LIST(V)
31853186
llvm::DenseMap<const DeclContext *,
31863187
llvm::SmallPtrSet<const CXXRecordDecl *, 4>>
31873188
SYCLKernelNamingTypes;
3189+
std::unique_ptr<ItaniumMangleContext> SYCLKernelFilterContext;
3190+
void FilterSYCLKernelNamingDecls(
3191+
const CXXRecordDecl *RD,
3192+
llvm::SmallVectorImpl<const CXXRecordDecl *> &Decls);
31883193
};
31893194

31903195
/// Insertion operator for diagnostics.

clang/lib/AST/ASTContext.cpp

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11715,25 +11715,27 @@ bool ASTContext::IsSYCLKernelNamingDecl(const NamedDecl *ND) const {
1171511715

1171611716
// Filters the Decls list to those that share the lambda mangling with the
1171711717
// passed RD.
11718-
static void FilterSYCLKernelNamingDecls(
11719-
ASTContext &Ctx, const CXXRecordDecl *RD,
11718+
void ASTContext::FilterSYCLKernelNamingDecls(
11719+
const CXXRecordDecl *RD,
1172011720
llvm::SmallVectorImpl<const CXXRecordDecl *> &Decls) {
11721-
static std::unique_ptr<ItaniumMangleContext> Mangler{
11722-
ItaniumMangleContext::create(Ctx, Ctx.getDiagnostics())};
11721+
11722+
if (!SYCLKernelFilterContext)
11723+
SYCLKernelFilterContext.reset(
11724+
ItaniumMangleContext::create(*this, getDiagnostics()));
1172311725

1172411726
llvm::SmallString<128> LambdaSig;
1172511727
llvm::raw_svector_ostream Out(LambdaSig);
11726-
Mangler->mangleLambdaSig(RD, Out);
11728+
SYCLKernelFilterContext->mangleLambdaSig(RD, Out);
1172711729

11728-
llvm::erase_if(Decls, [&LambdaSig](const CXXRecordDecl *LocalRD) {
11730+
llvm::erase_if(Decls, [this, &LambdaSig](const CXXRecordDecl *LocalRD) {
1172911731
llvm::SmallString<128> LocalLambdaSig;
1173011732
llvm::raw_svector_ostream LocalOut(LocalLambdaSig);
11731-
Mangler->mangleLambdaSig(LocalRD, LocalOut);
11733+
SYCLKernelFilterContext->mangleLambdaSig(LocalRD, LocalOut);
1173211734
return LambdaSig != LocalLambdaSig;
1173311735
});
1173411736
}
1173511737

11736-
unsigned ASTContext::GetSYCLKernelNamingIndex(const NamedDecl *ND) const {
11738+
unsigned ASTContext::GetSYCLKernelNamingIndex(const NamedDecl *ND) {
1173711739
assert(getLangOpts().isSYCL() && "Only valid for SYCL programs");
1173811740
assert(IsSYCLKernelNamingDecl(ND) &&
1173911741
"Lambda not involved in mangling asked for a naming index?");
@@ -11753,7 +11755,7 @@ unsigned ASTContext::GetSYCLKernelNamingIndex(const NamedDecl *ND) const {
1175311755
// doesn't use the itanium mangler, and just sets the lambda mangling number
1175411756
// incrementally, with no consideration to the signature.
1175511757
if (Target->getCXXABI().getKind() != TargetCXXABI::Microsoft)
11756-
FilterSYCLKernelNamingDecls(const_cast<ASTContext &>(*this), RD, Decls);
11758+
FilterSYCLKernelNamingDecls(RD, Decls);
1175711759

1175811760
llvm::sort(Decls, [](const CXXRecordDecl *LHS, const CXXRecordDecl *RHS) {
1175911761
return LHS->getLambdaManglingNumber() < RHS->getLambdaManglingNumber();
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
// RUN: %clang_cc1 %s %s -std=c++17 -triple x86_64-linux-gnu -fsycl-is-device -verify -fsyntax-only -Wno-unused
2+
3+
// This would crash due to the double-inputs, since the 'magic static' use in
4+
// the AST Context SCYL Filtering would end up caching an old version of the
5+
// ASTContext object, which no longer exists in the second file's invocation.
6+
//
7+
// expected-no-diagnostics
8+
class Empty {};
9+
template <typename, typename F> __attribute__((sycl_kernel)) void kernel(F) {
10+
__builtin_sycl_unique_stable_name(F);
11+
}
12+
13+
void use() {
14+
[](Empty) {
15+
auto lambda = []{};
16+
kernel<class i>(lambda);
17+
};
18+
}

0 commit comments

Comments
 (0)