Skip to content

[ESIMD] Implement the new non-experimental low-level API for DPAS #6834

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 2 commits into from
Sep 22, 2022

Conversation

v-klochkov
Copy link
Contributor

@v-klochkov v-klochkov commented Sep 21, 2022

The new DPAS API are added to the new esimd::xmx (Xe Matrix eXtension)
namespace.
The old/experimental DPAS API is marked as deprecated and now it
simply calls the new DPAS API.
The DPAS emulation sequences has got the automatic detection of
the execution size instead of being defined through the macro ESIMD_XE_HPC.

Signed-off-by: Vyacheslav N Klochkov [email protected]

@v-klochkov v-klochkov force-pushed the dpas_new_low_level_api branch 5 times, most recently from 08028cb to 96ae9bd Compare September 21, 2022 09:11
@v-klochkov v-klochkov changed the title [ESIMD] Implement the initial version of the new low-level API for DPAS [ESIMD] Implement the new non-experimental low-level API for DPAS Sep 21, 2022
@v-klochkov v-klochkov marked this pull request as ready for review September 21, 2022 09:16
@v-klochkov v-klochkov requested a review from a team as a code owner September 21, 2022 09:16
Comment on lines 286 to 287
int N, int BN, int AN>
__ESIMD_NS::simd<T, N> dpasw(__ESIMD_NS::simd<BT, BN> B,
Copy link
Contributor Author

@v-klochkov v-klochkov Sep 21, 2022

Choose a reason for hiding this comment

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

I realized there is a major problem here: This second version of dpasw() (without src matrix C) must not accept the 'N' parameter, but instead must compute it. It must be similar to 2nd version of dpas() on the line 220 above.
I'll fix it shortly.

Suggested change
int N, int BN, int AN>
__ESIMD_NS::simd<T, N> dpasw(__ESIMD_NS::simd<BT, BN> B,
int BN, int AN>
auto dpasw(__ESIMD_NS::simd<BT, BN> B,

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I resolved it in this additional fix: 25db74d

Copy link
Contributor

@kbobrovs kbobrovs left a comment

Choose a reason for hiding this comment

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

LGTM, as we already reviewed the code internally.

But, as a follow-up commit please improve user documentation:

  • API usage example right in the doxygen, as this is extremely complex API
  • allowed element types and how they are modeled, in which cases user must specify precision etc
  • all constraints on the template parameters
    • specialize them by architecture (PVC, ACM)
  • element packing details

The new DPAS API are added to the new esimd::xmx (Xe Matrix eXtension)
namespace.
The old/experimental DPAS API is marked as deprecated and now it
simply calls the new DPAS API.
The DPAS emulation sequences has got the automatic detection of
the execution size instead of being defined through the macro ESIMD_XE_HPC.

Signed-off-by: Vyacheslav N Klochkov <[email protected]>
@v-klochkov v-klochkov force-pushed the dpas_new_low_level_api branch from c4b194f to 25db74d Compare September 21, 2022 22:30
@v-klochkov
Copy link
Contributor Author

This fix doesn't need to wait for the corresponding tests-PR CI here (intel/llvm-test-suite#1281)
because #1281 only adds 2 new tests that are not started in precommit CI anyways (already reported as UNSUPPORTED there).
Merging now.

@v-klochkov v-klochkov merged commit 55bf1a0 into intel:sycl Sep 22, 2022
@v-klochkov v-klochkov deleted the dpas_new_low_level_api branch September 22, 2022 02:01
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.

2 participants