Skip to content

[SYCL] Allow relaxed function pointer support in frontend #17274

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

Open
wants to merge 13 commits into
base: sycl
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 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
3 changes: 3 additions & 0 deletions clang/include/clang/Basic/DiagnosticSemaKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -12805,6 +12805,9 @@ def err_sycl_taking_address_of_wrong_function : Error<
"taking address of a function not marked with "
"'intel::device_indirectly_callable' attribute is not allowed in SYCL device "
"code">;
def err_sycl_taking_address_of_function_with_no_definition : Error<
"taking address of a function without a definition and not "
"marked with 'intel::device_indirectly_callable'">;
def err_sycl_add_ir_attr_filter_list_invalid_arg : Error<
"only the first argument of attribute %0 can be an initializer list">;
def err_sycl_add_ir_attribute_must_have_pairs : Error<
Expand Down
4 changes: 3 additions & 1 deletion clang/include/clang/Basic/LangOptions.def
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,9 @@ LANGOPT(OffloadViaLLVM, 1, 0, "target LLVM/Offload as portable offloading runtim
LANGOPT(SYCLIsDevice , 1, 0, "Generate code for SYCL device")
LANGOPT(SYCLIsHost , 1, 0, "SYCL host compilation")
LANGOPT(IntelFPGA , 1, 0, "Perform ahead-of-time compilation for FPGA")
LANGOPT(SYCLAllowFuncPtr , 1, 0, "Allow function pointers in SYCL device code")
ENUM_LANGOPT(SYCLAllowFuncPtr, SYCLFuncPtrPreference, 2,
SYCLFuncPtrPreference::Off,
"Allow function pointers in SYCL device code")
LANGOPT(SYCLStdLayoutKernelParams, 1, 0, "Enable standard layout requirement for SYCL kernel parameters")
LANGOPT(SYCLUnnamedLambda , 1, 0, "Allow unnamed lambda SYCL kernels")
LANGOPT(SYCLForceInlineKernelLambda , 1, 0, "Force inline SYCL kernel lambdas in entry point")
Expand Down
9 changes: 9 additions & 0 deletions clang/include/clang/Basic/LangOptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,15 @@ class LangOptionsBase {
Force,
};

enum class SYCLFuncPtrPreference : int {
Off, //< Never allowed.
LabeledOnly, //< Allowed if the function has the
//intel::device_indirectly_callable attribute.
DefinedOnly, //< Allowed if the function has a definition in the TU or has
//the intel::device_indirectly_callable attribute.
On, //< Allowed.
};

enum HLSLLangStd {
HLSL_Unset = 0,
HLSL_2015 = 2015,
Expand Down
24 changes: 19 additions & 5 deletions clang/include/clang/Driver/Options.td
Original file line number Diff line number Diff line change
Expand Up @@ -7264,6 +7264,25 @@ defm sycl_allow_device_image_dependencies: BoolOptionWithoutMarshalling<"f", "sy
def fsycl_dump_device_code_EQ : Joined<["-"], "fsycl-dump-device-code=">,
Flags<[NoXarchOption]>,
HelpText<"Dump device code into the user provided directory.">;

def fsycl_allow_func_ptr_EQ : Joined<["-"], "fsycl-allow-func-ptr=">,
Visibility<[ClangOption, CLOption, CC1Option]>,
Values<"off,labeled,defined">,
NormalizedValuesScope<"LangOptions::SYCLFuncPtrPreference">,
NormalizedValues<["Off", "LabeledOnly", "DefinedOnly"]>,
MarshallingInfoEnum<LangOpts<"SYCLAllowFuncPtr">, "Off">,
HelpText<"Options for allowing function pointers in SYCL device: "
"off (no function pointers allowed) "
"labeled (only with functions with the intel::device_indirectly_callable attribute) "
"defined (only with labeled or defined functions)."
"Default is 'off'">;
def fsycl_allow_func_ptr : Flag<["-"], "fsycl-allow-func-ptr">,
Alias<fsycl_allow_func_ptr_EQ>, AliasArgs<["labeled"]>,
HelpText<"Same as -fsycl-allow-func-ptr=labeled.">;
def fno_sycl_allow_func_ptr : Flag<["-"], "fno-sycl-allow-func-ptr">,
Alias<fsycl_allow_func_ptr_EQ>, AliasArgs<["off"]>,
HelpText<"Same as -fsycl-allow-func-ptr=off instead.">;

} // let Group = sycl_Group

// FIXME: -fsycl-explicit-simd is deprecated. remove it when support is dropped.
Expand Down Expand Up @@ -8824,11 +8843,6 @@ def fsycl_unique_prefix_EQ
def fsycl_std_layout_kernel_params: Flag<["-"], "fsycl-std-layout-kernel-params">,
HelpText<"Enable standard layout requirement for SYCL kernel parameters.">,
MarshallingInfoFlag<LangOpts<"SYCLStdLayoutKernelParams">>;
defm sycl_allow_func_ptr: BoolFOption<"sycl-allow-func-ptr",
LangOpts<"SYCLAllowFuncPtr">, DefaultFalse,
PosFlag<SetTrue, [], [ClangOption], "Allow">,
NegFlag<SetFalse, [], [ClangOption], "Disallow">,
BothFlags<[], [ClangOption, CLOption, DXCOption, CC1Option], " function pointers in SYCL device.">>;
def fenable_sycl_dae : Flag<["-"], "fenable-sycl-dae">,
HelpText<"Enable Dead Argument Elimination in SPIR kernels">,
MarshallingInfoFlag<LangOpts<"EnableDAEInSpirKernels">>;
Expand Down
19 changes: 19 additions & 0 deletions clang/include/clang/Sema/SemaSYCL.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include "clang/Sema/SemaBase.h"
#include "clang/Sema/SemaDiagnostic.h"
#include "llvm/ADT/DenseSet.h"
#include "llvm/ADT/MapVector.h"
#include "llvm/ADT/SetVector.h"

namespace clang {
Expand Down Expand Up @@ -263,6 +264,15 @@ class SemaSYCL : public SemaBase {

llvm::DenseSet<const FunctionDecl *> SYCLKernelFunctions;

// Function which had their address taken and for which we need to check they
// have a body in this TU.
// Maps the function to check against the context and location their address
// is taken.
llvm::MapVector<
const FunctionDecl *,
llvm::SmallVector<std::pair<const FunctionDecl *, SourceLocation>>>
FunctionAddressTakenToVerify;

public:
SemaSYCL(Sema &S);

Expand Down Expand Up @@ -292,6 +302,15 @@ class SemaSYCL : public SemaBase {
SourceLocation Loc, unsigned DiagID,
DeviceDiagnosticReason Reason = DeviceDiagnosticReason::Sycl |
DeviceDiagnosticReason::Esimd);
SemaDiagnosticBuilder DiagIfDeviceCode(
SourceLocation Loc, unsigned DiagID,
const FunctionDecl *FD,
DeviceDiagnosticReason Reason = DeviceDiagnosticReason::Sycl |
DeviceDiagnosticReason::Esimd);

void delayFunctionBodyCheckForAddressTaken(const FunctionDecl *FD, SourceLocation Loc);

void checkFunctionWithAddressTaken();

void deepTypeCheckForDevice(SourceLocation UsedAt,
llvm::DenseSet<QualType> Visited,
Expand Down
6 changes: 2 additions & 4 deletions clang/lib/Driver/ToolChains/Clang.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5744,10 +5744,8 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
}
}

if (Args.hasFlag(options::OPT_fsycl_allow_func_ptr,
options::OPT_fno_sycl_allow_func_ptr, false)) {
CmdArgs.push_back("-fsycl-allow-func-ptr");
}
if (Arg *A = Args.getLastArg(options::OPT_fsycl_allow_func_ptr_EQ))
A->render(Args, CmdArgs);

Args.AddLastArg(CmdArgs, options::OPT_fsycl_decompose_functor,
options::OPT_fno_sycl_decompose_functor);
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 @@ -1192,6 +1192,7 @@ void Sema::ActOnEndOfTranslationUnitFragment(TUFragmentKind Kind) {
if (SYCL().hasSyclIntegrationHeader())
SYCL().getSyclIntegrationHeader().emit(getLangOpts().SYCLIntHeader);
SYCL().MarkDevices();
SYCL().checkFunctionWithAddressTaken();
}

emitDeferredDiags();
Expand Down
14 changes: 12 additions & 2 deletions clang/lib/Sema/SemaOverload.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11060,11 +11060,21 @@ static bool checkAddressOfFunctionIsAvailable(Sema &S, const FunctionDecl *FD,
bool InOverloadResolution,
SourceLocation Loc) {
if (Complain && S.getLangOpts().SYCLIsDevice &&
S.getLangOpts().SYCLAllowFuncPtr) {
if (!FD->hasAttr<SYCLDeviceIndirectlyCallableAttr>()) {
S.getLangOpts().getSYCLAllowFuncPtr() !=
LangOptions::SYCLFuncPtrPreference::Off) {
bool IsMarked = FD->hasAttr<SYCLDeviceIndirectlyCallableAttr>();
if (S.getLangOpts().getSYCLAllowFuncPtr() ==
LangOptions::SYCLFuncPtrPreference::LabeledOnly &&
!IsMarked) {
S.SYCL().DiagIfDeviceCode(Loc,
diag::err_sycl_taking_address_of_wrong_function,
Sema::DeviceDiagnosticReason::Sycl);
} else if (S.getLangOpts().getSYCLAllowFuncPtr() ==
LangOptions::SYCLFuncPtrPreference::DefinedOnly &&
!FD->hasBody() && !IsMarked) {
// The function has no body, but might be defined later.
// Delay the diagnostic until the end of the translation unit.
S.SYCL().delayFunctionBodyCheckForAddressTaken(FD, Loc);
}
}

Expand Down
34 changes: 32 additions & 2 deletions clang/lib/Sema/SemaSYCL.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -682,7 +682,7 @@ class DiagDeviceFunction : public RecursiveASTVisitor<DiagDeviceFunction> {
SemaSYCLRef.Diag(e->getExprLoc(), diag::err_builtin_target_unsupported)
<< Name << "SYCL device";
}
} else if (!SemaSYCLRef.getLangOpts().SYCLAllowFuncPtr &&
} else if (SemaSYCLRef.getLangOpts().getSYCLAllowFuncPtr() == LangOptions::SYCLFuncPtrPreference::Off &&
!e->isTypeDependent() &&
!isa<CXXPseudoDestructorExpr>(e->getCallee())) {
bool MaybeConstantExpr = false;
Expand Down Expand Up @@ -5792,10 +5792,10 @@ void SemaSYCL::ProcessFreeFunction(FunctionDecl *FD) {

Sema::SemaDiagnosticBuilder
SemaSYCL::DiagIfDeviceCode(SourceLocation Loc, unsigned DiagID,
const FunctionDecl *FD,
DeviceDiagnosticReason Reason) {
assert(getLangOpts().SYCLIsDevice &&
"Should only be called during SYCL compilation");
FunctionDecl *FD = dyn_cast<FunctionDecl>(SemaRef.getCurLexicalContext());
SemaDiagnosticBuilder::Kind DiagKind = [this, FD, Reason] {
if (DiagnosingSYCLKernel)
return SemaDiagnosticBuilder::K_ImmediateWithCallStack;
Expand All @@ -5822,6 +5822,36 @@ SemaSYCL::DiagIfDeviceCode(SourceLocation Loc, unsigned DiagID,
return SemaDiagnosticBuilder(DiagKind, Loc, DiagID, FD, SemaRef, Reason);
}

Sema::SemaDiagnosticBuilder
SemaSYCL::DiagIfDeviceCode(SourceLocation Loc, unsigned DiagID,
DeviceDiagnosticReason Reason) {
assert(getLangOpts().SYCLIsDevice &&
"Should only be called during SYCL compilation");
FunctionDecl *FD = dyn_cast<FunctionDecl>(SemaRef.getCurLexicalContext());
return DiagIfDeviceCode(Loc, DiagID, FD, Reason);
}

void SemaSYCL::delayFunctionBodyCheckForAddressTaken(const FunctionDecl *FD,
SourceLocation Loc) {
if (FunctionDecl *FDCtx =
dyn_cast<FunctionDecl>(SemaRef.getCurLexicalContext()))
FunctionAddressTakenToVerify[FD].emplace_back(FDCtx, Loc);
}

void SemaSYCL::checkFunctionWithAddressTaken() {
for (auto &FuncUsePair : FunctionAddressTakenToVerify) {
const FunctionDecl *FD = FuncUsePair.first;
if (!FD->hasBody())
for (auto &CtxLocPair : FuncUsePair.second) {
const FunctionDecl *LexCtx = CtxLocPair.first;
SourceLocation Loc = CtxLocPair.second;
SemaRef.SYCL().DiagIfDeviceCode(
Loc, diag::err_sycl_taking_address_of_function_with_no_definition, LexCtx,
Sema::DeviceDiagnosticReason::Sycl);
}
}
}

void SemaSYCL::deepTypeCheckForDevice(SourceLocation UsedAt,
llvm::DenseSet<QualType> Visited,
ValueDecl *DeclToCheck) {
Expand Down
12 changes: 11 additions & 1 deletion clang/test/SemaSYCL/wrong-address-taking.cpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
// RUN: %clang_cc1 -fsycl-is-device -fsycl-allow-func-ptr -internal-isystem %S/Inputs -fsyntax-only -verify -sycl-std=2020 -std=c++17 %s
// RUN: %clang_cc1 -fsycl-is-device -fsycl-allow-func-ptr=defined -internal-isystem %S/Inputs -fsyntax-only -verify=defined-func -sycl-std=2020 -std=c++17 %s

#include "sycl.hpp"

Expand Down Expand Up @@ -50,6 +51,10 @@ template <typename T> void templatedContext() {
// expected-error@+1 {{taking address of a function not marked with 'intel::device_indirectly_callable' attribute is not allowed in SYCL device code}}
auto p1 = &ForMembers::badMember;

// expected-error@+2 {{taking address of a function not marked with 'intel::device_indirectly_callable' attribute is not allowed in SYCL device code}}
// defined-func-error@+1 {{taking address of a function without a definition and not marked with 'intel::device_indirectly_callable'}}
auto p2 = &externalBadFoo;

// expected-note@+1 {{called by 'templatedContext<int>'}}
templateCaller1<badFoo>(1);
}
Expand All @@ -58,6 +63,7 @@ int main() {

myQueue.submit([&](sycl::handler &h) {
// expected-note@#KernelSingleTaskKernelFuncCall 2{{called by 'kernel_single_task<Basic}}
// defined-func-note@#KernelSingleTaskKernelFuncCall {{called by 'kernel_single_task<Basic}}
h.single_task<class Basic>(
[=]() {
// expected-error@+1 {{taking address of a function not marked with 'intel::device_indirectly_callable' attribute is not allowed in SYCL device code}}
Expand All @@ -69,9 +75,13 @@ int main() {
int (*p3)(int) = &goodFoo;
int (*p4)(int) = goodFoo;

// expected-error@+1 {{taking address of a function not marked with 'intel::device_indirectly_callable' attribute is not allowed in SYCL device code}}
// expected-error@+2 {{taking address of a function not marked with 'intel::device_indirectly_callable' attribute is not allowed in SYCL device code}}
// defined-func-error@+1 {{taking address of a function without a definition and not marked with 'intel::device_indirectly_callable'}}
auto p5 = &externalBadFoo;
auto *p6 = &externalGoodFoo;
// expected-error@+2 {{taking address of a function not marked with 'intel::device_indirectly_callable' attribute is not allowed in SYCL device code}}
// defined-func-error@+1 {{taking address of a function without a definition and not marked with 'intel::device_indirectly_callable'}}
auto p7 = &externalBadFoo;

// Make sure that assignment is diagnosed correctly;
int (*a)(int);
Expand Down
Loading