[GPU] Set minimum memory of count for reduce mean mode of scatter_elements_update, fix typos and remove space #30491

steve-y · 2025-05-11T12:25:25Z

Details:

Set minimum memory of count for reduce mean mode of scatter_elements_update
Fix typos and remove redundant spaces
Add unit test

Tickets:

155068

src/plugins/intel_gpu/src/kernel_selector/cl_kernels/scatter_elements_update_ref.cl

isanghao · 2025-05-12T03:35:58Z

src/plugins/intel_gpu/src/kernel_selector/cl_kernels/scatter_elements_update_ref.cl

-            __local int count_v[1];
+            __local int count_k[COUNT_LIMIT/64];
+            __local int count_v[COUNT_LIMIT/64];
+            count_length = COUNT_LIMIT/64;


what about implementing it from jitter-side? I think that will be clearer.

The fix was implemented from jitter-side as you commented.
Thanks for your comment.

isanghao · 2025-05-13T05:16:33Z

.../intel_gpu/src/kernel_selector/kernels/scatter_update/scatter_elements_update_kernel_ref.cpp

@@ -201,6 +201,7 @@ KernelsData ScatterElementsUpdateKernelRef::GetKernelsData(const Params& params)
        if (i == 1) {
            cldnn_jit.AddConstant(MakeJitConstant("IS_SECOND_ITER", "true"));
            cldnn_jit.AddConstant(MakeJitConstant("COUNT_LIMIT", params.engineInfo.maxLocalMemSize));
+            cldnn_jit.AddConstant(MakeJitConstant("COUNT_MINIMUM", params.engineInfo.maxLocalMemSize/64));


What does it mean that COUNT_LENGTH==0? Then I guess total workitem size is just 0 and no code will be executed. Isn't it?

what about just setting COUNT_LENGTH = dispatchData.gws[0] * dispatchData.gws[1] * dispatchData.gws[2] if dispatchData.gws[0] * dispatchData.gws[1] * dispatchData.gws[2] != 0 else COUNT_MINIMUM? Then you don't need to introduce additional variable.

Your suggested code did not introduce additional variable, and it was applied. Thanks for your comments.

src/plugins/intel_gpu/src/kernel_selector/cl_kernels/scatter_elements_update_ref.cl

yeonbok · 2025-05-28T02:39:39Z

.../intel_gpu/src/kernel_selector/kernels/scatter_update/scatter_elements_update_kernel_ref.cpp

-            cldnn_jit.AddConstant(MakeJitConstant("COUNT_LENGTH", dispatchData.gws[0] * dispatchData.gws[1] * dispatchData.gws[2]));
+            cldnn_jit.AddConstant(MakeJitConstant("COUNT_LENGTH", dispatchData.gws[0] * dispatchData.gws[1] * dispatchData.gws[2] != 0 ?
+                                                                  dispatchData.gws[0] * dispatchData.gws[1] * dispatchData.gws[2] :
+                                                                  params.engineInfo.maxLocalMemSize/64));


If dynamic shape, always use slm? why? I think it should use global mem if we cannot ensure the size to fit in the local mem.

As __global didn't work for function calling in cl kernel, it was updated to use __local and exception. Thanks for your comments.

yeonbok · 2025-06-04T18:27:25Z

.../intel_gpu/src/kernel_selector/kernels/scatter_update/scatter_elements_update_kernel_ref.cpp

@@ -199,9 +199,15 @@ KernelsData ScatterElementsUpdateKernelRef::GetKernelsData(const Params& params)
        auto entry_point = GetEntryPoint(kernelName, newParams.layerID, params, i);

        if (i == 1) {
+            auto count_limit = params.engineInfo.maxLocalMemSize;
+            auto count_length = dispatchData.gws[0] * dispatchData.gws[1] * dispatchData.gws[2];
+            auto min_count_length = count_limit / 64;


what is this 64 for ? if we need to add a hard coded number, please use a const variable name with more info e.g., const int var_name_for_purpose = 64;

Also having ref kernel be limited to # of element fit for local mem is not acceptable. e.g., desktop igpus w/ local mem size 64K/64 => 1024 elements is not acceptable.

So we should be able to handle such cases not fit for local mem size.

i.e., we should have two versions 1) use slm 2) not use slm. Please check softmax implementation

PR was updated like;

As count values could be known while execution, allocatable local memory needed to be provided.

Max allocatable local memory size was found. It was about maxLocalMemSize / 8, not maxLocalMemSize. (It was checked using cliloader.)

As count had k and v, each allocatable memory was maxLocalMemSize / 8 / 2.

If required memory was over allocatable memory, it was allocated in global memory. And if not, it was allocated in local memory.

As __global could not be used in function call, macro functions were used instead.

[DRAFT] [GPU] Set internal memory of count for reduce mean #30930 uses internal buffer for dynamic case.

Thanks for your comments.

What I request was to use SLM when available (i.e., count_length is less than the limit) and use global memory when not availalbe (i.e., count_length is larger than the limit). Currently if dynamic shape, always using global memory.

COUNT_LENGTH is used for array size for count_k and count_v, so I thought COUNT_LENGTH should be fixed before run and not changed when running. And COUNT_LENGTH == COUNT_LIMIT for dynamic shape, __local is used with its maximum size. What do you think?

…ve redundant spaces

…ments_update and move the fix to jitter-side

…x get_count argument logic

… is requested

…ding on its requested size

yeonbok · 2025-06-20T03:06:55Z

.../intel_gpu/src/kernel_selector/kernels/scatter_update/scatter_elements_update_kernel_ref.cpp

-            cldnn_jit.AddConstant(MakeJitConstant("COUNT_LIMIT", params.engineInfo.maxLocalMemSize));
-            cldnn_jit.AddConstant(MakeJitConstant("COUNT_LENGTH", newParams.inputs[1].LogicalSize()));
+            cldnn_jit.AddConstant(MakeJitConstant("COUNT_LIMIT", maxAllocatableMemSize));
+            cldnn_jit.AddConstant(MakeJitConstant("COUNT_LENGTH", newParams.inputs[1].LogicalSize() != 0 ?


Why if dynamic, always count_length is max slm size?
We should define actual size using the shape something like INPUT_SIZE_XXX.....

As count_length is used for array size of count, it should be fixed before run, so it is max slm size.
I think I didn't understand how INPUT_SIZE_XXX... used, and could you explain more?

steve-y requested review from a team as code owners May 11, 2025 12:25

github-actions bot added the category: GPU OpenVINO GPU plugin label May 11, 2025

isanghao reviewed May 12, 2025

View reviewed changes

steve-y changed the title ~~[GPU] Set minimum memory of count for reduce mean, fix typos and remove space~~ [GPU] Set minimum memory of count for reduce mean mode of scatter_elements_update, fix typos and remove space May 12, 2025

steve-y force-pushed the sy/fix_reduce_mean branch from 53c2aa7 to dd8ef87 Compare May 13, 2025 00:00

isanghao reviewed May 13, 2025

View reviewed changes

michal-miotk reviewed May 26, 2025

View reviewed changes

src/plugins/intel_gpu/src/kernel_selector/cl_kernels/scatter_elements_update_ref.cl Outdated Show resolved Hide resolved

steve-y force-pushed the sy/fix_reduce_mean branch from dd8ef87 to 09c57b5 Compare May 27, 2025 06:16

yeonbok reviewed May 28, 2025

View reviewed changes

steve-y force-pushed the sy/fix_reduce_mean branch 4 times, most recently from 08ff7c1 to fe00c91 Compare June 2, 2025 05:31

yeonbok reviewed Jun 4, 2025

View reviewed changes

steve-y force-pushed the sy/fix_reduce_mean branch from fe00c91 to eab2f42 Compare June 10, 2025 03:01

steve-y changed the title ~~[GPU] Set minimum memory of count for reduce mean mode of scatter_elements_update, fix typos and remove space~~ [DO NOT REVIEW] [GPU] Set minimum memory of count for reduce mean mode of scatter_elements_update, fix typos and remove space Jun 10, 2025

steve-y changed the title ~~[DO NOT REVIEW] [GPU] Set minimum memory of count for reduce mean mode of scatter_elements_update, fix typos and remove space~~ [GPU] Set minimum memory of count for reduce mean mode of scatter_elements_update, fix typos and remove space Jun 10, 2025

steve-y added 5 commits June 10, 2025 10:41

[GPU] Set minimum memory of count for reduce mean, fix typos and remo…

9bfecfa

…ve redundant spaces

[GPU] Set minimum memory of count for reduce mean mode of scatter_ele…

2f8ac4e

…ments_update and move the fix to jitter-side

[GPU] Modify jitter-side not to introduce additional variable, and fi…

a42e179

…x get_count argument logic

[GPU] Update count functions and add throw exception if invalid count…

6d22c62

… is requested

[GPU] Allocate array count in GPU local memory or global memory depen…

eab2f42

…ding on its requested size

steve-y mentioned this pull request Jun 11, 2025

[DRAFT] [GPU] Set internal memory of count for reduce mean #30930

Open

yeonbok reviewed Jun 20, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[GPU] Set minimum memory of count for reduce mean mode of scatter_elements_update, fix typos and remove space #30491

[GPU] Set minimum memory of count for reduce mean mode of scatter_elements_update, fix typos and remove space #30491

steve-y commented May 11, 2025 •

edited

Loading

Uh oh!

Uh oh!

isanghao May 12, 2025

Uh oh!

steve-y May 13, 2025

Uh oh!

isanghao May 13, 2025

Uh oh!

steve-y May 27, 2025

Uh oh!

Uh oh!

yeonbok May 28, 2025

Uh oh!

steve-y May 28, 2025

Uh oh!

yeonbok Jun 4, 2025 •

edited

Loading

Uh oh!

steve-y Jun 10, 2025 •

edited

Loading

Uh oh!

yeonbok Jun 20, 2025

Uh oh!

steve-y Jun 20, 2025

Uh oh!

yeonbok Jun 20, 2025

Uh oh!

steve-y Jun 20, 2025

Uh oh!

Uh oh!

[GPU] Set minimum memory of count for reduce mean mode of scatter_elements_update, fix typos and remove space #30491

Are you sure you want to change the base?

[GPU] Set minimum memory of count for reduce mean mode of scatter_elements_update, fix typos and remove space #30491

Conversation

steve-y commented May 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Details:

Tickets:

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yeonbok Jun 4, 2025 • 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!

steve-y Jun 10, 2025 • 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

steve-y commented May 11, 2025 •

edited

Loading

yeonbok Jun 4, 2025 •

edited

Loading

steve-y Jun 10, 2025 •

edited

Loading