Skip to content

[SYCL] defer kernel diagnostics #181

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 1 commit into from
Jul 12, 2019
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
5 changes: 3 additions & 2 deletions clang/include/clang/Basic/DiagnosticSemaKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -9721,11 +9721,12 @@ def err_sycl_restrict : Error<
"|use rtti"
"|use a non-const static data variable"
"|call a virtual function"
"|use exceptions"
"|call a recursive function"
"|call through a function pointer"
"|allocate storage"
"|use exceptions"
"|use inline assembly}0">;
"|use inline assembly"
"|have a class with a virtual function table}0">;
def err_sycl_virtual_types : Error<
"No class with a vtable can be used in a SYCL kernel or any code included in the kernel">;
def note_sycl_used_here : Note<"used here">;
Expand Down
14 changes: 14 additions & 0 deletions clang/include/clang/Sema/Sema.h
Original file line number Diff line number Diff line change
Expand Up @@ -11275,8 +11275,22 @@ class Sema {
return *SyclIntHeader.get();
}

enum SYCLRestrictKind {
KernelGlobalVariable,
KernelRTTI,
KernelNonConstStaticDataVariable,
KernelCallVirtualFunction,
KernelUseExceptions,
KernelCallRecursiveFunction,
KernelCallFunctionPointer,
KernelAllocateStorage,
KernelUseAssembly,
KernelHavePolymorphicClass
};
DeviceDiagBuilder SYCLDiagIfDeviceCode(SourceLocation Loc, unsigned DiagID);
void ConstructOpenCLKernel(FunctionDecl *KernelCallerFunc);
void MarkDevice(void);
bool CheckSYCLCall(SourceLocation Loc, FunctionDecl *Callee);
};

/// RAII object that enters a new expression evaluation context.
Expand Down
2 changes: 2 additions & 0 deletions clang/lib/Sema/SemaDeclCXX.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13031,6 +13031,8 @@ Sema::BuildCXXConstructExpr(SourceLocation ConstructLoc, QualType DeclInitType,
MarkFunctionReferenced(ConstructLoc, Constructor);
if (getLangOpts().CUDA && !CheckCUDACall(ConstructLoc, Constructor))
return ExprError();
if (getLangOpts().SYCLIsDevice)
CheckSYCLCall(ConstructLoc, Constructor);

return CXXConstructExpr::Create(
Context, DeclInitType, ConstructLoc, Constructor, Elidable,
Expand Down
15 changes: 14 additions & 1 deletion clang/lib/Sema/SemaExpr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,9 @@ bool Sema::DiagnoseUseOfDecl(NamedDecl *D, ArrayRef<SourceLocation> Locs,

if (getLangOpts().CUDA && !CheckCUDACall(Loc, FD))
return true;

if (getLangOpts().SYCLIsDevice)
CheckSYCLCall(Loc, FD);
}

if (auto *MD = dyn_cast<CXXMethodDecl>(D)) {
Expand Down Expand Up @@ -14903,6 +14906,8 @@ void Sema::MarkFunctionReferenced(SourceLocation Loc, FunctionDecl *Func,

if (getLangOpts().CUDA)
CheckCUDACall(Loc, Func);
if (getLangOpts().SYCLIsDevice)
CheckSYCLCall(Loc, Func);

// If we don't need to mark the function as used, and we don't need to
// try to provide a definition, there's nothing more to do.
Expand Down Expand Up @@ -16124,7 +16129,15 @@ namespace {
}

void VisitCXXNewExpr(CXXNewExpr *E) {
if (E->getOperatorNew())
FunctionDecl *FD = E->getOperatorNew();
if (FD && S.getLangOpts().SYCLIsDevice) {
if (FD->isReplaceableGlobalAllocationFunction())
S.SYCLDiagIfDeviceCode(E->getExprLoc(), diag::err_sycl_restrict)
<< S.KernelAllocateStorage;
else if (FunctionDecl *Def = FD->getDefinition())
S.CheckSYCLCall(E->getExprLoc(), Def);
}
if (FD)
S.MarkFunctionReferenced(E->getBeginLoc(), E->getOperatorNew());
if (E->getOperatorDelete())
S.MarkFunctionReferenced(E->getBeginLoc(), E->getOperatorDelete());
Expand Down
10 changes: 10 additions & 0 deletions clang/lib/Sema/SemaExprCXX.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -760,6 +760,11 @@ ExprResult Sema::BuildCXXThrow(SourceLocation OpLoc, Expr *Ex,
CUDADiagIfDeviceCode(OpLoc, diag::err_cuda_device_exceptions)
<< "throw" << CurrentCUDATarget();

// Exceptions aren't allowed in SYCL device code.
if (getLangOpts().SYCLIsDevice)
SYCLDiagIfDeviceCode(OpLoc, diag::err_sycl_restrict)
<< Sema::KernelUseExceptions;

if (getCurScope() && getCurScope()->isOpenMPSimdDirectiveScope())
Diag(OpLoc, diag::err_omp_simd_region_cannot_use_stmt) << "throw";

Expand Down Expand Up @@ -2193,11 +2198,16 @@ Sema::BuildCXXNew(SourceRange Range, bool UseGlobal,
if (DiagnoseUseOfDecl(OperatorNew, StartLoc))
return ExprError();
MarkFunctionReferenced(StartLoc, OperatorNew);
if (getLangOpts().SYCLIsDevice) {
CheckSYCLCall(StartLoc, OperatorNew);
}
}
if (OperatorDelete) {
if (DiagnoseUseOfDecl(OperatorDelete, StartLoc))
return ExprError();
MarkFunctionReferenced(StartLoc, OperatorDelete);
if (getLangOpts().SYCLIsDevice)
CheckSYCLCall(StartLoc, OperatorDelete);
}

return CXXNewExpr::Create(Context, UseGlobal, OperatorNew, OperatorDelete,
Expand Down
6 changes: 6 additions & 0 deletions clang/lib/Sema/SemaOverload.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12396,6 +12396,8 @@ Sema::CreateOverloadedUnaryOp(SourceLocation OpLoc, UnaryOperatorKind Opc,
FnDecl->getType()->castAs<FunctionProtoType>()))
return ExprError();

if (getLangOpts().SYCLIsDevice)
CheckSYCLCall(OpLoc, FnDecl);
return MaybeBindToTemporary(TheCall);
} else {
// We matched a built-in operator. Convert the arguments, then
Expand Down Expand Up @@ -12639,6 +12641,8 @@ Sema::CreateOverloadedBinOp(SourceLocation OpLoc,
isa<CXXMethodDecl>(FnDecl), OpLoc, TheCall->getSourceRange(),
VariadicDoesNotApply);

if (getLangOpts().SYCLIsDevice)
CheckSYCLCall(OpLoc, FnDecl);
return MaybeBindToTemporary(TheCall);
} else {
// We matched a built-in operator. Convert the arguments, then
Expand Down Expand Up @@ -12852,6 +12856,8 @@ Sema::CreateOverloadedArraySubscriptExpr(SourceLocation LLoc,
Method->getType()->castAs<FunctionProtoType>()))
return ExprError();

if (getLangOpts().SYCLIsDevice)
CheckSYCLCall(RLoc, FnDecl);
return MaybeBindToTemporary(TheCall);
} else {
// We matched a built-in operator. Convert the arguments, then
Expand Down
130 changes: 76 additions & 54 deletions clang/lib/Sema/SemaSYCL.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,18 +38,6 @@ enum target {
image_array
};

enum RestrictKind {
KernelGlobalVariable,
KernelRTTI,
KernelNonConstStaticDataVariable,
KernelCallVirtualFunction,
KernelCallRecursiveFunction,
KernelCallFunctionPointer,
KernelAllocateStorage,
KernelUseExceptions,
KernelUseAssembly
};

using ParamDesc = std::tuple<QualType, IdentifierInfo *, TypeSourceInfo *>;

/// Various utilities.
Expand Down Expand Up @@ -95,16 +83,16 @@ class MarkDeviceFunction : public RecursiveASTVisitor<MarkDeviceFunction> {
// definitions.
if (RecursiveSet.count(Callee)) {
SemaRef.Diag(e->getExprLoc(), diag::err_sycl_restrict)
<< KernelCallRecursiveFunction;
<< Sema::KernelCallRecursiveFunction;
SemaRef.Diag(Callee->getSourceRange().getBegin(),
diag::note_sycl_recursive_function_declared_here)
<< KernelCallRecursiveFunction;
<< Sema::KernelCallRecursiveFunction;
}

if (const CXXMethodDecl *Method = dyn_cast<CXXMethodDecl>(Callee))
if (Method->isVirtual())
SemaRef.Diag(e->getExprLoc(), diag::err_sycl_restrict)
<< KernelCallVirtualFunction;
<< Sema::KernelCallVirtualFunction;

CheckSYCLType(Callee->getReturnType(), Callee->getSourceRange());

Expand All @@ -116,7 +104,7 @@ class MarkDeviceFunction : public RecursiveASTVisitor<MarkDeviceFunction> {
}
} else if (!SemaRef.getLangOpts().SYCLAllowFuncPtr)
SemaRef.Diag(e->getExprLoc(), diag::err_sycl_restrict)
<< KernelCallFunctionPointer;
<< Sema::KernelCallFunctionPointer;
return true;
}

Expand Down Expand Up @@ -144,12 +132,12 @@ class MarkDeviceFunction : public RecursiveASTVisitor<MarkDeviceFunction> {
}

bool VisitCXXTypeidExpr(CXXTypeidExpr *E) {
SemaRef.Diag(E->getExprLoc(), diag::err_sycl_restrict) << KernelRTTI;
SemaRef.Diag(E->getExprLoc(), diag::err_sycl_restrict) << Sema::KernelRTTI;
return true;
}

bool VisitCXXDynamicCastExpr(const CXXDynamicCastExpr *E) {
SemaRef.Diag(E->getExprLoc(), diag::err_sycl_restrict) << KernelRTTI;
SemaRef.Diag(E->getExprLoc(), diag::err_sycl_restrict) << Sema::KernelRTTI;
return true;
}

Expand Down Expand Up @@ -178,7 +166,7 @@ class MarkDeviceFunction : public RecursiveASTVisitor<MarkDeviceFunction> {
bool IsConst = VD->getType().getNonReferenceType().isConstQualified();
if (!IsConst && VD->isStaticDataMember())
SemaRef.Diag(E->getExprLoc(), diag::err_sycl_restrict)
<< KernelNonConstStaticDataVariable;
<< Sema::KernelNonConstStaticDataVariable;
}
return true;
}
Expand All @@ -189,11 +177,11 @@ class MarkDeviceFunction : public RecursiveASTVisitor<MarkDeviceFunction> {
bool IsConst = VD->getType().getNonReferenceType().isConstQualified();
if (!IsConst && VD->isStaticDataMember())
SemaRef.Diag(E->getExprLoc(), diag::err_sycl_restrict)
<< KernelNonConstStaticDataVariable;
<< Sema::KernelNonConstStaticDataVariable;
else if (!IsConst && VD->hasGlobalStorage() && !VD->isStaticLocal() &&
!VD->isStaticDataMember() && !isa<ParmVarDecl>(VD))
SemaRef.Diag(E->getLocation(), diag::err_sycl_restrict)
<< KernelGlobalVariable;
<< Sema::KernelGlobalVariable;
if (!VD->isLocalVarDeclOrParm() && VD->hasGlobalStorage()) {
VD->addAttr(SYCLDeviceAttr::CreateImplicit(SemaRef.Context));
SemaRef.addSyclDeviceDecl(VD);
Expand All @@ -213,7 +201,7 @@ class MarkDeviceFunction : public RecursiveASTVisitor<MarkDeviceFunction> {
if (FunctionDecl *FD = E->getOperatorNew()) {
if (FD->isReplaceableGlobalAllocationFunction()) {
SemaRef.Diag(E->getExprLoc(), diag::err_sycl_restrict)
<< KernelAllocateStorage;
<< Sema::KernelAllocateStorage;
} else if (FunctionDecl *Def = FD->getDefinition()) {
if (!Def->hasAttr<SYCLDeviceAttr>()) {
Def->addAttr(SYCLDeviceAttr::CreateImplicit(SemaRef.Context));
Expand All @@ -223,40 +211,16 @@ class MarkDeviceFunction : public RecursiveASTVisitor<MarkDeviceFunction> {
}
return true;
}

bool VisitCXXThrowExpr(CXXThrowExpr *E) {
SemaRef.Diag(E->getExprLoc(), diag::err_sycl_restrict)
<< KernelUseExceptions;
return true;
}

bool VisitCXXCatchStmt(CXXCatchStmt *S) {
SemaRef.Diag(S->getBeginLoc(), diag::err_sycl_restrict)
<< KernelUseExceptions;
return true;
}

bool VisitCXXTryStmt(CXXTryStmt *S) {
SemaRef.Diag(S->getBeginLoc(), diag::err_sycl_restrict)
<< KernelUseExceptions;
return true;
}

bool VisitSEHTryStmt(SEHTryStmt *S) {
SemaRef.Diag(S->getBeginLoc(), diag::err_sycl_restrict)
<< KernelUseExceptions;
return true;
}


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

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

Expand Down Expand Up @@ -361,21 +325,31 @@ class MarkDeviceFunction : public RecursiveASTVisitor<MarkDeviceFunction> {
return true;

if (CRD->isPolymorphic()) {
SemaRef.Diag(CRD->getLocation(), diag::err_sycl_virtual_types);
SemaRef.Diag(Loc.getBegin(), diag::note_sycl_used_here);
// Exceptions aren't allowed in SYCL device code.
if (SemaRef.getLangOpts().SYCLIsDevice) {
SemaRef.SYCLDiagIfDeviceCode(CRD->getLocation(),
diag::err_sycl_restrict)
<< Sema::KernelHavePolymorphicClass;
SemaRef.SYCLDiagIfDeviceCode(Loc.getBegin(),
diag::note_sycl_used_here);
}
return false;
}

for (const auto &Field : CRD->fields()) {
if (!CheckSYCLType(Field->getType(), Field->getSourceRange(), Visited)) {
SemaRef.Diag(Loc.getBegin(), diag::note_sycl_used_here);
if (SemaRef.getLangOpts().SYCLIsDevice)
SemaRef.SYCLDiagIfDeviceCode(Loc.getBegin(),
diag::note_sycl_used_here);
return false;
}
}
} else if (const auto *RD = Ty->getAsRecordDecl()) {
for (const auto &Field : RD->fields()) {
if (!CheckSYCLType(Field->getType(), Field->getSourceRange(), Visited)) {
SemaRef.Diag(Loc.getBegin(), diag::note_sycl_used_here);
if (SemaRef.getLangOpts().SYCLIsDevice)
SemaRef.SYCLDiagIfDeviceCode(Loc.getBegin(),
diag::note_sycl_used_here);
return false;
}
}
Expand Down Expand Up @@ -1036,6 +1010,54 @@ void Sema::MarkDevice(void) {
}
}
}
//
// Do we know that we will eventually codegen the given function?
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;

// Otherwise, the function is known-emitted if it's in our set of
// known-emitted functions.
return S.DeviceKnownEmittedFns.count(FD) > 0;
}

Sema::DeviceDiagBuilder Sema::SYCLDiagIfDeviceCode(SourceLocation Loc,
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi Premanand, I need something like this function to avoid emitting an error for a function that has varargs and which is not part of DEVICE code.
In my case I have "include <stdio.h>" in the beginning of the source file and no uses of any functions like printf(f, ...) in DEVICE code, but clang still reports an error on Windows for such code.

Q1: Would your fix help with this error: "variadic function cannot use spir_function calling convention" emitted from SemaType.cpp:7014 ?
If NO, can this patch be extended to handle that case?

Q2: This MergeRequest was created almost 2 weeks ago. Is there some ETA for this fix approval/merge?

unsigned DiagID) {
assert(getLangOpts().SYCLIsDevice &&
"Should only be called during SYCL compilation");
DeviceDiagBuilder::Kind DiagKind = [this] {
if (isKnownEmitted(*this, dyn_cast<FunctionDecl>(CurContext)))
return DeviceDiagBuilder::K_ImmediateWithCallStack;
return DeviceDiagBuilder::K_Deferred;
}();
return DeviceDiagBuilder(DiagKind, Loc, DiagID,
dyn_cast<FunctionDecl>(CurContext), *this);
}

bool Sema::CheckSYCLCall(SourceLocation Loc, FunctionDecl *Callee) {

assert(Callee && "Callee may not be null.");
FunctionDecl *Caller = getCurFunctionDecl();

// 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.
if (//!isOpenMPDeviceDelayedContext(*this) ||
(Caller && Caller->hasAttr<SYCLKernelAttr>()) ||
(Caller && Caller->hasAttr<SYCLDeviceAttr>()) ||
(Caller && isKnownEmitted(*this, Caller)))
markKnownEmitted(*this, Caller, Callee, Loc, isKnownEmitted);
else if (Caller)
DeviceCallGraph[Caller].insert({Callee, Loc});
return true;
}

// -----------------------------------------------------------------------------
// Integration header functionality implementation
Expand Down
10 changes: 10 additions & 0 deletions clang/lib/Sema/SemaStmt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4017,6 +4017,11 @@ StmtResult Sema::ActOnCXXTryBlock(SourceLocation TryLoc, Stmt *TryBlock,
CUDADiagIfDeviceCode(TryLoc, diag::err_cuda_device_exceptions)
<< "try" << CurrentCUDATarget();

// Exceptions aren't allowed in SYCL device code.
if (getLangOpts().SYCLIsDevice)
SYCLDiagIfDeviceCode(TryLoc, diag::err_sycl_restrict)
<< KernelUseExceptions;

if (getCurScope() && getCurScope()->isOpenMPSimdDirectiveScope())
Diag(TryLoc, diag::err_omp_simd_region_cannot_use_stmt) << "try";

Expand Down Expand Up @@ -4114,6 +4119,11 @@ StmtResult Sema::ActOnSEHTryBlock(bool IsCXXTry, SourceLocation TryLoc,
}
}

// Exceptions aren't allowed in SYCL device code.
if (getLangOpts().SYCLIsDevice)
SYCLDiagIfDeviceCode(TryLoc, diag::err_sycl_restrict)
<< KernelUseExceptions;

FSI->setHasSEHTry(TryLoc);

// Reject __try in Obj-C methods, blocks, and captured decls, since we don't
Expand Down
Loading