[ESIMD] Add semi-dynamic SLM allocation - esimd::experimental::slm_allocator. #7759

kbobrovs · 2022-12-13T08:15:22Z

This patch adds new class - slm_allocator - and its lowering. This is RAII-style class used to implement "semi-dynamic" SLM allocation. SLM is allocated in the constructor and released in the destructor, that's why it is "dynamic", as opposed to fully static allocation style of 'slm_init'. Actual offset of SLM chunk allocated by the call is calculated at compile time, that's why it is "semi-". To calculate SLM usage by a kernel, compiler finds a path in a callgraph with the largest amount of SLM "locked" by slm_allocator objects live along the paths. slm_init call also participates in calculating SLM budget. It can be modelled as slm_allocator object declared at the very beginning of a kernel and live till its the very end.

Since a call graph is used, function pointers and recursion is not supported.
Complementary E2E test: intel/llvm-test-suite#1449.

Signed-off-by: Konstantin S Bobrovsky [email protected]

…locator. This patch adds new class - slm_allocator - and its lowering. This is RAII-style class used to implement "semi-dynamic" SLM allocation. SLM is allocated in the constructor and released in the destructor, that's why it is "dynamic", as opposed to fully static allocation style of 'slm_init'. Actual offset of SLM chunk allocated by the call is calculated at compile time, that's why it is "semi-". To calculate SLM usage by a kernel, compiler finds a path in a callgraph with the largest amount of SLM "locked" by slm_allocator objects live along the paths. slm_init call also participates in calculating SLM budget. It can be modelled as slm_allocator object declared at the very beginning of a kernel and live till its the very end. Since a call graph is used, function pointers and recursion is not supported. Signed-off-by: Konstantin S Bobrovsky <[email protected]>

kbobrovs · 2022-12-13T08:15:50Z

Unit tests are in flight

llvm/include/llvm/SYCLLowerIR/ESIMD/ESIMDUtils.h

llvm/lib/SYCLLowerIR/CMakeLists.txt

llvm/lib/SYCLLowerIR/ESIMD/ESIMDUtils.cpp

kbobrovs · 2022-12-13T08:21:25Z

sycl/include/sycl/ext/intel/experimental/esimd/memory.hpp

@@ -1755,6 +1755,36 @@ __ESIMD_API simd<T, N> atomic_update(T *p, simd<unsigned, N> offset,
      p, offset, src1, src0, mask);
 }

+/// RAII-style class used to implement "semi-dynamic" SLM allocation.


@petercaday, please review this part. E2E test - intel/llvm-test-suite#1449.

kbobrovs · 2022-12-13T08:44:03Z

llvm/lib/SYCLLowerIR/LowerInvokeSimd.cpp

@@ -19,7 +19,9 @@
 //   deducible when BE is ready

 #include "llvm/SYCLLowerIR/LowerInvokeSimd.h"
+


@rolandschulz, please review this file

v-klochkov · 2022-12-13T22:42:13Z

@kbobrovs - please take a look at BUILD+LIT fail: esimd/genx_func_attr.cpp

kbobrovs · 2022-12-14T00:14:23Z

@kbobrovs - please take a look at BUILD+LIT fail: esimd/genx_func_attr.cpp

Yes, looking into this.

sycl/include/sycl/ext/intel/experimental/esimd/memory.hpp

v-klochkov

Please take a look at few ESIMD fails, e.g.
slm_gather_scatter_rgba.cpp test prints:
LLVM ERROR: slm_init call met in non-kernel function _ZNK6KernelIiLj8ELN4sycl3_V13ext5intel5esimd17rgba_channel_maskE1EEclENS1_7nd_itemILi1EEE

llvm/lib/SYCLLowerIR/ESIMD/LowerESIMDSlmReservation.cpp

kbobrovs · 2022-12-15T09:21:41Z

/verify with intel/llvm-test-suite#1449

v-klochkov · 2022-12-15T23:27:24Z

@kbobrovs - I just noticed that esimd/genx_func_attr.cpp test fails. Is that the test that you mentioned as the one failing with slm_init call in SYCL EXTERNAL FUNCTION ?

v-klochkov · 2022-12-15T23:50:11Z

@asudarsa - would you please review this patch to let Konst fix the issues in sycl-post-link if there are any concerns from your side.

asudarsa · 2022-12-16T00:53:18Z

I have started looking at this. Sorry for delay. I had a quick question about how we handle overflows. Thanks

kbobrovs · 2022-12-16T01:05:26Z

I have started looking at this. Sorry for delay. I had a quick question about how we handle overflows. Thanks

Overflows are detected at runtime by the JITter, as we don't know available SLM size at compile-time.

kbobrovs · 2022-12-16T01:05:57Z

@kbobrovs - I just noticed that esimd/genx_func_attr.cpp test fails. Is that the test that you mentioned as the one failing with slm_init call in SYCL EXTERNAL FUNCTION ?

Yes, I'm looking into this

asudarsa · 2022-12-16T01:08:33Z

Hi @kbobrovs and @v-klochkov

All the sycl-post-link related changes are NFC changes and look fine. I will take a quick look at the full patch before approval. Hope that's ok. Please let me know if I missed something.

Thanks

asudarsa

Looks good from DPCPP Tools perspective. Thanks

petercaday · 2022-12-19T06:00:54Z

sycl/include/sycl/ext/intel/experimental/esimd/memory.hpp

+/// as a class' template argument.
+///
+/// Since a call graph is used, function pointers and recursion is not
+/// supported.


Are these cases detected and flagged as compile-time errors?

Actually, no. Recursion is not a problem, because it is not supported by SYCL. But presence of function pointers should be detected, agree. Can diagnostics be added as a follow-up commit? Tagging @v-klochkov.

But presence of function pointers should be detected, agree.

@petercaday, I added diagnostics in 8d20150

Thanks, @kbobrovs!

petercaday

Looks good to me. Thanks for implementing this, @kbobrovs!

bader · 2022-12-22T01:15:06Z

@v-klochkov, could you fix pre-commit failures, please?

v-klochkov · 2022-12-22T05:46:07Z

@v-klochkov, could you fix pre-commit failures, please?

I'll start looking at this LIT test fail after returning from vacation (Jan 2023)

kbobrovs · 2022-12-25T14:23:11Z

@v-klochkov, could you fix pre-commit failures, please?

I'll start looking at this LIT test fail after returning from vacation (Jan 2023)

Folks, I'm looking into this failure. Hope can fix before the NY

* maximum SLM offset calculation for each scope * maximum SLM usage per kernel This also fixed a bug in calculating maximum SLM usage per kernel. - Update the unit test to use new API and to require correct SLM size for each kernel. Multiple slm_init's and slm_init's in non-inlined SYCL_EXTERNAL functions did not work anyway. Signed-off-by: kbobrovs <[email protected]>

Signed-off-by: kbobrovs <[email protected]>

kbobrovs · 2022-12-28T10:14:20Z

@bader, would you mind merging, please?

AlexeySachkov · 2022-12-28T12:44:45Z

Post-commit failed on MacOS due to missing include, the fix is in #7880

kbobrovs requested review from v-klochkov, rolandschulz and a team as code owners December 13, 2022 08:15

kbobrovs commented Dec 13, 2022

View reviewed changes

llvm/include/llvm/SYCLLowerIR/ESIMD/ESIMDUtils.h Show resolved Hide resolved

kbobrovs commented Dec 13, 2022

View reviewed changes

llvm/lib/SYCLLowerIR/CMakeLists.txt Show resolved Hide resolved

kbobrovs commented Dec 13, 2022

View reviewed changes

llvm/lib/SYCLLowerIR/ESIMD/ESIMDUtils.cpp Show resolved Hide resolved

kbobrovs mentioned this pull request Dec 13, 2022

[ESIMD] E2E test for slm_allocator API. intel/llvm-test-suite#1449

Merged

kbobrovs commented Dec 13, 2022

View reviewed changes

kbobrovs requested a review from kychendev December 13, 2022 08:45

v-klochkov reviewed Dec 14, 2022

View reviewed changes

sycl/include/sycl/ext/intel/experimental/esimd/memory.hpp Outdated Show resolved Hide resolved

v-klochkov reviewed Dec 14, 2022

View reviewed changes

llvm/lib/SYCLLowerIR/ESIMD/LowerESIMDSlmReservation.cpp Outdated Show resolved Hide resolved

llvm/lib/SYCLLowerIR/ESIMD/LowerESIMDSlmReservation.cpp Outdated Show resolved Hide resolved

kbobrovs added 5 commits December 14, 2022 14:31

address review comments - SLM amount is now template parameter.

7fdc39c

Fix test failure, allow non-constants in slm_init if w/o slm_allocator.

919bebc

clang-format

7eb44c4

fixed bug in SLM frame size calculation for kernel.

d328e79

clang-format

5d8ee93

kbobrovs added 2 commits December 15, 2022 01:25

Merge remote-tracking branch 'intel/sycl' into dyn_slm_alloc

7648834

fix comment

29967aa

kbobrovs requested a review from v-klochkov December 15, 2022 09:27

v-klochkov approved these changes Dec 15, 2022

View reviewed changes

asudarsa approved these changes Dec 16, 2022

View reviewed changes

petercaday reviewed Dec 19, 2022

View reviewed changes

petercaday approved these changes Dec 19, 2022

View reviewed changes

kbobrovs added 3 commits December 27, 2022 18:23

Merge remote-tracking branch 'intel/sycl' into dyn_slm_alloc

b36583e

Fix the unit test.

aa7b0e0

Signed-off-by: kbobrovs <[email protected]>

kbobrovs temporarily deployed to aws December 27, 2022 14:13 — with GitHub Actions Inactive

disable too eager diagnostics - add TODO.

8d20150

Signed-off-by: kbobrovs <[email protected]>

kbobrovs force-pushed the dyn_slm_alloc branch from e814d80 to 8d20150 Compare December 27, 2022 14:34

kbobrovs temporarily deployed to aws December 27, 2022 14:58 — with GitHub Actions Inactive

kbobrovs temporarily deployed to aws December 27, 2022 15:28 — with GitHub Actions Inactive

Diagnose joint use of indirect calls and slm_allocator.

beb5ef5

Signed-off-by: kbobrovs <[email protected]>

kbobrovs temporarily deployed to aws December 28, 2022 09:28 — with GitHub Actions Inactive

kbobrovs temporarily deployed to aws December 28, 2022 09:58 — with GitHub Actions Inactive

AlexeySachkov merged commit e767401 into intel:sycl Dec 28, 2022

		@@ -19,7 +19,9 @@
		// deducible when BE is ready

		#include "llvm/SYCLLowerIR/LowerInvokeSimd.h"

[ESIMD] Add semi-dynamic SLM allocation - esimd::experimental::slm_allocator. #7759

[ESIMD] Add semi-dynamic SLM allocation - esimd::experimental::slm_allocator. #7759

Uh oh!

Conversation

kbobrovs commented Dec 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kbobrovs commented Dec 13, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kbobrovs Dec 13, 2022

Choose a reason for hiding this comment

Uh oh!

kbobrovs Dec 13, 2022

Choose a reason for hiding this comment

Uh oh!

v-klochkov commented Dec 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kbobrovs commented Dec 14, 2022

Uh oh!

Uh oh!

v-klochkov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

kbobrovs commented Dec 15, 2022

Uh oh!

v-klochkov commented Dec 15, 2022

Uh oh!

v-klochkov commented Dec 15, 2022

Uh oh!

asudarsa commented Dec 16, 2022

Uh oh!

kbobrovs commented Dec 16, 2022

Uh oh!

kbobrovs commented Dec 16, 2022

Uh oh!

asudarsa commented Dec 16, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

asudarsa left a comment

Choose a reason for hiding this comment

Uh oh!

petercaday Dec 19, 2022

Choose a reason for hiding this comment

Uh oh!

kbobrovs Dec 27, 2022

Choose a reason for hiding this comment

Uh oh!

kbobrovs Dec 28, 2022

Choose a reason for hiding this comment

Uh oh!

petercaday Dec 31, 2022

Choose a reason for hiding this comment

Uh oh!

petercaday left a comment

Choose a reason for hiding this comment

Uh oh!

bader commented Dec 22, 2022

Uh oh!

v-klochkov commented Dec 22, 2022

Uh oh!

kbobrovs commented Dec 25, 2022

Uh oh!

kbobrovs commented Dec 28, 2022

Uh oh!

AlexeySachkov commented Dec 28, 2022

Uh oh!

Uh oh!

kbobrovs commented Dec 13, 2022 •

edited

Loading

v-klochkov commented Dec 13, 2022 •

edited

Loading

asudarsa commented Dec 16, 2022 •

edited

Loading