genenric:sycl: Group Norm Forward #2733

AD2605 · 2025-02-21T23:41:19Z

Description

Adds the implementation of group normalization for the SYCL backend.

General

Do all unit and benchdnn tests (make test and make test_benchdnn_*) pass locally for each commit?
Have you formatted the code using clang-format?

AD2605 · 2025-02-22T00:40:23Z

make test
disable build_vendor_intel
disable build_vendor_intel
disable test_device_cpu
enable build_vendor_generic
enable arch_gpu_ampere
enable compiler_icx-oss

AD2605 · 2025-02-22T00:44:47Z

I'm holding off on rebasing onto the latest main(22/02/2025) to resolve the conflict as it is leading to unrelated build failures -

src/gpu/generic/sycl/CMakeFiles/dnnl_gpu_generic_sycl.dir/ref_convolution.cpp.o.d -o src/gpu/generic/sycl/CMakeFiles/dnnl_gpu_generic_sycl.dir/ref_convolution.cpp.o -c /home/atharva/oneDNN_main/oneDNN/src/gpu/generic/sycl/ref_convolution.cpp
/home/atharva/oneDNN_main/oneDNN/src/gpu/generic/sycl/ref_convolution.cpp:51:56: error: no member named 'common' in 'dnnl::impl::zero_points_t'
   51 |     conf_.single_data_zeropoint = attr()->zero_points_.common(DNNL_ARG_SRC_0);
      |                                   ~~~~~~~~~~~~~~~~~~~~ ^
/home/atharva/oneDNN_main/oneDNN/src/gpu/generic/sycl/ref_convolution.cpp:114:56: error: no member named 'common' in 'dnnl::impl::zero_points_t'
  114 |     conf_.single_data_zeropoint = attr()->zero_points_.common(DNNL_ARG_SRC_0);
      |                                   ~~~~~~~~~~~~~~~~~~~~ ^
/home/atharva/oneDNN_main/oneDNN/src/gpu/generic/sycl/ref_convolution.cpp:178:56: error: no member named 'common' in 'dnnl::impl::zero_points_t'
  178 |     conf_.single_data_zeropoint = attr()->zero_points_.common(DNNL_ARG_SRC_0);
      |                                   ~~~~~~~~~~~~~~~~~~~~ ^
3 errors generated.
[38/156] Building CXX object src/gpu/generic/sycl/CMakeFiles/dnnl_gpu_generic_sycl.dir/simple_reduction.cpp.o
ninja: build stopped: subcommand failed.
atharva@machine:~/oneDNN_main/oneDNN/__bld$ git remote -v
origin	git@github.com:oneapi-src/oneDNN.git (fetch)
origin	git@github.com:oneapi-src/oneDNN.git (push)
atharva@machine:~/oneDNN_main/oneDNN/__bld$ git show HEAD
commit 4cf009ce71a33819a2125a5ad471b717b8723aa7 (HEAD -> main, origin/main, origin/HEAD)
Author: Dmitrii Zarukin <dmitry.zarukin@intel.com>
Date:   Thu Feb 20 12:26:51 2025 -0800

(The above build is on the latest main (22/02/2025) with gpu vendor as generic)
Hence triggering the CI before resolving the conflict

dzarukin · 2025-02-22T20:27:46Z

My bad, somehow it slipped from me during the development, and Cuda-based build wasn't triggered by default for PR validation, I'll follow up on this.
I've pushed build fixes to the main branch, please rebase to pick them up.
Sorry for inconveniences.

AD2605 · 2025-02-22T21:11:44Z

Thanks a lot @dzarukin , and no issues at all :)

src/gpu/generic/sycl/ref_group_normalization.cpp

sgeor255 · 2025-02-24T10:54:37Z

@AD2605 we should also update the README file with group norm.

AD2605 · 2025-02-24T11:14:24Z

@AD2605 we should also update the README file with group norm.

Thanks for pointing that out, I forgot about. Will update the PR

AD2605 · 2025-02-24T12:25:27Z

make test
disable build_vendor_intel
disable build_vendor_intel
disable test_device_cpu
enable build_vendor_generic
enable arch_gpu_ampere
enable compiler_icx-oss

AD2605 · 2025-02-24T15:07:05Z

Pre commit failures are unrelated to the changes in this PR

mgouicem · 2025-02-25T17:05:13Z

src/gpu/generic/sycl/group_normalization_kernel.hpp

+* Copyright 2023-2024 Intel Corporation
+* Copyright 2024-2025 Codeplay Software Limited


For new files, please just use the current year. Please also fix the other new files in the PR.

mgouicem · 2025-02-25T17:13:49Z

src/gpu/generic/sycl/group_normalization_kernel.hpp

+                if (conf_.src_scaling) {
+                    // Only one scaling factor per tensor is allowed,
+                    // as per the spec. Scaling factor will also always be f32 as per spec.
+                    normalized_value *= load_float_value(
+                            data_type::f32, src_scale.get_pointer(), 0);
+                }
+                float prev_value = normalized_value;
+                normalized_value = conf_.post_ops.apply(
+                        normalized_value, prev_value, po_args_, logical_index);
+                if (conf_.dst_scaling) {
+                    // Only one scaling factor per tensor is allowed,
+                    // as per the spec. Scaling factor will also always be f32 as per spec.
+                    normalized_value *= (1.0f
+                            / load_float_value(data_type::f32,
+                                    dst_scale.get_pointer(), 0));
+                }


The common check seems to only cover scales mask, not scales datatype.
Could you at least add a proper check in the init() for the scales datatype?

Added the check

mgouicem · 2025-02-25T17:18:30Z

src/gpu/generic/sycl/ref_group_normalization.cpp

+            | primitive_attr_t::skip_mask_t::post_ops;
+    VDISPATCH_GNORM(
+            attr()->has_default_values(attr_mask), VERBOSE_UNSUPPORTED_ATTR);
+    VDISPATCH_GNORM(attr_scales_ok(), VERBOSE_UNSUPPORTED_SCALES_CFG);


In general, it is preferable for each implementation to check for attributes they skipped default checking, to avoid issues when we extend functionality.
Could you add the extra restrictions from implementation here (for datatype and mask)?

Added the check for both

AD2605 · 2025-03-03T12:42:25Z

@mgouicem please let me know in case you are happy with the current state of the PR and the resolutions to your comments. just wanted to check-in with you before the merge as the PR has received the required approvals

AD2605 requested review from a team as code owners February 21, 2025 23:41

github-actions bot added platform:gpu-generic Codeowner: @oneapi-src/onednn-gpu-generic component:tests Codeowner: @oneapi-src/onednn-arch labels Feb 21, 2025

AD2605 force-pushed the atharva/group_norm branch from 21ced41 to 0b960b9 Compare February 22, 2025 21:29

sgeor255 approved these changes Feb 24, 2025

View reviewed changes

src/gpu/generic/sycl/ref_group_normalization.cpp Outdated Show resolved Hide resolved

src/gpu/generic/sycl/ref_group_normalization.cpp Outdated Show resolved Hide resolved

AD2605 force-pushed the atharva/group_norm branch from 2a25d50 to 9b57921 Compare February 24, 2025 12:18

AD2605 requested a review from a team as a code owner February 24, 2025 12:18

github-actions bot added the documentation A request to change/fix/improve the documentation. Codeowner: @oneapi-src/onednn-doc label Feb 24, 2025

AD2605 changed the title ~~gpu:generic:ref_group_norm_fwd_t~~ genenric:sycl: Group Norm Forward Feb 24, 2025

AD2605 force-pushed the atharva/group_norm branch from 9b57921 to 99cc959 Compare February 24, 2025 12:24

mgouicem reviewed Feb 25, 2025

View reviewed changes

AD2605 force-pushed the atharva/group_norm branch from 99cc959 to 2e850eb Compare February 27, 2025 12:24

vpirogov approved these changes Feb 28, 2025

View reviewed changes

AD2605 force-pushed the atharva/group_norm branch from 2e850eb to 0e74961 Compare February 28, 2025 16:22

AD2605 requested a review from mgouicem March 3, 2025 12:35

gpu: generic: group norm fwd

0b5863f

AD2605 force-pushed the atharva/group_norm branch from 0e74961 to 0b5863f Compare March 4, 2025 10:37

AD2605 merged commit 639f1a0 into uxlfoundation:main Mar 4, 2025
22 checks passed

AD2605 mentioned this pull request Mar 5, 2025

ref: sycl: fix group norm build after quant refactor part 3 #2823

Merged

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

genenric:sycl: Group Norm Forward #2733

genenric:sycl: Group Norm Forward #2733

AD2605 commented Feb 21, 2025

AD2605 commented Feb 22, 2025

AD2605 commented Feb 22, 2025

dzarukin commented Feb 22, 2025

AD2605 commented Feb 22, 2025

sgeor255 commented Feb 24, 2025

AD2605 commented Feb 24, 2025

AD2605 commented Feb 24, 2025

AD2605 commented Feb 24, 2025

mgouicem Feb 25, 2025

AD2605 Feb 27, 2025

mgouicem Feb 25, 2025

AD2605 Feb 27, 2025

mgouicem Feb 25, 2025

AD2605 Feb 27, 2025

AD2605 commented Mar 3, 2025

		* Copyright 2023-2024 Intel Corporation
		* Copyright 2024-2025 Codeplay Software Limited

genenric:sycl: Group Norm Forward #2733

genenric:sycl: Group Norm Forward #2733

Conversation

AD2605 commented Feb 21, 2025

Description

General

AD2605 commented Feb 22, 2025

AD2605 commented Feb 22, 2025

dzarukin commented Feb 22, 2025

AD2605 commented Feb 22, 2025

sgeor255 commented Feb 24, 2025

AD2605 commented Feb 24, 2025

AD2605 commented Feb 24, 2025

AD2605 commented Feb 24, 2025

mgouicem Feb 25, 2025

Choose a reason for hiding this comment

AD2605 Feb 27, 2025

Choose a reason for hiding this comment

mgouicem Feb 25, 2025

Choose a reason for hiding this comment

AD2605 Feb 27, 2025

Choose a reason for hiding this comment

mgouicem Feb 25, 2025

Choose a reason for hiding this comment

AD2605 Feb 27, 2025

Choose a reason for hiding this comment

AD2605 commented Mar 3, 2025