-
Notifications
You must be signed in to change notification settings - Fork 743
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
[SYCL][Graph] Fix CUDA/HIP local mem argument update bug #16025
Open
EwanC
wants to merge
1
commit into
intel:sycl
Choose a base branch
from
reble:ewan/cuda_local_mem_fix
base: sycl
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1 @@ | ||
# commit 9937d029c7fdcbf101e89f8515f640c145e059c5 | ||
# Merge: 9ac6d5d9 10b0e101 | ||
# Author: Callum Fare <[email protected]> | ||
# Date: Wed Nov 20 14:49:17 2024 +0000 | ||
# Merge pull request #2258 from aarongreig/aaron/tryUseExtensionSubgroupInfo | ||
# Use extension version of clGetKernelSubGroupInfo when necessary. | ||
set(UNIFIED_RUNTIME_TAG 9937d029c7fdcbf101e89f8515f640c145e059c5) | ||
set(UNIFIED_RUNTIME_TAG "ewan/cuda_update_local_size") |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
10 changes: 10 additions & 0 deletions
10
sycl/test-e2e/Graph/Explicit/local_accessor_multiple_accessors.cpp
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
// RUN: %{build} -o %t.out | ||
// RUN: %{run} %t.out | ||
// Extra run to check for leaks in Level Zero using UR_L0_LEAKS_DEBUG | ||
// RUN: %if level_zero %{env SYCL_PI_LEVEL_ZERO_USE_IMMEDIATE_COMMANDLISTS=0 %{l0_leak_check} %{run} %t.out 2>&1 | FileCheck %s --implicit-check-not=LEAK %} | ||
// Extra run to check for immediate-command-list in Level Zero | ||
// RUN: %if level_zero %{env SYCL_PI_LEVEL_ZERO_USE_IMMEDIATE_COMMANDLISTS=1 %{l0_leak_check} %{run} %t.out 2>&1 | FileCheck %s --implicit-check-not=LEAK %} | ||
|
||
#define GRAPH_E2E_EXPLICIT | ||
|
||
#include "../Inputs/local_accessor_multiple_accessors.cpp" |
10 changes: 10 additions & 0 deletions
10
sycl/test-e2e/Graph/Explicit/local_accessor_multiple_nodes.cpp
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
// RUN: %{build} -o %t.out | ||
// RUN: %{run} %t.out | ||
// Extra run to check for leaks in Level Zero using UR_L0_LEAKS_DEBUG | ||
// RUN: %if level_zero %{env SYCL_PI_LEVEL_ZERO_USE_IMMEDIATE_COMMANDLISTS=0 %{l0_leak_check} %{run} %t.out 2>&1 | FileCheck %s --implicit-check-not=LEAK %} | ||
// Extra run to check for immediate-command-list in Level Zero | ||
// RUN: %if level_zero %{env SYCL_PI_LEVEL_ZERO_USE_IMMEDIATE_COMMANDLISTS=1 %{l0_leak_check} %{run} %t.out 2>&1 | FileCheck %s --implicit-check-not=LEAK %} | ||
|
||
#define GRAPH_E2E_EXPLICIT | ||
|
||
#include "../Inputs/local_accessor_multiple_nodes.cpp" |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
// RUN: %{build} -o %t.out | ||
// RUN: %{run} %t.out | ||
// Extra run to check for leaks in Level Zero using UR_L0_LEAKS_DEBUG | ||
// RUN: %if level_zero %{env SYCL_PI_LEVEL_ZERO_USE_IMMEDIATE_COMMANDLISTS=0 %{l0_leak_check} %{run} %t.out 2>&1 | FileCheck %s --implicit-check-not=LEAK %} | ||
// Extra run to check for immediate-command-list in Level Zero | ||
// RUN: %if level_zero %{env SYCL_PI_LEVEL_ZERO_USE_IMMEDIATE_COMMANDLISTS=1 %{l0_leak_check} %{run} %t.out 2>&1 | FileCheck %s --implicit-check-not=LEAK %} | ||
|
||
// REQUIRES: ocloc && (opencl || level_zero) | ||
|
||
#define GRAPH_E2E_EXPLICIT | ||
|
||
#include "../Inputs/opencl_local_acc.cpp" |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
62 changes: 62 additions & 0 deletions
62
sycl/test-e2e/Graph/Inputs/local_accessor_multiple_accessors.cpp
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,62 @@ | ||
// Tests adding of nodes with more than one local accessor, | ||
// and submitting the graph. | ||
|
||
#include "../graph_common.hpp" | ||
|
||
int main() { | ||
queue Queue{}; | ||
|
||
using T = int; | ||
|
||
const size_t LocalSize = 128; | ||
|
||
std::vector<T> HostData(Size); | ||
|
||
std::iota(HostData.begin(), HostData.end(), 10); | ||
|
||
exp_ext::command_graph Graph{Queue.get_context(), Queue.get_device()}; | ||
|
||
T *PtrIn = malloc_device<T>(Size, Queue); | ||
T *PtrOut = malloc_device<T>(Size, Queue); | ||
|
||
Queue.memset(PtrOut, 0, Size * sizeof(T)); | ||
Queue.copy(HostData.data(), PtrIn, Size); | ||
Queue.wait_and_throw(); | ||
|
||
auto Node = add_node(Graph, Queue, [&](handler &CGH) { | ||
local_accessor<T, 1> LocalMemA(LocalSize, CGH); | ||
local_accessor<T, 1> LocalMemB(LocalSize, CGH); | ||
|
||
CGH.parallel_for(nd_range({Size}, {LocalSize}), [=](nd_item<1> Item) { | ||
auto LocalID = Item.get_local_linear_id(); | ||
auto GlobalID = Item.get_global_linear_id(); | ||
LocalMemA[LocalID] = GlobalID; | ||
LocalMemB[LocalID] = PtrIn[GlobalID]; | ||
PtrOut[GlobalID] += LocalMemA[LocalID] * LocalMemB[LocalID]; | ||
}); | ||
}); | ||
|
||
auto GraphExec = Graph.finalize(); | ||
|
||
for (unsigned n = 0; n < Iterations; n++) { | ||
Queue.submit([&](handler &CGH) { CGH.ext_oneapi_graph(GraphExec); }); | ||
} | ||
|
||
Queue.wait_and_throw(); | ||
|
||
Queue.copy(PtrOut, HostData.data(), Size); | ||
Queue.wait_and_throw(); | ||
|
||
free(PtrIn, Queue); | ||
free(PtrOut, Queue); | ||
|
||
for (size_t i = 0; i < Size; i++) { | ||
T Ref = 0; | ||
for (size_t n = 0; n < Iterations; n++) { | ||
Ref += (i * (10 + i)); | ||
} | ||
assert(check_value(i, Ref, HostData[i], "PtrOut")); | ||
} | ||
|
||
return 0; | ||
} |
71 changes: 71 additions & 0 deletions
71
sycl/test-e2e/Graph/Inputs/local_accessor_multiple_nodes.cpp
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,71 @@ | ||
// Test creating a graph where more than one nodes uses local accessors, | ||
// and submits of the graph. | ||
|
||
#include "../graph_common.hpp" | ||
|
||
int main() { | ||
queue Queue{}; | ||
|
||
using T = int; | ||
|
||
const size_t LocalSize = 128; | ||
|
||
std::vector<T> HostData(Size); | ||
|
||
std::iota(HostData.begin(), HostData.end(), 10); | ||
|
||
exp_ext::command_graph Graph{Queue.get_context(), Queue.get_device()}; | ||
|
||
T *PtrA = malloc_device<T>(Size, Queue); | ||
|
||
Queue.copy(HostData.data(), PtrA, Size); | ||
Queue.wait_and_throw(); | ||
|
||
auto NodeA = add_node(Graph, Queue, [&](handler &CGH) { | ||
local_accessor<T, 1> LocalMem(LocalSize, CGH); | ||
|
||
CGH.parallel_for(nd_range({Size}, {LocalSize}), [=](nd_item<1> Item) { | ||
LocalMem[Item.get_local_linear_id()] = Item.get_global_linear_id() * 2; | ||
PtrA[Item.get_global_linear_id()] += LocalMem[Item.get_local_linear_id()]; | ||
}); | ||
}); | ||
|
||
auto NodeB = add_node( | ||
Graph, Queue, | ||
[&](handler &CGH) { | ||
local_accessor<T, 1> LocalMem(LocalSize, CGH); | ||
depends_on_helper(CGH, NodeA); | ||
|
||
CGH.parallel_for(nd_range({Size}, {LocalSize}), [=](nd_item<1> Item) { | ||
LocalMem[Item.get_local_linear_id()] = 3; | ||
PtrA[Item.get_global_linear_id()] *= | ||
LocalMem[Item.get_local_linear_id()]; | ||
}); | ||
}, | ||
NodeA); | ||
|
||
auto GraphExec = Graph.finalize(); | ||
|
||
for (unsigned n = 0; n < Iterations; n++) { | ||
Queue.submit([&](handler &CGH) { CGH.ext_oneapi_graph(GraphExec); }); | ||
} | ||
|
||
Queue.wait_and_throw(); | ||
|
||
Queue.copy(PtrA, HostData.data(), Size); | ||
Queue.wait_and_throw(); | ||
|
||
free(PtrA, Queue); | ||
|
||
for (size_t i = 0; i < Size; i++) { | ||
T Ref = 10 + i; | ||
|
||
for (size_t n = 0; n < Iterations; n++) { | ||
Ref += i * 2; | ||
Ref *= 3; | ||
} | ||
assert(check_value(i, Ref, HostData[i], "PtrA")); | ||
} | ||
|
||
return 0; | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,78 @@ | ||
// Tests using an OpenCL-C defined kernel with multiple local accessors | ||
|
||
#include "../graph_common.hpp" | ||
|
||
using source_kb = sycl::kernel_bundle<sycl::bundle_state::ext_oneapi_source>; | ||
using exe_kb = sycl::kernel_bundle<sycl::bundle_state::executable>; | ||
|
||
auto constexpr LocalAccCLSource = R"===( | ||
kernel void test_la(__global int *out, __local int* local_ptr1, | ||
__local int2* local_ptr2, int n) { | ||
__local int4 local_data[1]; | ||
|
||
size_t gid = get_global_id(0); | ||
size_t lid = get_local_id(0); | ||
size_t wg_size = get_num_groups(0); | ||
|
||
local_ptr1[lid] = lid; | ||
local_ptr2[lid].x = n; | ||
local_ptr2[lid].y = wg_size; | ||
|
||
if (lid == 0) { | ||
local_data[lid] = (int4)(0xA, 0xB, 0xC, 0xD); | ||
} | ||
|
||
barrier(CLK_LOCAL_MEM_FENCE); | ||
|
||
int acc = local_data[0].x + local_data[0].y + local_data[0].z + | ||
local_data[0].w; | ||
out[gid] = (local_ptr1[lid] * local_ptr2[lid].x) + | ||
(local_ptr2[lid].y * acc); | ||
} | ||
)==="; | ||
|
||
int main() { | ||
queue Queue; | ||
|
||
source_kb kbSrc = exp_ext::create_kernel_bundle_from_source( | ||
Queue.get_context(), exp_ext::source_language::opencl, LocalAccCLSource); | ||
exe_kb kbExe1 = exp_ext::build(kbSrc); | ||
sycl::kernel test_kernel = kbExe1.ext_oneapi_get_kernel("test_la"); | ||
|
||
exp_ext::command_graph Graph{Queue}; | ||
|
||
int32_t *Ptr = malloc_device<int32_t>(Size, Queue); | ||
|
||
int32_t N = 42; | ||
constexpr size_t LocalSize = 256; | ||
auto Node = add_node(Graph, Queue, [&](handler &cgh) { | ||
local_accessor<int32_t, 1> acc_local1(LocalSize, cgh); | ||
local_accessor<sycl::int2, 1> acc_local2(LocalSize, cgh); | ||
|
||
cgh.set_arg(0, Ptr); | ||
cgh.set_arg(1, acc_local1); | ||
cgh.set_arg(2, acc_local2); | ||
cgh.set_arg(3, N); | ||
|
||
cgh.parallel_for(nd_range<1>(Size, LocalSize), test_kernel); | ||
}); | ||
|
||
auto ExecGraph = Graph.finalize(); | ||
Queue.ext_oneapi_graph(ExecGraph).wait(); | ||
|
||
std::vector<int32_t> HostData(Size); | ||
Queue.copy(Ptr, HostData.data(), Size).wait(); | ||
|
||
constexpr int32_t Acc = 0xA + 0xB + 0xC + 0xD; | ||
constexpr int32_t WorkGroups = Size / LocalSize; | ||
constexpr int32_t Tmp = Acc * WorkGroups; | ||
for (size_t i = 0; i < Size; i++) { | ||
int32_t local_id = i % LocalSize; | ||
int32_t Ref = (local_id * N) + Tmp; | ||
assert(HostData[i] == Ref); | ||
} | ||
|
||
sycl::free(Ptr, Queue); | ||
|
||
return 0; | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not related to the changes but why does this test have
on line 9?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, I can see the other tests also use that. I guess it's convenient
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question, it's in a lot of our tests. The idea we that it makes it easier to test with different input/output types if we want. However, I don't think that hypothetical advantage has ever been taken advantage of.
Thinking about it, it might make more sense to put this in the
graph_common.hpp
header and let individual tests use it if they want.