Skip to content

Commit 9d4b9c1

Browse files
Erich Keanebader
authored andcommitted
[SYCL] Fix regression introduced by f537293 (#1019)
This fixes the immediate problem, where diagnostics were being missed in kernels. The problem ends up being that diagnostics can happen at three different times during translation for templates: 1- During Phase 1 2- During Phase 2 instantiation 3- During ConstructOpenCLKernels. f537293 suppressed 3, and did a better job at suppressing 1. However, CallExprs are always rebuilt during instantation (for technical reasons). The result was the variadic call was being diagnosed in all 3 while developing f537293. I erronously thought the best way was to simply wait for instantiation to diagnose all SYCL diagnostics. Unfortunately, this was short-sighted. Not everything in tree-transform gets rebuilt every time. One perfect example is the inline-asm (see the added example). It doesn't get rebuilt in some cases, so it was never diagnosed during instantiation. Because I had fixed Phase 1/ConstructOpenCLKernels, this left no time for it to be diagnosed! The fix was to keep suppressing 3, and simply fix the call-expr check by moving the check to Sema::checkCall. This is how Clang works around this issue for warnings. The downside is that the diagnostics will happen slightly later for this (see the sycl-restrict.cpp test, which reverted to the previous version before f537293), but invalid programs still won't be accepted. Signed-off-by: Erich Keane <erich.keane@intel.com>
1 parent 7a7f47d commit 9d4b9c1

File tree

7 files changed

+32
-19
lines changed

7 files changed

+32
-19
lines changed

clang/lib/Sema/SemaChecking.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4458,6 +4458,12 @@ void Sema::checkCall(NamedDecl *FDecl, const FunctionProtoType *Proto,
44584458

44594459
if (FD)
44604460
diagnoseArgDependentDiagnoseIfAttrs(FD, ThisArg, Args, Loc);
4461+
4462+
// Diagnose variadic calls in SYCL.
4463+
if (FD && FD ->isVariadic() && getLangOpts().SYCLIsDevice &&
4464+
!isUnevaluatedContext() && !isKnownGoodSYCLDecl(FD))
4465+
SYCLDiagIfDeviceCode(Loc, diag::err_sycl_restrict)
4466+
<< Sema::KernelCallVariadicFunction;
44614467
}
44624468

44634469
/// CheckConstructorCall - Check a constructor call for correctness and safety

clang/lib/Sema/SemaExpr.cpp

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5404,12 +5404,6 @@ bool Sema::GatherArgumentsForCall(SourceLocation CallLoc, FunctionDecl *FDecl,
54045404

54055405
// Otherwise do argument promotion, (C99 6.5.2.2p7).
54065406
} else {
5407-
// Diagnose variadic calls in SYCL.
5408-
if (getLangOpts().SYCLIsDevice && !isUnevaluatedContext() &&
5409-
!isKnownGoodSYCLDecl(FDecl))
5410-
SYCLDiagIfDeviceCode(CallLoc, diag::err_sycl_restrict)
5411-
<< Sema::KernelCallVariadicFunction;
5412-
54135407
for (Expr *A : Args.slice(ArgIx)) {
54145408
ExprResult Arg = DefaultVariadicArgumentPromotion(A, CallType, FDecl);
54155409
Invalid |= Arg.isInvalid();

clang/lib/Sema/SemaOverload.cpp

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14230,11 +14230,6 @@ Sema::BuildCallToObjectOfClassType(Scope *S, Expr *Obj,
1423014230

1423114231
// If this is a variadic call, handle args passed through "...".
1423214232
if (Proto->isVariadic()) {
14233-
// Diagnose variadic calls in SYCL.
14234-
if (getLangOpts().SYCLIsDevice && !isUnevaluatedContext())
14235-
SYCLDiagIfDeviceCode(LParenLoc, diag::err_sycl_restrict)
14236-
<< Sema::KernelCallVariadicFunction;
14237-
1423814233
// Promote the arguments (C99 6.5.2.2p7).
1423914234
for (unsigned i = NumParams, e = Args.size(); i < e; i++) {
1424014235
ExprResult Arg = DefaultVariadicArgumentPromotion(Args[i], VariadicMethod,

clang/lib/Sema/SemaSYCL.cpp

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1393,10 +1393,6 @@ static bool isKnownEmitted(Sema &S, FunctionDecl *FD) {
13931393
if (!FD)
13941394
return true; // Seen in LIT testing
13951395

1396-
// Templates are emitted when they're instantiated.
1397-
if (FD->isDependentContext())
1398-
return false;
1399-
14001396
if (FD->hasAttr<SYCLDeviceAttr>() || FD->hasAttr<SYCLKernelAttr>())
14011397
return true;
14021398

@@ -1411,7 +1407,7 @@ Sema::DeviceDiagBuilder Sema::SYCLDiagIfDeviceCode(SourceLocation Loc,
14111407
"Should only be called during SYCL compilation");
14121408
FunctionDecl *FD = dyn_cast<FunctionDecl>(getCurLexicalContext());
14131409
DeviceDiagBuilder::Kind DiagKind = [this, FD] {
1414-
if (ConstructingOpenCLKernel || (FD && FD->isDependentContext()))
1410+
if (ConstructingOpenCLKernel)
14151411
return DeviceDiagBuilder::K_Nop;
14161412
else if (isKnownEmitted(*this, FD))
14171413
return DeviceDiagBuilder::K_ImmediateWithCallStack;

clang/test/SemaSYCL/inline-asm.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,11 @@ template <typename name, typename Func>
2323
__attribute__((sycl_kernel)) void kernel_single_task(Func kernelFunc) {
2424
// expected-note@+1 {{called by 'kernel_single_task<fake_kernel, (lambda}}
2525
kernelFunc();
26+
#ifdef LINUX_ASM
27+
__asm__("int3"); // expected-error {{SYCL kernel cannot use inline assembly}}
28+
#else
29+
__asm int 3 // expected-error {{SYCL kernel cannot use inline assembly}}
30+
#endif // LINUX_ASM
2631
}
2732

2833
int main() {
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
// RUN: %clang_cc1 -fcxx-exceptions -fsycl-is-device -verify -fsyntax-only -std=c++17 %s
2+
3+
template <typename name, typename Func>
4+
__attribute__((sycl_kernel))
5+
void kernel_single_task(Func kernelFunc) {
6+
// expected-note@+1 {{called by 'kernel_single_task}}
7+
kernelFunc();
8+
}
9+
10+
void foo() {
11+
// expected-error@+1 {{SYCL kernel cannot use exceptions}}
12+
throw 3;
13+
}
14+
15+
int main() {
16+
// expected-note@+1 {{called by 'operator()'}}
17+
kernel_single_task<class fake_kernel>([]() { foo(); });
18+
}

clang/test/SemaSYCL/sycl-restrict.cpp

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,6 @@ void eh_not_ok(void)
124124

125125
void usage(myFuncDef functionPtr) {
126126

127-
// expected-note@+1 {{called by 'usage'}}
128127
eh_not_ok();
129128

130129
#if ALLOW_FP
@@ -170,6 +169,7 @@ int use2 ( a_type ab, a_type *abp ) {
170169
return ns::glob +
171170
// expected-error@+1 {{SYCL kernel cannot use a global variable}}
172171
AnotherNS::moar_globals;
172+
// expected-note@+1 {{called by 'use2'}}
173173
eh_not_ok();
174174
Check_RTTI_Restriction:: A *a;
175175
Check_RTTI_Restriction:: isa_B(a);
@@ -180,16 +180,15 @@ int use2 ( a_type ab, a_type *abp ) {
180180

181181
template <typename name, typename Func>
182182
__attribute__((sycl_kernel)) void kernel_single_task(Func kernelFunc) {
183-
// expected-note@+1 {{called by 'kernel_single_task}}
184183
kernelFunc();
185184
a_type ab;
186185
a_type *p;
186+
// expected-note@+1 {{called by 'kernel_single_task'}}
187187
use2(ab, p);
188188
}
189189

190190
int main() {
191191
a_type ab;
192-
// expected-note@+1 {{called by 'operator()'}}
193192
kernel_single_task<class fake_kernel>([]() { usage( &addInt ); });
194193
return 0;
195194
}

0 commit comments

Comments
 (0)