Skip to content

[SYCL][UX] Diagnostic for undefined device functions #1026

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 58 commits into from
Mar 10, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
58 commits
Select commit Hold shift + click to select a range
3ff2deb
[SYCL] [UX] Kernel's call whitelisting.
Dec 17, 2019
699b77e
[SYCL] [UX] Lookup for mangled names in whitelist
Dec 17, 2019
46a90ca
[SYCL] Fix review comments
Dec 18, 2019
768f44f
[SYCL] [UX] Remove explicit whitelisting of methods called by kernel
Jan 15, 2020
30c66dd
[SYCL] [UX] Add SYCL_EXTERNAL to all __spirv methods
Jan 15, 2020
22ac951
[SYCL] [UX] Reword diagnostic message
Jan 15, 2020
a355390
[SYCL] [NFC] Shift __spirv_ocl_printf to spirv_ops.hpp
Jan 16, 2020
d37a0c3
[SYCL] [UX] Only emit diagnostic there will be a call in runtime
Jan 17, 2020
19ec821
[SYCL] [UX] Add SYCL_EXTERNAL attribute to spirv methods.
Jan 17, 2020
96945e4
[SYCL] [UX] Remove unwanted macros
Jan 20, 2020
b6abd6c
[SYCL][UX][NFC] Reword diagnostic message
Jan 21, 2020
d3a0b94
[SYCL] Fix review comments.
Jan 21, 2020
0c2ba82
[SYCL] Remove format attribute from __spirv_ocl_printf
Jan 21, 2020
ec27d5a
[SYCL] Fix review comment
Jan 28, 2020
4ff23c4
[SYCL] Add test for new diagnostic
Jan 28, 2020
444e30e
[SYCL] Fix review comment
Jan 28, 2020
9121b3b
[SYCL] Fix tests
Jan 29, 2020
0f8e11a
[SYCL] Allow for __spirv_ocl_printf to be called without being marked…
Jan 31, 2020
9fc0f2a
[SYCL] Fix test
Jan 31, 2020
8cc61f9
[SYCL] Employ deferred diagnostic
Jan 31, 2020
d5f3c07
[SYCL] Improve test
Feb 3, 2020
e03d71c
[SYCL] Fix employing deferred diagnostic
Feb 3, 2020
d2a57a4
[SYCL] Introduce sycl_has_definition attribute
Feb 11, 2020
a9077dc
[SYCL] Fix review comments
Feb 11, 2020
423aa08
[SYCL] Codestyle fixes. Fix review comment
Jan 21, 2020
c13c7ae
[SYCL] Cache values of isUnevaluatedContext for future use
Jan 28, 2020
e956ef3
[SYCL] Fix review comments
Jan 28, 2020
86241ca
[SYCL] Allow for calls of builtins
Jan 28, 2020
4303df4
[SYCL] Fix tests
Feb 3, 2020
0f4a972
[SYCL] Improve test
Feb 4, 2020
1ed0f9d
[SYCL] Work on diagnostic
Feb 5, 2020
bf9897d
[SYCL] Fix test
Feb 10, 2020
ad87643
[SYCL] Fix review comments
Feb 11, 2020
49b3671
[SYCL] Fix tests
Feb 11, 2020
c0002be
[SYCL] Fix review comments
Feb 11, 2020
7ad4529
[SYCL] Add __SYCL_HAS_DEFINITION__ for math spirv ops
Feb 12, 2020
31d10c8
[SYCL] Update comment
Feb 12, 2020
a5f1e73
[SYCL] Fix test
Feb 12, 2020
ed494da
[SYCL] Fix review comments
Feb 12, 2020
ac9d797
[SYCL] Fix review comments.
Feb 14, 2020
fb214af
[SYCL] Fix review comment
Feb 14, 2020
28afc0d
[SYCL] Use array of string tags/opts instead of a mere number in attr…
Feb 17, 2020
f514a83
Merge remote-tracking branch 'public/sycl' into private/s-kanaev/func…
Feb 27, 2020
ff3692d
[SYCL] Fix tests
Feb 27, 2020
1c4a66f
[SYCL] Reword comment to represent redesigned solution
Feb 27, 2020
eabbf41
[SYCL] Add -Wno-sycl-strict if user didn't provide explicit SYCL version
Feb 27, 2020
48b7653
[SYCL] Temporary change for testing purposes
Feb 27, 2020
11db09c
[SYCL] Remove temporary change. Fix tests.
Feb 28, 2020
3063419
[SYCL] Remove tags/options from sycl_device attribute
Feb 28, 2020
a78909d
[SYCL] Fix some code-style issues
Feb 28, 2020
7a4639a
[SYCL] Fix some code-style issues
Feb 28, 2020
bba50f2
[SYCL] Reorder comment and LIT lines
Feb 28, 2020
a806343
[SYCL] Fix some code-style issues
Feb 28, 2020
cdfee11
[SYCL] Fix some code-style issues
Feb 28, 2020
e72badd
Merge branch 'sycl' into private/s-kanaev/function-whitelist
Mar 2, 2020
283b638
Merge branch 'sycl' into private/s-kanaev/function-whitelist
Mar 10, 2020
68af609
[SYCL] Fix test. Fix review comments.
Mar 10, 2020
98f3ef3
[SYCL] Fix typo
s-kanaev Mar 10, 2020
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
4 changes: 3 additions & 1 deletion clang/include/clang/Basic/DiagnosticSemaKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -10642,7 +10642,9 @@ def err_sycl_restrict : Error<
"|allocate storage"
"|use inline assembly"
"|call a dllimport function"
"|call a variadic function}0">;
"|call a variadic function"
"|call an undefined function without SYCL_EXTERNAL attribute"
"}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
31 changes: 24 additions & 7 deletions clang/include/clang/Sema/Sema.h
Original file line number Diff line number Diff line change
Expand Up @@ -12298,11 +12298,13 @@ class Sema final {
KernelAllocateStorage,
KernelUseAssembly,
KernelCallDllimportFunction,
KernelCallVariadicFunction
};
KernelCallVariadicFunction,
KernelCallUndefinedFunction
};

bool isKnownGoodSYCLDecl(const Decl *D);
void ConstructOpenCLKernel(FunctionDecl *KernelCallerFunc, MangleContext &MC);
void MarkDevice(void);
void MarkDevice();

/// Creates a DeviceDiagBuilder that emits the diagnostic if the current
/// context is "used as device code".
Expand All @@ -12323,10 +12325,25 @@ class Sema final {
/// SYCLDiagIfDeviceCode(Loc, diag::err_thread_unsupported);
DeviceDiagBuilder SYCLDiagIfDeviceCode(SourceLocation Loc, unsigned DiagID);

/// Checks if Callee function is a device function and emits
/// diagnostics if it is known that it is a device function, adds this
/// function to the DeviceCallGraph otherwise.
void checkSYCLDeviceFunction(SourceLocation Loc, FunctionDecl *Callee);
/// Check whether we're allowed to call Callee from the current context.
///
/// - If the call is never allowed in a semantically-correct program
/// emits an error and returns false.
///
/// - If the call is allowed in semantically-correct programs, but only if
/// it's never codegen'ed, creates a deferred diagnostic to be emitted if
/// and when the caller is codegen'ed, and returns true.
///
/// - Otherwise, returns true without emitting any diagnostics.
///
/// Adds Callee to DeviceCallGraph if we don't know if its caller will be
/// codegen'ed yet.
bool checkSYCLDeviceFunction(SourceLocation Loc, FunctionDecl *Callee);

/// Emit diagnostic that can't be emitted with deferred diagnostics mechanism.
/// At this step we imply that all device functions are marked with
/// sycl_device attribute.
void finalizeSYCLDelayedAnalysis();
};

template <typename AttrType>
Expand Down
4 changes: 4 additions & 0 deletions clang/lib/Driver/ToolChains/Clang.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4124,6 +4124,10 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
} else if (IsSYCL) {
// Ensure the default version in SYCL mode is 1.2.1
CmdArgs.push_back("-sycl-std=1.2.1");
// The user had not pass SYCL version, thus we'll employ no-sycl-strict
// to allow address-space unqualified pointers in function params/return
// along with marking the same function with explicit SYCL_EXTERNAL
CmdArgs.push_back("-Wno-sycl-strict");
}

if (IsOpenMPDevice) {
Expand Down
1 change: 1 addition & 0 deletions clang/lib/Sema/Sema.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -974,6 +974,7 @@ void Sema::ActOnEndOfTranslationUnitFragment(TUFragmentKind Kind) {
if (SyclIntHeader != nullptr)
SyclIntHeader->emit(getLangOpts().SYCLIntHeader);
MarkDevice();
finalizeSYCLDelayedAnalysis();
}

// Finalize analysis of OpenMP-specific constructs.
Expand Down
23 changes: 23 additions & 0 deletions clang/lib/Sema/SemaDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18016,6 +18016,12 @@ Decl *Sema::getObjCDeclContext() const {
}

Sema::FunctionEmissionStatus Sema::getEmissionStatus(FunctionDecl *FD) {
// Due to SYCL functions are template we check if they have appropriate
Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't seem to be a full sentence. Word missing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can't see what is missing in "Due to SYCL functions are template we check if they have appropriate attribute prior to checking if it is a template"

// attribute prior to checking if it is a template
if (LangOpts.SYCLIsDevice &&
(FD->hasAttr<SYCLDeviceAttr>() || FD->hasAttr<SYCLKernelAttr>()))
return FunctionEmissionStatus::Emitted;

// Templates are emitted when they're instantiated.
if (FD->isDependentContext())
return FunctionEmissionStatus::TemplateDiscarded;
Expand Down Expand Up @@ -18080,6 +18086,23 @@ Sema::FunctionEmissionStatus Sema::getEmissionStatus(FunctionDecl *FD) {
return FunctionEmissionStatus::Emitted;
}

if (getLangOpts().SYCLIsDevice) {
if (!FD->hasAttr<SYCLDeviceAttr>() && !FD->hasAttr<SYCLKernelAttr>())
return FunctionEmissionStatus::Unknown;

// Check whether this function is externally visible -- if so, it's
// known-emitted.
//
// We have to check the GVA linkage of the function's *definition* -- if we
// only have a declaration, we don't know whether or not the function will
// be emitted, because (say) the definition could include "inline".
FunctionDecl *Def = FD->getDefinition();

if (Def &&
!isDiscardableGVALinkage(getASTContext().GetGVALinkageForFunction(Def)))
return FunctionEmissionStatus::Emitted;
}

// Otherwise, the function is known-emitted if it's in our set of
// known-emitted functions.
return (DeviceKnownEmittedFns.count(FD) > 0)
Expand Down
5 changes: 3 additions & 2 deletions clang/lib/Sema/SemaDeclCXX.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14684,8 +14684,9 @@ Sema::BuildCXXConstructExpr(SourceLocation ConstructLoc, QualType DeclInitType,
MarkFunctionReferenced(ConstructLoc, Constructor);
if (getLangOpts().CUDA && !CheckCUDACall(ConstructLoc, Constructor))
return ExprError();
if (getLangOpts().SYCLIsDevice)
checkSYCLDeviceFunction(ConstructLoc, Constructor);
if (getLangOpts().SYCLIsDevice &&
!checkSYCLDeviceFunction(ConstructLoc, Constructor))
return ExprError();

return CXXConstructExpr::Create(
Context, DeclInitType, ConstructLoc, Constructor, Elidable,
Expand Down
4 changes: 2 additions & 2 deletions clang/lib/Sema/SemaExpr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -301,8 +301,8 @@ bool Sema::DiagnoseUseOfDecl(NamedDecl *D, ArrayRef<SourceLocation> Locs,
if (getLangOpts().CUDA && !CheckCUDACall(Loc, FD))
return true;

if (getLangOpts().SYCLIsDevice)
checkSYCLDeviceFunction(Loc, FD);
if (getLangOpts().SYCLIsDevice && !checkSYCLDeviceFunction(Loc, FD))
return true;
}

if (auto *MD = dyn_cast<CXXMethodDecl>(D)) {
Expand Down
102 changes: 83 additions & 19 deletions clang/lib/Sema/SemaSYCL.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -537,6 +537,7 @@ class MarkDeviceFunction : public RecursiveASTVisitor<MarkDeviceFunction> {
}
return true;
}

Sema &SemaRef;
};

Expand Down Expand Up @@ -1410,18 +1411,6 @@ void Sema::MarkDevice(void) {
// SYCL device specific diagnostics implementation
// -----------------------------------------------------------------------------

// Do we know that we will eventually codegen the given function?
static bool isKnownEmitted(Sema &S, FunctionDecl *FD) {
assert(FD && "Given function may not be null.");

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;
}

Sema::DeviceDiagBuilder Sema::SYCLDiagIfDeviceCode(SourceLocation Loc,
unsigned DiagID) {
assert(getLangOpts().SYCLIsDevice &&
Expand All @@ -1430,29 +1419,104 @@ Sema::DeviceDiagBuilder Sema::SYCLDiagIfDeviceCode(SourceLocation Loc,
DeviceDiagBuilder::Kind DiagKind = [this, FD] {
if (ConstructingOpenCLKernel || !FD)
return DeviceDiagBuilder::K_Nop;
if (isKnownEmitted(*this, FD))
if (getEmissionStatus(FD) == Sema::FunctionEmissionStatus::Emitted)
return DeviceDiagBuilder::K_ImmediateWithCallStack;
return DeviceDiagBuilder::K_Deferred;
}();
return DeviceDiagBuilder(DiagKind, Loc, DiagID, FD, *this);
}

void Sema::checkSYCLDeviceFunction(SourceLocation Loc, FunctionDecl *Callee) {
bool Sema::checkSYCLDeviceFunction(SourceLocation Loc, FunctionDecl *Callee) {
assert(getLangOpts().SYCLIsDevice &&
"Should only be called during SYCL compilation");
assert(Callee && "Callee may not be null.");

// Errors in unevaluated context don't need to be generated,
// so we can safely skip them.
if (isUnevaluatedContext())
return;
if (isUnevaluatedContext() || isConstantEvaluated())
return true;

FunctionDecl *Caller = dyn_cast<FunctionDecl>(getCurLexicalContext());

if (!Caller)
return true;

bool CallerKnownEmitted =
getEmissionStatus(Caller) == FunctionEmissionStatus::Emitted;

// 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 (Caller && isKnownEmitted(*this, Caller))
markKnownEmitted(*this, Caller, Callee, Loc, isKnownEmitted);
else if (Caller)
if (CallerKnownEmitted)
markKnownEmitted(*this, Caller, Callee, Loc, [](Sema &S, FunctionDecl *FD) {
return S.getEmissionStatus(FD) == Sema::FunctionEmissionStatus::Emitted;
});
else
DeviceCallGraph[Caller].insert({Callee, Loc});

DeviceDiagBuilder::Kind DiagKind = DeviceDiagBuilder::K_Nop;

// TODO Set DiagKind to K_Immediate/K_Deferred to emit diagnostics for Callee

DeviceDiagBuilder(DiagKind, Loc, diag::err_sycl_restrict, Caller, *this)
<< Sema::KernelCallUndefinedFunction;
DeviceDiagBuilder(DiagKind, Callee->getLocation(), diag::note_previous_decl,
Caller, *this)
<< Callee;

return DiagKind != DeviceDiagBuilder::K_Immediate &&
DiagKind != DeviceDiagBuilder::K_ImmediateWithCallStack;
}

static void emitCallToUndefinedFnDiag(Sema &SemaRef, const FunctionDecl *Callee,
const FunctionDecl *Caller,
const SourceLocation &Loc) {
// Somehow an unspecialized template appears to be in callgraph or list of
// device functions. We don't want to emit diagnostic here.
if (Callee->getTemplatedKind() == FunctionDecl::TK_FunctionTemplate)
return;

// Don't emit diagnostic for functions not called from device code
if (!Caller->hasAttr<SYCLDeviceAttr>() && !Caller->hasAttr<SYCLKernelAttr>())
return;

bool RedeclHasAttr = false;

for (const Decl *Redecl : Callee->redecls()) {
if (const FunctionDecl *FD = dyn_cast_or_null<FunctionDecl>(Redecl)) {
if ((FD->hasAttr<SYCLDeviceAttr>() &&
!FD->getAttr<SYCLDeviceAttr>()->isImplicit()) ||
FD->hasAttr<SYCLKernelAttr>()) {
RedeclHasAttr = true;
break;
}
}
}

// Disallow functions with neither definition nor SYCL_EXTERNAL mark
bool NotDefinedNoAttr = !Callee->isDefined() && !RedeclHasAttr;

if (NotDefinedNoAttr && !Callee->getBuiltinID()) {
SemaRef.Diag(Loc, diag::err_sycl_restrict)
<< Sema::KernelCallUndefinedFunction;
SemaRef.Diag(Callee->getLocation(), diag::note_previous_decl) << Callee;
SemaRef.Diag(Caller->getLocation(), diag::note_called_by) << Caller;
}
}

void Sema::finalizeSYCLDelayedAnalysis() {
assert(getLangOpts().SYCLIsDevice &&
"Should only be called during SYCL compilation");

llvm::DenseSet<const FunctionDecl *> Checked;

for (const auto &EmittedWithLoc : DeviceKnownEmittedFns) {
const FunctionDecl *Caller = EmittedWithLoc.getSecond().FD;
const SourceLocation &Loc = EmittedWithLoc.getSecond().Loc;
const FunctionDecl *Callee = EmittedWithLoc.getFirst();

if (Checked.insert(Callee).second)
emitCallToUndefinedFnDiag(*this, Callee, Caller, Loc);
}
}

// -----------------------------------------------------------------------------
Expand Down
2 changes: 1 addition & 1 deletion clang/test/CodeGenSYCL/address-space-new.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ struct HasX {

struct Y : SpaceWaster, HasX {};

void bar(HasX &hx);
SYCL_EXTERNAL void bar(HasX &hx);

void baz(Y &y) {
bar(y);
Expand Down
12 changes: 6 additions & 6 deletions clang/test/CodeGenSYCL/bool-vectors.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,17 @@ using bool4 = bool __attribute__((ext_vector_type(4)));
using bool8 = bool __attribute__((ext_vector_type(8)));
using bool16 = bool __attribute__((ext_vector_type(16)));

extern bool1 foo1();
extern SYCL_EXTERNAL bool1 foo1();
// CHECK-DAG: declare spir_func zeroext i1 @[[FOO1:[a-zA-Z0-9_]+]]()
extern bool2 foo2();
extern SYCL_EXTERNAL bool2 foo2();
// CHECK-DAG: declare spir_func <2 x i1> @[[FOO2:[a-zA-Z0-9_]+]]()
extern bool3 foo3();
extern SYCL_EXTERNAL bool3 foo3();
// CHECK-DAG: declare spir_func <3 x i1> @[[FOO3:[a-zA-Z0-9_]+]]()
extern bool4 foo4();
extern SYCL_EXTERNAL bool4 foo4();
// CHECK-DAG: declare spir_func <4 x i1> @[[FOO4:[a-zA-Z0-9_]+]]()
extern bool8 foo8();
extern SYCL_EXTERNAL bool8 foo8();
// CHECK-DAG: declare spir_func <8 x i1> @[[FOO8:[a-zA-Z0-9_]+]]()
extern bool16 foo16();
extern SYCL_EXTERNAL bool16 foo16();
// CHECK-DAG: declare spir_func <16 x i1> @[[FOO16:[a-zA-Z0-9_]+]]()

void bar (bool1 b) {};
Expand Down
6 changes: 3 additions & 3 deletions clang/test/CodeGenSYCL/fpga_pipes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@
// CHECK: %opencl.pipe_ro_t

using WPipeTy = __attribute__((pipe("write_only"))) const int;
WPipeTy WPipeCreator();
SYCL_EXTERNAL WPipeTy WPipeCreator();

using RPipeTy = __attribute__((pipe("read_only"))) const int;
RPipeTy RPipeCreator();
SYCL_EXTERNAL RPipeTy RPipeCreator();

template <typename PipeTy>
void foo(PipeTy Pipe) {}
Expand All @@ -24,7 +24,7 @@ template <int N>
constexpr PipeStorageTy
TempStorage __attribute__((io_pipe_id(N))) = {2};

void boo(PipeStorageTy PipeStorage);
SYCL_EXTERNAL void boo(PipeStorageTy PipeStorage);

template <int ID>
struct ethernet_pipe {
Expand Down
2 changes: 1 addition & 1 deletion clang/test/CodeGenSYCL/unique-stable-name.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
// CHECK: @[[LAMBDA_IN_DEP_INT:[^\w]+]] = private unnamed_addr constant [[DEP_INT_SIZE:\[[0-9]+ x i8\]]] c"_ZTSZ28lambda_in_dependent_functionIiEvvEUlvE23->12\00",
// CHECK: @[[LAMBDA_IN_DEP_X:[^\w]+]] = private unnamed_addr constant [[DEP_LAMBDA_SIZE:\[[0-9]+ x i8\]]] c"_ZTSZ28lambda_in_dependent_functionIZZ4mainENKUlvE42->5clEvEUlvE46->16EvvEUlvE23->12\00",

extern "C" void printf(const char*);
extern "C" void printf(const char *) {}

template <typename T>
void template_param() {
Expand Down
Loading