Skip to content

[SYCL] Move variadic call diagnostics to delayed diagnostics, fixes. #998

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.

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
Jan 15, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 6 additions & 0 deletions clang/include/clang/Sema/Sema.h
Original file line number Diff line number Diff line change
Expand Up @@ -12015,6 +12015,11 @@ class Sema final {
// is associated with.
std::unique_ptr<SYCLIntegrationHeader> SyclIntHeader;

// Used to suppress diagnostics during kernel construction, since these were
// already emitted earlier. Diagnosing during Kernel emissions also skips the
// useful notes that shows where the kernel was called.
bool ConstructingOpenCLKernel = false;

public:
void addSyclDeviceDecl(Decl *d) { SyclDeviceDecls.push_back(d); }
SmallVectorImpl<Decl *> &syclDeviceDecls() { return SyclDeviceDecls; }
Expand All @@ -12041,6 +12046,7 @@ class Sema final {
KernelCallVariadicFunction
};
DeviceDiagBuilder SYCLDiagIfDeviceCode(SourceLocation Loc, unsigned DiagID);
bool isKnownGoodSYCLDecl(const Decl *D);
void ConstructOpenCLKernel(FunctionDecl *KernelCallerFunc, MangleContext &MC);
void MarkDevice(void);
bool CheckSYCLCall(SourceLocation Loc, FunctionDecl *Callee);
Expand Down
6 changes: 6 additions & 0 deletions clang/lib/Sema/SemaExpr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5380,6 +5380,12 @@ bool Sema::GatherArgumentsForCall(SourceLocation CallLoc, FunctionDecl *FDecl,

// Otherwise do argument promotion, (C99 6.5.2.2p7).
} else {
// Diagnose variadic calls in SYCL.
if (getLangOpts().SYCLIsDevice && !isUnevaluatedContext() &&
!isKnownGoodSYCLDecl(FDecl))
SYCLDiagIfDeviceCode(CallLoc, diag::err_sycl_restrict)
<< Sema::KernelCallVariadicFunction;

for (Expr *A : Args.slice(ArgIx)) {
ExprResult Arg = DefaultVariadicArgumentPromotion(A, CallType, FDecl);
Invalid |= Arg.isInvalid();
Expand Down
5 changes: 5 additions & 0 deletions clang/lib/Sema/SemaOverload.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14027,6 +14027,11 @@ Sema::BuildCallToObjectOfClassType(Scope *S, Expr *Obj,

// If this is a variadic call, handle args passed through "...".
if (Proto->isVariadic()) {
// Diagnose variadic calls in SYCL.
if (getLangOpts().SYCLIsDevice && !isUnevaluatedContext())
SYCLDiagIfDeviceCode(LParenLoc, diag::err_sycl_restrict)
<< Sema::KernelCallVariadicFunction;

// Promote the arguments (C99 6.5.2.2p7).
for (unsigned i = NumParams, e = Args.size(); i < e; i++) {
ExprResult Arg = DefaultVariadicArgumentPromotion(Args[i], VariadicMethod,
Expand Down
39 changes: 14 additions & 25 deletions clang/lib/Sema/SemaSYCL.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ static bool IsSyclMathFunc(unsigned BuiltinID) {
return true;
}

static bool isKnownGoodDecl(const Decl *D) {
bool Sema::isKnownGoodSYCLDecl(const Decl *D) {
if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(D)) {
const IdentifierInfo *II = FD->getIdentifier();
const DeclContext *DC = FD->getDeclContext();
Expand Down Expand Up @@ -326,7 +326,7 @@ class MarkDeviceFunction : public RecursiveASTVisitor<MarkDeviceFunction> {

bool VisitDeclRefExpr(DeclRefExpr *E) {
Decl* D = E->getDecl();
if (isKnownGoodDecl(D))
if (SemaRef.isKnownGoodSYCLDecl(D))
return true;

CheckSYCLType(E->getType(), E->getSourceRange());
Expand Down Expand Up @@ -372,18 +372,6 @@ class MarkDeviceFunction : public RecursiveASTVisitor<MarkDeviceFunction> {
return true;
}

bool VisitGCCAsmStmt(GCCAsmStmt *S) {
SemaRef.Diag(S->getBeginLoc(), diag::err_sycl_restrict)
<< Sema::KernelUseAssembly;
return true;
}

bool VisitMSAsmStmt(MSAsmStmt *S) {
SemaRef.Diag(S->getBeginLoc(), diag::err_sycl_restrict)
<< Sema::KernelUseAssembly;
return true;
}

// The call graph for this translation unit.
Copy link
Contributor

Choose a reason for hiding this comment

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

Were these dead code and safe to remove?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They were not, but are now :) When This is the case in the commit message where we had done 2 versions of diagnostics for the inline assembly, but bugs in each made them not cover all cases, but combined, they seemed to. My fix to the delayed diagnostics made the assembly error handling in normal 'Sema' cover the cases that this caught.

CallGraph SYCLCG;
// The set of functions called by a kernel function.
Expand Down Expand Up @@ -549,9 +537,6 @@ class MarkDeviceFunction : public RecursiveASTVisitor<MarkDeviceFunction> {
}
}
} else if (const auto *FPTy = dyn_cast<FunctionProtoType>(Ty)) {
if (FPTy->isVariadic() && SemaRef.getLangOpts().SYCLIsDevice)
SemaRef.SYCLDiagIfDeviceCode(Loc.getBegin(), diag::err_sycl_restrict)
<< Sema::KernelCallVariadicFunction;
for (const auto &ParamTy : FPTy->param_types())
if (!CheckSYCLType(ParamTy, Loc, Visited))
return false;
Expand Down Expand Up @@ -1308,8 +1293,10 @@ void Sema::ConstructOpenCLKernel(FunctionDecl *KernelCallerFunc,
// in different translation units.
OpenCLKernel->setImplicitlyInline(KernelCallerFunc->isInlined());

ConstructingOpenCLKernel = true;
CompoundStmt *OpenCLKernelBody =
CreateOpenCLKernelBody(*this, KernelCallerFunc, OpenCLKernel);
ConstructingOpenCLKernel = false;
OpenCLKernel->setBody(OpenCLKernelBody);
addSyclDeviceDecl(OpenCLKernel);
}
Expand Down Expand Up @@ -1404,13 +1391,13 @@ static bool isKnownEmitted(Sema &S, FunctionDecl *FD) {
if (!FD)
return true; // Seen in LIT testing

if (FD->hasAttr<SYCLDeviceAttr>() || FD->hasAttr<SYCLKernelAttr>())
return true;

// Templates are emitted when they're instantiated.
if (FD->isDependentContext())
return false;

if (FD->hasAttr<SYCLDeviceAttr>() || FD->hasAttr<SYCLKernelAttr>())
return true;

// Otherwise, the function is known-emitted if it's in our set of
// known-emitted functions.
return S.DeviceKnownEmittedFns.count(FD) > 0;
Expand All @@ -1420,18 +1407,20 @@ Sema::DeviceDiagBuilder Sema::SYCLDiagIfDeviceCode(SourceLocation Loc,
unsigned DiagID) {
assert(getLangOpts().SYCLIsDevice &&
"Should only be called during SYCL compilation");
DeviceDiagBuilder::Kind DiagKind = [this] {
if (isKnownEmitted(*this, dyn_cast<FunctionDecl>(CurContext)))
FunctionDecl *FD = dyn_cast<FunctionDecl>(getCurLexicalContext());
DeviceDiagBuilder::Kind DiagKind = [this, FD] {
if (ConstructingOpenCLKernel || (FD && FD->isDependentContext()))
return DeviceDiagBuilder::K_Nop;
else if (isKnownEmitted(*this, FD))
return DeviceDiagBuilder::K_ImmediateWithCallStack;
return DeviceDiagBuilder::K_Deferred;
}();
return DeviceDiagBuilder(DiagKind, Loc, DiagID,
dyn_cast<FunctionDecl>(CurContext), *this);
return DeviceDiagBuilder(DiagKind, Loc, DiagID, FD, *this);
}

bool Sema::CheckSYCLCall(SourceLocation Loc, FunctionDecl *Callee) {
assert(Callee && "Callee may not be null.");
FunctionDecl *Caller = getCurFunctionDecl();
FunctionDecl *Caller = dyn_cast<FunctionDecl>(getCurLexicalContext());

// If the caller is known-emitted, mark the callee as known-emitted.
// Otherwise, mark the call in our call graph so we can traverse it later.
Expand Down
1 change: 0 additions & 1 deletion clang/lib/Sema/SemaType.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4685,7 +4685,6 @@ static TypeSourceInfo *GetFullTypeForDeclarator(TypeProcessingState &state,
// OpenCL doesn't support variadic functions and blocks
// (s6.9.e and s6.12.5 OpenCL v2.0) except for printf.
// We also allow here any toolchain reserved identifiers.
// FIXME: Use deferred diagnostics engine to skip host side issues.
if (FTI.isVariadic &&
!LangOpts.SYCLIsDevice &&
!(D.getIdentifier() &&
Expand Down
2 changes: 2 additions & 0 deletions clang/test/SemaSYCL/inline-asm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,13 @@ void bar() {

template <typename name, typename Func>
__attribute__((sycl_kernel)) void kernel_single_task(Func kernelFunc) {
// expected-note@+1 {{called by 'kernel_single_task<fake_kernel, (lambda}}
kernelFunc();
}

int main() {
foo();
// expected-note@+1 {{called by 'operator()'}}
kernel_single_task<class fake_kernel>([]() { bar(); });
return 0;
}
2 changes: 2 additions & 0 deletions clang/test/SemaSYCL/sycl-cconv.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,12 @@ __cdecl __attribute__((sycl_kernel)) void kernel_single_task(Func kernelFunc) {
printf("cannot call from here\n");
// expected-no-error@+1
moo();
// expected-note@+1{{called by}}
kernelFunc();
}

int main() {
//expected-note@+2 {{in instantiation of}}
//expected-error@+1 {{SYCL kernel cannot call a variadic function}}
kernel_single_task<class fake_kernel>([]() { printf("world\n"); });
bar();
Expand Down
5 changes: 3 additions & 2 deletions clang/test/SemaSYCL/sycl-restrict.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ void eh_not_ok(void)

void usage(myFuncDef functionPtr) {

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

#if ALLOW_FP
Expand Down Expand Up @@ -169,7 +170,6 @@ int use2 ( a_type ab, a_type *abp ) {
return ns::glob +
// expected-error@+1 {{SYCL kernel cannot use a global variable}}
AnotherNS::moar_globals;
// expected-note@+1 {{called by 'use2'}}
eh_not_ok();
Check_RTTI_Restriction:: A *a;
Check_RTTI_Restriction:: isa_B(a);
Expand All @@ -180,15 +180,16 @@ int use2 ( a_type ab, a_type *abp ) {

template <typename name, typename Func>
__attribute__((sycl_kernel)) void kernel_single_task(Func kernelFunc) {
// expected-note@+1 {{called by 'kernel_single_task}}
kernelFunc();
a_type ab;
a_type *p;
// expected-note@+1 {{called by 'kernel_single_task'}}
use2(ab, p);
}

int main() {
a_type ab;
// expected-note@+1 {{called by 'operator()'}}
kernel_single_task<class fake_kernel>([]() { usage( &addInt ); });
return 0;
}
Expand Down
2 changes: 1 addition & 1 deletion clang/test/SemaSYCL/sycl-varargs-cconv.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ void bar() {

template <typename name, typename Func>
__attribute__((sycl_kernel)) void kernel_single_task(Func kernelFunc) {
kernelFunc();
kernelFunc(); //expected-note 2+ {{called by 'kernel_single_task}}
}

int main() {
Expand Down
39 changes: 39 additions & 0 deletions clang/test/SemaSYCL/variadic-func-call.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
// RUN: %clang_cc1 -triple spir64-unknown-unknown -fsycl-is-device -fsyntax-only -verify %s

void variadic(int, ...);
namespace NS {
void variadic(int, ...);
}

struct S {
S(int, ...);
void operator()(int, ...);
};

void foo() {
auto x = [](int, ...) {};
x(5, 10); //expected-error{{SYCL kernel cannot call a variadic function}}
}

void overloaded(int, int);
void overloaded(int, ...);
template <typename, typename Func>
__attribute__((sycl_kernel)) void task(Func KF) {
KF(); // expected-note 2 {{called by 'task}}
}

int main() {
task<class FK>([]() {
variadic(5); //expected-error{{SYCL kernel cannot call a variadic function}}
variadic(5, 2); //expected-error{{SYCL kernel cannot call a variadic function}}
NS::variadic(5, 3); //expected-error{{SYCL kernel cannot call a variadic function}}
S s(5, 4); //expected-error{{SYCL kernel cannot call a variadic function}}
S s2(5); //expected-error{{SYCL kernel cannot call a variadic function}}
s(5, 5); //expected-error{{SYCL kernel cannot call a variadic function}}
s2(5); //expected-error{{SYCL kernel cannot call a variadic function}}
foo(); //expected-note{{called by 'operator()'}}
overloaded(5, 6); //expected-no-error
overloaded(5, s); //expected-error{{SYCL kernel cannot call a variadic function}}
overloaded(5); //expected-error{{SYCL kernel cannot call a variadic function}}
});
}