Skip to content

[ESIMD] Fix inconsistencies in the ESIMD API signatures. #4800

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 8 commits into from
Oct 28, 2021
Merged

[ESIMD] Fix inconsistencies in the ESIMD API signatures. #4800

merged 8 commits into from
Oct 28, 2021

Conversation

kbobrovs
Copy link
Contributor

@kbobrovs kbobrovs commented Oct 21, 2021 •
edited
Loading

This is API breaking patch.

  • Remove 'esimd_' prefix from public API names. This prefix is redundant, because all APIs are in ...esimd:: namespace already. API names follow C-for-Metal naming.

  • esimd_sat -> saturate

Memory access API changes:

  • Un-deprecate block_load/store
  • rename slm_atomic -> slm_atomic_update, flat_atomic -> atomic_update. atomic is not used to avoid conflict with C++ atomic class
  • Offset is now expected in bytes(was in elements).
    scatter, gather, scalar_load, scalar_store
  • Make 'offsets' argument preceed the 'val' argument to be consistent with
    other memory APIs.
    scatter, scatter_rgba, slm_scatter
  • Remove unused L1 / L3 CachHint template parameters.
    scatter, gather, scalar_load, scalar_store, scatter_rgba, gather_rgba
  • Rename enum EsimdFenceMask -> enum fence_mask and its elements (deprecate old ones)

Old behavior is preserved in deprecated APIs :

  • scatter->scatter1
  • gather->gather1
  • scalar_load->scalar_load1
  • scalar_store->scalar_store1
  • gather_rgba->gather4
  • scatter_rgba->scatter4
  • slm_scatter->slm_store

Complementary test patch intel/llvm-test-suite#530

Signed-off-by: Konstantin S Bobrovsky konstantin.s.bobrovsky@intel.com

@kbobrovs
[ESIMD] Remove 'esimd_' prefix from public API names.
e5ecf5a 
This prefix is redundant, because all APIs are in ...esimd:: namespace already.
This is API breaking patch.

Signed-off-by: Konstantin S Bobrovsky <konstantin.s.bobrovsky@intel.com>
@kbobrovs kbobrovs mentioned this pull request Oct 21, 2021
Merged
v-klochkov
v-klochkov previously approved these changes Oct 22, 2021
View reviewed changes
@v-klochkov
Copy link
Contributor

Does this comment mean that this patch is actually a DRAFT now?

[More changes will be added here and then committed as a single patch to reduce disruption]

@kbobrovs
Un-deprecate block_load/store, slm_atomic -> slm_atomic_update
59c8511
@kbobrovs
Remove unused CachHint template arguments, deprecate in public APIs.
58563d5
@kbobrovs
Fix offset units, parameter order, more unused CacheHint removed.
4d99f21 
Fixes:
- Offset is now expected in bytes (was in elements).
  scatter, gather, scalar_load, scalar_store

- Make 'offsets' argument preceed the 'val' argument to be consistent with
  other memory APIs.
  scatter, scatter_rgba, slm_scatter

- Remove unused L1/L3 CachHint template parameters.
  scatter, gather, scalar_load, scalar_store, scatter_rgba, gather_rgba

Old behavior is preserved in deprecated APIs:
- scatter -> scatter1
- gather -> gather1
- scalar_load -> scalar_load1
- scalar_store -> scalar_store1
- gather_rgba -> gather4
- scatter_rgba -> scatter4
- slm_scatter -> slm_store

Signed-off-by: Konstantin S Bobrovsky <konstantin.s.bobrovsky@intel.com>
@kbobrovs kbobrovs changed the title [WIP][ESIMD] Fix inconsistencies in the ESIMD API signatures. [ESIMD] Fix inconsistencies in the ESIMD API signatures. Oct 25, 2021
@kbobrovs
Copy link
Contributor Author

Does this comment mean that this patch is actually a DRAFT now?

[More changes will be added here and then committed as a single patch to reduce disruption]

Yes, it was a draft. Now it can be reviewed.

v-klochkov
v-klochkov reviewed Oct 25, 2021
View reviewed changes
@kbobrovs
Copy link
Contributor Author

@v-klochkov, please review. Comments are addressed

@kbobrovs
Copy link
Contributor Author

Jenkins/precommit test failures are expected, and will be fixed by intel/llvm-test-suite#530
(except ESIMD/spec_const/spec_const_bool.cpp, which should be marked as XFAIL, due to known SYCL RT problem with setting 0 spec constant value).

[2021-10-28T00:42:57.251Z] SYCL :: DeprecatedFeatures/ESIMD/spec_const_float.cpp
[2021-10-28T00:42:57.251Z] SYCL :: DeprecatedFeatures/ESIMD/spec_const_int.cpp
[2021-10-28T00:42:57.251Z] SYCL :: DeprecatedFeatures/ESIMD/spec_const_redefine_esimd.cpp
[2021-10-28T00:42:57.251Z] SYCL :: DeprecatedFeatures/ESIMD/spec_const_short.cpp
[2021-10-28T00:42:57.251Z] SYCL :: DeprecatedFeatures/ESIMD/spec_const_uint.cpp
[2021-10-28T00:42:57.251Z] SYCL :: DeprecatedFeatures/ESIMD/spec_const_ushort.cpp
[2021-10-28T00:42:57.251Z] SYCL :: ESIMD/BitonicSortKv2.cpp
[2021-10-28T00:42:57.251Z] SYCL :: ESIMD/PrefixSum.cpp
[2021-10-28T00:42:57.251Z] SYCL :: ESIMD/Prefix_Local_sum1.cpp
[2021-10-28T00:42:57.251Z] SYCL :: ESIMD/Prefix_Local_sum2.cpp
[2021-10-28T00:42:57.251Z] SYCL :: ESIMD/Prefix_Local_sum3.cpp
[2021-10-28T00:42:57.251Z] SYCL :: ESIMD/accessor_gather_scatter.cpp
[2021-10-28T00:42:57.251Z] SYCL :: ESIMD/accessor_load_store.cpp
[2021-10-28T00:42:57.251Z] SYCL :: ESIMD/api/ballot.cpp
[2021-10-28T00:42:57.251Z] SYCL :: ESIMD/api/esimd_bit_ops.cpp
[2021-10-28T00:42:57.251Z] SYCL :: ESIMD/api/slm_gather_scatter.cpp
[2021-10-28T00:42:57.251Z] SYCL :: ESIMD/api/slm_gather_scatter_heavy.cpp
[2021-10-28T00:42:57.251Z] SYCL :: ESIMD/api/slm_gather_scatter_rgba.cpp
[2021-10-28T00:42:57.251Z] SYCL :: ESIMD/ext_math.cpp
[2021-10-28T00:42:57.251Z] SYCL :: ESIMD/histogram_256_slm.cpp
[2021-10-28T00:42:57.251Z] SYCL :: ESIMD/histogram_raw_send.cpp
[2021-10-28T00:42:57.251Z] SYCL :: ESIMD/kmeans/kmeans.cpp
[2021-10-28T00:42:57.251Z] SYCL :: ESIMD/regression/usm_gather_scatter_32.cpp
[2021-10-28T00:42:57.251Z] SYCL :: ESIMD/slm_barrier.cpp
[2021-10-28T00:42:57.251Z] SYCL :: ESIMD/slm_split_barrier.cpp
[2021-10-28T00:42:57.252Z] SYCL :: ESIMD/spec_const/spec_const_bool.cpp
[2021-10-28T00:42:57.252Z] SYCL :: ESIMD/spec_const/spec_const_float.cpp
[2021-10-28T00:42:57.252Z] SYCL :: ESIMD/spec_const/spec_const_int.cpp
[2021-10-28T00:42:57.252Z] SYCL :: ESIMD/spec_const/spec_const_short.cpp
[2021-10-28T00:42:57.252Z] SYCL :: ESIMD/spec_const/spec_const_uint.cpp
[2021-10-28T00:42:57.252Z] SYCL :: ESIMD/spec_const/spec_const_ushort.cpp
[2021-10-28T00:42:57.252Z] SYCL :: ESIMD/usm_gather_scatter_rgba.cpp
[2021-10-28T00:42:57.252Z] SYCL :: ESIMD/vadd_raw_send.cpp

pvchupin pushed a commit to intel/llvm-test-suite that referenced this pull request Oct 28, 2021
@kbobrovs
[ESIMD] Fix tests to reflect APIs changes. (#530)
b351868 
Fix tests to reflect esimd_ prefix removal in APIs.
Complementary patch for intel/llvm#4800.

* remove 'esimd_' in memory intrins, use atomic_update
* Fixes due to memory API changes.
    API changes:
    - Offset is now expected in bytes (was in elements).
      scatter, gather, scalar_load, scalar_store

    - Make 'offsets' argument preceed the 'val' argument to be consistent with
      other memory APIs.
      scatter, scatter_rgba, slm_scatter

    - Remove unused L1/L3 CachHint template parameters.
      scatter, gather, scalar_load, scalar_store, scatter_rgba, gather_rgba

Signed-off-by: Konstantin S Bobrovsky <konstantin.s.bobrovsky@intel.com>
@kbobrovs kbobrovs merged commit 695e2cf into intel:sycl Oct 28, 2021
@kbobrovs kbobrovs deleted the api_signatures_fix branch October 28, 2021 17:27
@kbobrovs kbobrovs mentioned this pull request Oct 29, 2021
Merged
aelovikov-intel pushed a commit to aelovikov-intel/llvm that referenced this pull request Mar 27, 2023
@kbobrovs
[ESIMD] Fix tests to reflect APIs changes. (intel/llvm-test-suite#530)
9ae8aa6 
Fix tests to reflect esimd_ prefix removal in APIs.
Complementary patch for intel#4800.

* remove 'esimd_' in memory intrins, use atomic_update
* Fixes due to memory API changes.
    API changes:
    - Offset is now expected in bytes (was in elements).
      scatter, gather, scalar_load, scalar_store

    - Make 'offsets' argument preceed the 'val' argument to be consistent with
      other memory APIs.
      scatter, scatter_rgba, slm_scatter

    - Remove unused L1/L3 CachHint template parameters.
      scatter, gather, scalar_load, scalar_store, scatter_rgba, gather_rgba

Signed-off-by: Konstantin S Bobrovsky <konstantin.s.bobrovsky@intel.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Pennycook Pennycook Pennycook left review comments

@v-klochkov v-klochkov v-klochkov approved these changes

@kychendev kychendev Awaiting requested review from kychendev

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@kbobrovs @v-klochkov @Pennycook