Skip to content

Commit 3f43d47

Browse files
fineg74v-klochkov
andauthored
[SYCL][ESIMD] Report an error when slm_init is called more than once in the kernel (#12804)
The patch reports an error if: - sim_init() is used together with local_accessor - slm_init() is called not from ESIMD kernel - slm_init() is called more than once in ESIMD kernel Co-authored-by: Vyacheslav Klochkov <[email protected]>
1 parent 6e9a3dd commit 3f43d47

File tree

8 files changed

+247
-3
lines changed

8 files changed

+247
-3
lines changed

llvm/lib/SYCLLowerIR/ESIMD/LowerESIMD.cpp

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1745,6 +1745,83 @@ void lowerGlobalsToVector(Module &M) {
17451745

17461746
} // namespace
17471747

1748+
static void checkSLMInit(Module &M) {
1749+
SmallPtrSet<const Function *, 8u> SLMInitKernels;
1750+
SmallPtrSet<const Function *, 8u> LocalAccessorKernels;
1751+
1752+
for (auto &F : M) {
1753+
if (!isSlmInit(F)) {
1754+
if (!llvm::esimd::isESIMDKernel(F))
1755+
continue;
1756+
unsigned Idx = 0;
1757+
for (const Argument &Arg : F.args()) {
1758+
if (Arg.getType()->isPointerTy()) {
1759+
auto *KernelArgAccPtrs = F.getMetadata("kernel_arg_accessor_ptr");
1760+
1761+
if (KernelArgAccPtrs) {
1762+
auto *AccMD =
1763+
cast<ConstantAsMetadata>(KernelArgAccPtrs->getOperand(Idx));
1764+
auto AccMDVal = cast<ConstantInt>(AccMD->getValue())->getValue();
1765+
bool IsAcc = static_cast<unsigned>(AccMDVal.getZExtValue());
1766+
1767+
constexpr unsigned LocalAS{3};
1768+
if (IsAcc && cast<PointerType>(Arg.getType())->getAddressSpace() ==
1769+
LocalAS) {
1770+
LocalAccessorKernels.insert(&F);
1771+
break;
1772+
}
1773+
}
1774+
}
1775+
Idx++;
1776+
}
1777+
} else {
1778+
for (User *U : F.users()) {
1779+
auto *FCall = dyn_cast<CallInst>(U);
1780+
if (FCall && FCall->getCalledFunction() == &F) {
1781+
Function *GenF = FCall->getFunction();
1782+
SmallPtrSet<Function *, 32> Visited;
1783+
sycl::utils::traverseCallgraphUp(
1784+
GenF,
1785+
[&](Function *GraphNode) {
1786+
if (llvm::esimd::isESIMDKernel(*GraphNode)) {
1787+
if (SLMInitKernels.contains(GraphNode)) {
1788+
StringRef KernelName = GraphNode->getName();
1789+
std::string ErrorMsg =
1790+
std::string("slm_init is called more than once "
1791+
"from kernel '") +
1792+
demangle(KernelName.str()) + "'.";
1793+
GraphNode->getContext().emitError(ErrorMsg);
1794+
} else {
1795+
SLMInitKernels.insert(GraphNode);
1796+
}
1797+
}
1798+
},
1799+
Visited, false);
1800+
bool VisitedKernel = false;
1801+
for (const Function *Caller : Visited) {
1802+
if (llvm::esimd::isESIMDKernel(*Caller)) {
1803+
VisitedKernel = true;
1804+
break;
1805+
}
1806+
}
1807+
if (!VisitedKernel) {
1808+
F.getContext().emitError(
1809+
"slm_init must be called directly from ESIMD kernel.");
1810+
}
1811+
} else {
1812+
F.getContext().emitError(
1813+
"slm_init can only be used as a direct call.");
1814+
}
1815+
}
1816+
}
1817+
for (const Function *Kernel : LocalAccessorKernels) {
1818+
if (SLMInitKernels.contains(Kernel))
1819+
F.getContext().emitError(
1820+
"slm_init can not be used with local accessors.");
1821+
}
1822+
}
1823+
}
1824+
17481825
bool SYCLLowerESIMDPass::prepareForAlwaysInliner(Module &M) {
17491826

17501827
auto markAlwaysInlined = [](Function &F) -> bool {
@@ -1912,6 +1989,10 @@ static void fixFunctionReadWriteAttributes(Module &M) {
19121989

19131990
PreservedAnalyses SYCLLowerESIMDPass::run(Module &M,
19141991
ModuleAnalysisManager &MAM) {
1992+
1993+
// Check validity of slm_init calls.
1994+
checkSLMInit(M);
1995+
19151996
// AlwaysInlinerPass is required for correctness.
19161997
bool ForceInline = prepareForAlwaysInliner(M);
19171998
if (ForceInline) {

sycl/test-e2e/ESIMD/unified_memory_api/Inputs/block_store.hpp

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -344,9 +344,6 @@ bool testLocalAccSLM(queue Q, uint32_t Groups,
344344
auto OutPtr = Out.data();
345345

346346
CGH.parallel_for(Range, [=](sycl::nd_item<1> ndi) SYCL_ESIMD_KERNEL {
347-
constexpr uint32_t SLMSize = (GroupSize * N) * sizeof(T);
348-
slm_init<SLMSize>();
349-
350347
uint16_t GlobalID = ndi.get_global_id(0);
351348
uint16_t LocalID = ndi.get_local_id(0);
352349
uint32_t LocalElemOffset = LocalID * N * sizeof(T);

sycl/test/esimd/slm_init_check.cpp

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
// RUN: not %clangxx -fsycl %s 2>&1 | FileCheck %s
2+
3+
// This test verifies more than 1 call to slm_init triggers an error.
4+
5+
#include <iostream>
6+
#include <sycl/ext/intel/esimd.hpp>
7+
#include <sycl/sycl.hpp>
8+
9+
using namespace sycl;
10+
using namespace sycl::ext::intel::esimd;
11+
12+
int main() {
13+
queue Q;
14+
nd_range<1> NDR{range<1>{2}, range<1>{2}};
15+
Q.parallel_for(NDR, [=](nd_item<1> NDI) SYCL_ESIMD_KERNEL {
16+
slm_init(1024);
17+
slm_init(1024);
18+
}).wait();
19+
// CHECK: error: slm_init is called more than once from kernel 'typeinfo name for main::'lambda'(sycl::_V1::nd_item<1>)'.
20+
21+
return 0;
22+
}
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
// This test verifies call to slm_init from a function called through
2+
// invoke_simd triggers an error.
3+
4+
// RUN: not %clangxx -fsycl -fno-sycl-device-code-split-esimd -Xclang -fsycl-allow-func-ptr %s 2>&1 | FileCheck %s
5+
6+
#include <sycl/ext/intel/esimd.hpp>
7+
#include <sycl/ext/oneapi/experimental/invoke_simd.hpp>
8+
#include <sycl/ext/oneapi/experimental/uniform.hpp>
9+
#include <sycl/sycl.hpp>
10+
11+
#include <functional>
12+
#include <iostream>
13+
#include <type_traits>
14+
15+
using namespace sycl::ext::oneapi::experimental;
16+
using namespace sycl;
17+
namespace esimd = sycl::ext::intel::esimd;
18+
19+
SYCL_EXTERNAL
20+
[[intel::device_indirectly_callable]] void __regcall SIMD_CALLEE_VOID()
21+
SYCL_ESIMD_FUNCTION {
22+
esimd::slm_init<1024>();
23+
}
24+
25+
int main() {
26+
queue Q;
27+
nd_range<1> NDR{range<1>{2}, range<1>{2}};
28+
Q.parallel_for(NDR, [=](nd_item<1> NDI) [[intel::reqd_sub_group_size(16)]] {
29+
sub_group sg = NDI.get_sub_group();
30+
invoke_simd(sg, SIMD_CALLEE_VOID);
31+
}).wait();
32+
return 0;
33+
}
34+
// CHECK: slm_init must be called directly from ESIMD kernel.
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
// RUN: %clangxx -fsycl %s
2+
3+
// This test verifies usage of slm_init and local_accessor in different kernels
4+
// passes.
5+
6+
#include <iostream>
7+
#include <sycl/ext/intel/esimd.hpp>
8+
#include <sycl/sycl.hpp>
9+
10+
using namespace sycl;
11+
using namespace sycl::ext::intel::esimd;
12+
13+
int main() {
14+
queue Q;
15+
nd_range<1> NDR{range<1>{2}, range<1>{2}};
16+
Q.submit([&](handler &CGH) {
17+
auto InAcc = local_accessor<int, 1>(5, CGH);
18+
CGH.parallel_for(NDR, [=](nd_item<1> NDI) SYCL_ESIMD_KERNEL {
19+
scalar_load<int>(InAcc, 0);
20+
});
21+
}).wait();
22+
23+
Q.submit([&](handler &CGH) {
24+
CGH.parallel_for(NDR, [=](nd_item<1> NDI)
25+
SYCL_ESIMD_KERNEL { slm_init(1024); });
26+
}).wait();
27+
28+
return 0;
29+
}
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
// RUN: not %clangxx -fsycl %s 2>&1 | FileCheck %s
2+
3+
// This test verifies usage of slm_init and local_accessor triggers an error.
4+
5+
#include <iostream>
6+
#include <sycl/ext/intel/esimd.hpp>
7+
#include <sycl/sycl.hpp>
8+
9+
using namespace sycl;
10+
using namespace sycl::ext::intel::esimd;
11+
12+
int main() {
13+
queue Q;
14+
nd_range<1> NDR{range<1>{2}, range<1>{2}};
15+
Q.submit([&](handler &CGH) {
16+
auto InAcc = local_accessor<int, 1>(5, CGH);
17+
CGH.parallel_for(NDR, [=](nd_item<1> NDI) SYCL_ESIMD_KERNEL {
18+
slm_init(1024);
19+
scalar_load<int>(InAcc, 0);
20+
});
21+
}).wait();
22+
// CHECK: error: slm_init can not be used with local accessors.
23+
24+
return 0;
25+
}
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
// RUN: not %clangxx -fsycl %s 2>&1 | FileCheck %s
2+
3+
// This test verifies usage of slm_init and local_accessor triggers an error.
4+
5+
#include <iostream>
6+
#include <sycl/ext/intel/esimd.hpp>
7+
#include <sycl/sycl.hpp>
8+
9+
using namespace sycl;
10+
using namespace sycl::ext::intel::esimd;
11+
12+
int main() {
13+
queue Q;
14+
nd_range<1> NDR{range<1>{2}, range<1>{2}};
15+
Q.submit([&](handler &CGH) {
16+
auto InAcc = local_accessor<int, 1>(5, CGH);
17+
CGH.parallel_for(NDR, [=](nd_item<1> NDI) SYCL_ESIMD_KERNEL {
18+
slm_init(1024);
19+
InAcc[0] = 5;
20+
});
21+
}).wait();
22+
// CHECK: error: slm_init can not be used with local accessors.
23+
24+
return 0;
25+
}
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
// RUN: not %clangxx -fsycl %s 2>&1 | FileCheck %s
2+
3+
// This test verifies call to slm_init from a function marked as
4+
// noinline triggers an error.
5+
6+
#include <iostream>
7+
#include <sycl/ext/intel/esimd.hpp>
8+
#include <sycl/sycl.hpp>
9+
10+
using namespace sycl;
11+
using namespace sycl::ext::intel::esimd;
12+
13+
#ifdef _MSC_VER
14+
#define __SYCL_NOINLINE __declspec(noinline)
15+
#else
16+
#define __SYCL_NOINLINE __attribute__((noinline))
17+
#endif
18+
19+
__SYCL_NOINLINE void bar() { slm_init(1024); }
20+
__SYCL_NOINLINE void foo() {
21+
slm_init(1024);
22+
bar();
23+
}
24+
25+
int main() {
26+
queue Q;
27+
nd_range<1> NDR{range<1>{2}, range<1>{2}};
28+
Q.parallel_for(NDR, [=](nd_item<1> NDI) SYCL_ESIMD_KERNEL { foo(); }).wait();
29+
return 0;
30+
}
31+
// CHECK: error: slm_init is called more than once from kernel 'typeinfo name for main::'lambda'(sycl::_V1::nd_item<1>)'.

0 commit comments

Comments
 (0)