Skip to content

[SYCL][ESIMD] Add supported versions of raw_send APIs #11333

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 29, 2023

Conversation

sarnex
Copy link
Contributor

@sarnex sarnex commented Sep 27, 2023

Some design decisions I made here:

  1. Only add the versions with must-be-constant arguments as template parameters. The original versions had them as runtime parameters and it worked some times, but if the compiler can't optimize it, it will fail. It seems sane to me to only add the versions which are guaranteed to be correct.

  2. Replace eot/sendc arguments with enums, we discussed this one and is hopefully not controversial

  3. Move the intrinsics to supported and call the supported intrinsics in the experiemntal code just to prevent code duplication

Also, the histrogram_raw_send test doesn't even compile in HEAD, we didn't catch it because it requires gen9 and we don't test that anymore. It was doing something insane with execSize and once we made mask len == exec size it broke. They aren't even using the mask so just remove it to fix the test.

Manually verified both modified tests pass.

@sarnex sarnex temporarily deployed to WindowsCILock September 27, 2023 20:51 — with GitHub Actions Inactive
@sarnex sarnex temporarily deployed to WindowsCILock September 27, 2023 21:13 — with GitHub Actions Inactive
@sarnex sarnex marked this pull request as ready for review September 28, 2023 15:42
@sarnex sarnex requested a review from a team as a code owner September 28, 2023 15:42
@v-klochkov
Copy link
Contributor

We have the commend defining

/// @defgroup sycl_esimd_raw_send Raw send APIs.
/// Implements the \c send instruction to send messages to variaous components
/// of the Intel(R) processor graphics, as defined in the documentation at
/// https://01.org/sites/default/files/documentation/intel-gfx-prm-osrc-icllp-vol02a-commandreference-instructions_2.pdf

The reference there is obsolete/broke.
Can you please update it with something like:

  /// Click "Volume 2a: Command Reference: Instructions" at 
  /// https://www.intel.com/content/www/us/en/docs/graphics-for-linux/developer-reference/1-0/hardware-specs.html
  /// to get the definition and details of the 'send' instruction.

@v-klochkov
Copy link
Contributor

@sarnex - please add a 'deprecated' message for "new"/constexpr version of raw_send/sends in experimental namespace.

Signed-off-by: Sarnie, Nick <[email protected]>
@sarnex
Copy link
Contributor Author

sarnex commented Sep 28, 2023

@v-klochkov Thanks for the review, all feedback should be addressed in the latest commit

@sarnex sarnex temporarily deployed to WindowsCILock September 28, 2023 19:01 — with GitHub Actions Inactive
@sarnex sarnex temporarily deployed to WindowsCILock September 28, 2023 19:28 — with GitHub Actions Inactive
@sarnex
Copy link
Contributor Author

sarnex commented Sep 29, 2023

@intel/llvm-gatekeepers I think we are good to go on this one, thanks!

@aelovikov-intel aelovikov-intel merged commit a72409e into intel:sycl Sep 29, 2023
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.

3 participants