Skip to content

[SYCL][ESIMD][EMU] Enable dpas with ESIMD_EMULATOR backend #6475

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

Merged
merged 15 commits into from
Aug 16, 2022

Conversation

dongkyunahn-intel
Copy link
Contributor

  • +fix for 'saturate<>' for ESIMD_EMULATOR backend

- +fix for 'saturate<>' for ESIMD_EMULATOR backend
@dongkyunahn-intel dongkyunahn-intel requested a review from a team as a code owner July 25, 2022 21:18
@dongkyunahn-intel
Copy link
Contributor Author

Note : DPASW support will be enabled later as it requires updates in esimd_emulator backend interface as well as CM_EMU library for runtime support

v-klochkov
v-klochkov previously approved these changes Jul 26, 2022
@v-klochkov v-klochkov dismissed their stale review July 26, 2022 03:48

The patch caused compilation fail at esimd/dpas.cpp. Need additional changes.

@bader bader changed the title [SYCL][ESIMD][EMU] Enabling dpas with ESIMD_EMULATOR backend [SYCL][ESIMD][EMU] Enable dpas with ESIMD_EMULATOR backend Jul 26, 2022
- First argument for '__esimd_dpas_inner' is changed from 'const
__ESIMD_DNS::vector_type_t<RT, SZ>' to 'const void *'

- This is because 'std::addressof()' from caller generates 'short *
__attribute__((ext_vector_type(16)))' that cannot be converted into
'vector_type_t<>' while 'dpas.cpp' test is compiled.
- As template and runtime arguments are different (e.g. 'dpas_info' is
not used in ESIMD_EMULATOR backend (hostmode))
@pvchupin
Copy link
Contributor

Thanks @dongkyunahn-intel, do you know anything about SYCL :: unused_spec_const.cpp ?

@dongkyunahn-intel
Copy link
Contributor Author

Thanks @dongkyunahn-intel, do you know anything about SYCL :: unused_spec_const.cpp ?

@pvchupin , as far as I know 'spec_const' tests are disabled for ESIMD_EMULATOR backend as they are failing with Unsupported 'online_compiler' error. It could be because of 'AOT' that is not supported by the ESIMD_EMULATOR backend. Currently, there is no plan for enabling those tests with the backend.

- Template arguments for dpas operations (repeat count, systolic
depth, src1_precision, src2_precision) are converted into runtime
arguments

- __esimd_dpas* functions have same signature regardless of
__SYCL_DEVICE_ONLY__
@dongkyunahn-intel
Copy link
Contributor Author

Unrelated failure :

SYCL / Linux / CUDA LLVM Test Suite

@dongkyunahn-intel
Copy link
Contributor Author

Function signatures for __esimd_dpas* will be changed to have precision, systolic depth, and repeat count (from dpas_info) as template arguments. Host implementation will be changed in this PR. Device implementation will be changed later.

- For precision, repeat, depth, and signedness info, runtime function
arguments are changed to template argument for host mode compilation

- dpas_info encoding/decoding, ARG_UNUSED removed as they are no
longer used

- TODO : Refactor intrinsic declarations for device mode
__ESIMD_UNSUPPORTED_ON_HOST;
return __ESIMD_DNS::vector_type_t<T, N>();
#endif // __SYCL_EXPLICIT_SIMD_PLUGIN__
(__ESIMD_DNS::vector_type_t<T, N> *)std::addressof(src0), src1, src2);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This cast seems very suspicious

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is removed. dpas.cpp now fails as follows:

/export/users/dongkyun/github/opensource/intel_llvm/build/bin/../include/sycl/ext/intel/experimental/esimd/detail/math_intrin.hpp:473:1: note: candidate function template not viable: no known conversion from 'short * __attribute__((ext_vector_type(16)))' to 'const sycl::ext::intel::esimd::detail::vector_type_t<float, 16U> *' (aka 'const cl::sycl::ext::intel::esimd::detail::raw_vector_type<float, 16>::type *') for 1st argument
__esimd_dpas_inner(const __ESIMD_DNS::vector_type_t<RT, SZ> *src0,

#ifdef __SYCL_DEVICE_ONLY__

// TODO: __esimd_dpas* should have single declaration for host and device:
// void __esimd_dpas*(...)
Copy link
Contributor

@kbobrovs kbobrovs Aug 12, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is not void actually Ret __esimd_dpas*(...). // void __esimd_dpas*(...) can be simply removed if complete pseudo-code skeleton of the definition is not shown.

@@ -662,15 +671,13 @@ template <__ESIMD_ENS::argument_type src1_precision,
inline __ESIMD_DNS::vector_type_t<T, N>
__esimd_dpas(__ESIMD_DNS::vector_type_t<T0, N> src0,
__ESIMD_DNS::vector_type_t<T1, N1> src1,
__ESIMD_DNS::vector_type_t<T2, N2> src2) {
#ifdef __SYCL_EXPLICIT_SIMD_PLUGIN__
__ESIMD_DNS::vector_type_t<T2, N2> src2, int sign_res,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please remove the two unused arguments

T, T0, T1, T2, N, N1, N2>(src0.data(), src1.data(),
src2.data());
T, T0, T1, T2, N, N1, N2>(
src0.data(), src1.data(), src2.data(), dst_signed, src0_signed);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please remove this change as well

@dongkyunahn-intel
Copy link
Contributor Author

dongkyunahn-intel commented Aug 15, 2022

Unrelated failures

SYCL / Linux / HIP AMDGPU LLVM Test Suite 
- SYCL :: DeviceLib/imf_simd_emulate_test.cpp

SYCL / Linux / CUDA LLVM Test Suite
- SYCL :: DeviceLib/imf_simd_emulate_test.cpp

@kbobrovs
Copy link
Contributor

Unrelated failures

What are the list of failing tests per each configuration?

@dongkyunahn-intel
Copy link
Contributor Author

Unrelated failures

What are the list of failing tests per each configuration?

Updated the comment with failing tests.

@kbobrovs kbobrovs merged commit 3d506a3 into intel:sycl Aug 16, 2022
dongkyunahn-intel added a commit to dongkyunahn-intel/llvm-test-suite that referenced this pull request Aug 16, 2022
- Bringing back of PR#1110 as all dpas tests are passing after PR -
intel/llvm#6475 is merged in intel/llvm:sycl
kbobrovs pushed a commit to intel/llvm-test-suite that referenced this pull request Aug 17, 2022
- Bringing back of PR#1110 as all dpas tests are passing after PR -
intel/llvm#6475 is merged in intel/llvm:sycl
aelovikov-intel pushed a commit to aelovikov-intel/llvm that referenced this pull request Mar 27, 2023
…te#1151)

- Bringing back of PR#1110 as all dpas tests are passing after PR -
intel#6475 is merged in intel/llvm:sycl
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants