[SYCL] Add group local memory call lowering pass #3329

mlychkov · 2021-03-09T16:35:20Z

The following call is included into device code for work group
by SYCL headers to declare static local memory allocation in SYCL kernel:
'attribute((opencl_local)) uint8_t *Ptr = __sycl_allocateLocalMemory(sizeof(T), alignof(T))'.
Then during compilation it should be transformed somewhere to
equivalent of the following code:
'__local alignas(alignof(T)) uint8_t Mem[sizeof(T)];
__local uint8_t *Ptr = &Mem;'

Add a pass that handles creation of fixed-size allocations in local
address space at the kernel scope.

Signed-off-by: Mikhail Lychkov [email protected]

kbobrovs

Why is this needed, what problem does this patch solve?
Why this is part of post-link?
Please add tests

mlychkov · 2021-03-16T12:34:17Z

Why is this needed, what problem does this patch solve?

Why this is part of post-link?

Please add tests

Updated commit message.
Moved pass to clang FE.
Added test.

clang/lib/CodeGen/BackendUtil.cpp

bader

@mlychkov, please, resolve merge conflicts.

llvm/test/Transforms/LowerWGLocalMemoryCall/group_local_memory.ll

llvm/lib/Transforms/Utils/LowerWGLocalMemoryCall.cpp

The following call is included into device code for work group by SYCL headers to declare static local memory allocation in SYCL kernel: '__attribute__((opencl_local)) uint8_t *Ptr = __sycl_allocateLocalMemory(sizeof(T), alignof(T))'. Then during compilation it should be transformed somewhere to equivalent of the following code: '__local alignas(alignof(T)) uint8_t Mem[sizeof(T)]; __local uint8_t *Ptr = &Mem;' Add a pass that handles creation of fixed-size allocations in local address space at the kernel scope. Signed-off-by: Mikhail Lychkov <[email protected]>

llvm/lib/SYCLLowerIR/LowerWGLocalMemory.cpp

Signed-off-by: Mikhail Lychkov <[email protected]>

kbobrovs

Thanks for moving. Few more comments.

Do you have end-to-end test, BTW or this is WIP? It is necessary for such functionality.

llvm/lib/SYCLLowerIR/LowerWGLocalMemory.cpp

llvm/include/llvm/SYCLLowerIR/LowerWGLocalMemory.h

llvm/test/SYCLLowerIR/group_local_memory.ll

llvm/lib/SYCLLowerIR/LowerWGLocalMemory.cpp

clang/lib/CodeGen/BackendUtil.cpp

mlychkov · 2021-03-19T11:13:04Z

Thanks for moving. Few more comments.

Do you have end-to-end test, BTW or this is WIP? It is necessary for such functionality.

End-to-end test for this feature is added by Semenov Sergey: intel/llvm-test-suite#176.

Signed-off-by: Mikhail Lychkov <[email protected]>

bader

LGTM, although I'm not sure if it works for NVPTX target.

AlexeySachkov

LGTM, but the main LIT test for this change seems to be failing

Signed-off-by: Mikhail Lychkov <[email protected]>

premanandrao

FE changes look okay to me.

mlychkov · 2021-03-25T06:03:32Z

@kbobrovs Could you please review last changes?

sergey-semenov · 2021-03-25T14:03:00Z

LGTM, although I'm not sure if it works for NVPTX target.

@mlychkov @bader The calling convention assertion https://github.com/intel/llvm/pull/3329/files#diff-a107684934906a250c3dc947338a6a685dc924e9246d6a33adc0b82f9b9de756R66-R68 fails for NVPTX (the default C convention is returned instead of the expected SPIR_FUNC/SPIR_KERNEL), but the E2E test passes after commenting the assertion out.

bader · 2021-03-25T15:53:30Z

LGTM, although I'm not sure if it works for NVPTX target.

@mlychkov @bader The calling convention assertion https://github.com/intel/llvm/pull/3329/files#diff-a107684934906a250c3dc947338a6a685dc924e9246d6a33adc0b82f9b9de756R66-R68 fails for NVPTX (the default C convention is returned instead of the expected SPIR_FUNC/SPIR_KERNEL), but the E2E test passes after commenting the assertion out.

I suggest removing the assert then.

Signed-off-by: Mikhail Lychkov <[email protected]>

bader · 2021-03-25T16:18:10Z

@kbobrovs, ping.

kbobrovs · 2021-03-26T17:12:39Z

@kbobrovs, ping.

@bader, @mlychkov - LGTM, except the diagnostics - I'd request at least TODO + issue (see my comment above).

kbobrovs

Approved. As agreed with @mlychkov, TODO + issue will follow.

mlychkov requested review from AlexeySachkov, MrSidims, kbobrovs and sergey-semenov March 9, 2021 16:35

kbobrovs suggested changes Mar 10, 2021

View reviewed changes

sergey-semenov mentioned this pull request Mar 10, 2021

Add a test for SYCL_INTEL_local_memory extension intel/llvm-test-suite#176

Merged

mlychkov changed the title ~~[SYCL] Add WG local memory call lowering pass~~ [SYCL] Add group local memory call lowering pass Mar 16, 2021

mlychkov force-pushed the private/mlychkov/kernel_local_mem_pass branch from 053ff77 to 3e2f205 Compare March 16, 2021 12:32

mlychkov marked this pull request as ready for review March 16, 2021 12:55

mlychkov requested review from bader, elizabethandrews and premanandrao as code owners March 16, 2021 12:55

mlychkov requested a review from kbobrovs March 16, 2021 12:55

elizabethandrews reviewed Mar 16, 2021

View reviewed changes

clang/lib/CodeGen/BackendUtil.cpp Outdated Show resolved Hide resolved

bader reviewed Mar 16, 2021

View reviewed changes

llvm/test/Transforms/LowerWGLocalMemoryCall/group_local_memory.ll Outdated Show resolved Hide resolved

llvm/test/Transforms/LowerWGLocalMemoryCall/group_local_memory.ll Outdated Show resolved Hide resolved

llvm/lib/Transforms/Utils/LowerWGLocalMemoryCall.cpp Outdated Show resolved Hide resolved

kbobrovs suggested changes Mar 17, 2021

View reviewed changes

llvm/lib/Transforms/Utils/LowerWGLocalMemoryCall.cpp Outdated Show resolved Hide resolved

sergey-semenov mentioned this pull request Mar 17, 2021

[SYCL] Add support for the SYCL_INTEL_local_memory extension to headers #3228

Merged

mlychkov force-pushed the private/mlychkov/kernel_local_mem_pass branch from 3e2f205 to 162cc19 Compare March 18, 2021 13:21

mlychkov requested a review from DenisBakhvalov as a code owner March 18, 2021 13:21

MrSidims reviewed Mar 18, 2021

View reviewed changes

llvm/lib/SYCLLowerIR/LowerWGLocalMemory.cpp Outdated Show resolved Hide resolved

llvm/lib/SYCLLowerIR/LowerWGLocalMemory.cpp Outdated Show resolved Hide resolved

Apply remarks

2c78c26

Signed-off-by: Mikhail Lychkov <[email protected]>

kbobrovs reviewed Mar 18, 2021

View reviewed changes

DenisBakhvalov reviewed Mar 18, 2021

View reviewed changes

llvm/test/SYCLLowerIR/group_local_memory.ll Outdated Show resolved Hide resolved

DenisBakhvalov reviewed Mar 18, 2021

View reviewed changes

llvm/lib/SYCLLowerIR/LowerWGLocalMemory.cpp Outdated Show resolved Hide resolved

DenisBakhvalov reviewed Mar 18, 2021

View reviewed changes

clang/lib/CodeGen/BackendUtil.cpp Outdated Show resolved Hide resolved

Fix lit error, other improvements

43782d8

Signed-off-by: Mikhail Lychkov <[email protected]>

mlychkov requested a review from kbobrovs March 22, 2021 12:24

bader previously approved these changes Mar 23, 2021

View reviewed changes

AlexeySachkov previously approved these changes Mar 23, 2021

View reviewed changes

DenisBakhvalov previously approved these changes Mar 23, 2021

View reviewed changes

Simplify code of the pass and fix lit test

4f34903

Signed-off-by: Mikhail Lychkov <[email protected]>

mlychkov dismissed stale reviews from DenisBakhvalov, AlexeySachkov, and bader via 4f34903 March 24, 2021 08:47

mlychkov requested review from AlexeySachkov and bader March 24, 2021 11:19

bader requested a review from DenisBakhvalov March 24, 2021 11:41

bader previously approved these changes Mar 24, 2021

View reviewed changes

AlexeySachkov previously approved these changes Mar 24, 2021

View reviewed changes

premanandrao previously approved these changes Mar 24, 2021

View reviewed changes

Remove calling conv assertion check

8350829

Signed-off-by: Mikhail Lychkov <[email protected]>

mlychkov dismissed stale reviews from premanandrao, AlexeySachkov, and bader via 8350829 March 25, 2021 16:14

mlychkov requested review from bader and premanandrao March 25, 2021 16:15

bader approved these changes Mar 25, 2021

View reviewed changes

kbobrovs approved these changes Mar 26, 2021

View reviewed changes

bader merged commit 9a734f6 into intel:sycl Mar 26, 2021

mlychkov mentioned this pull request Mar 29, 2021

Validate the scope of group-local variables definintion #3436

Closed

mlychkov deleted the private/mlychkov/kernel_local_mem_pass branch March 30, 2021 09:28

jcosborn mentioned this pull request Dec 14, 2022

Query: QUDA Feature-SYCL branch lattice/quda#1332

Open

[SYCL] Add group local memory call lowering pass #3329

[SYCL] Add group local memory call lowering pass #3329

Uh oh!

Conversation

mlychkov commented Mar 9, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kbobrovs left a comment

Choose a reason for hiding this comment

Uh oh!

mlychkov commented Mar 16, 2021

Uh oh!

Uh oh!

bader left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kbobrovs left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mlychkov commented Mar 19, 2021

Uh oh!

bader left a comment

Choose a reason for hiding this comment

Uh oh!

AlexeySachkov left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

premanandrao left a comment

Choose a reason for hiding this comment

Uh oh!

mlychkov commented Mar 25, 2021

Uh oh!

sergey-semenov commented Mar 25, 2021

Uh oh!

bader commented Mar 25, 2021

Uh oh!

bader commented Mar 25, 2021

Uh oh!

kbobrovs commented Mar 26, 2021

Uh oh!

kbobrovs left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mlychkov commented Mar 9, 2021 •

edited

Loading

AlexeySachkov left a comment •

edited

Loading