Skip to content
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

[SYCL] Add basic support for the generic_space address space #5148

Merged
merged 5 commits into from Dec 22, 2021
Merged

[SYCL] Add basic support for the generic_space address space #5148

merged 5 commits into from Dec 22, 2021

Conversation

ghost
Copy link

@ghost ghost commented Dec 15, 2021

The basic support is added to let the sycl::atomic_ref class template
be instantiated with the address_space::generic_space address space.

@ghost ghost requested a review from vladimirlaz December 15, 2021 09:25
@ghost
Copy link
Author

ghost commented Dec 15, 2021

I'm working on a patch for the llvm-test-suite project to test sycl::atomic_refs with generic_space as well.

@vladimirlaz
Copy link
Contributor

/summary:run

@zjin-lcf
Copy link
Contributor

The basic support is added to let the sycl::atomic_ref class template be instantiated with the address_space::generic_space address space.

I have a question. When should generic_space be used in an example/kernel ? Thanks

@ghost
Copy link
Author

ghost commented Dec 16, 2021

@zjin-lcf I've found only the sentences about why generic address space can be not the best choice from the performance and linking point of view in the beginning of Section 4.7.7 Address space classes in the SYCL 2020 specification. I hope @bader or @gmlueck can answer more detailed.

@Naghasan
Copy link
Contributor

When should generic_space be used in an example/kernel ?

it was added for completeness rather than actual usefulness. Without you would force users to specialize for raw pointers.

that accounts for that kind of use case:

template<access::address_space Space>
void bar(multi_ptr<int, Space>);

void foo(int* p) {
  // without access::address_space::generic_space, you need to specialize bar or
  // try to find out to which address space p is pointing to.
  bar<access::address_space::generic_space>(p); 
}

@zjin-lcf
Copy link
Contributor

Thank you for the example and comments.

@vladimirlaz
Copy link
Contributor

/summary:run

1 similar comment
@ghost
Copy link
Author

ghost commented Dec 17, 2021

/summary:run

Pavel Samolysov added 5 commits December 20, 2021 10:33
@ghost
Copy link
Author

ghost commented Dec 21, 2021

Please have a look at PR intel/llvm-test-suite#619, the tests against this PR failed but not in the AtomicRef directory.

@ghost ghost marked this pull request as ready for review December 21, 2021 16:35
@ghost ghost self-requested a review as a code owner December 21, 2021 16:35
@bader bader merged commit e99f298 into intel:sycl Dec 22, 2021
alexbatashev added a commit to alexbatashev/llvm that referenced this pull request Dec 23, 2021
* upstream/sycl: (1382 commits)
  [SYCL][XPTI] Report memory allocation info from SYCL runtime (intel#5172)
  [CI] Switch labels for OCL x64 job (intel#5185)
  [SYCL] Add basic support for the generic_space address space (intel#5148)
  [CI] Update CODEOWNERS for SYCL printf support passes (intel#5199)
  [SYCL][Matrix] Enable wi_slice for joint_matrix (intel#4979)
  [SYCL][Group algorithms] Move group sort extension to experimental (intel#5169)
  [SYCL] Fix kernel bundles don't really carry kernel IDs (intel#5121)
  [SYCL] Initial printf support for non-constant AS format strings (intel#5069)
  [SYCL][NFC] Fix static code analysis concerns (intel#5189)
  [SYCL][Doc] Fix typos to fix doc build (intel#5190)
  [Driver][SYCL] Turn on -fsycl-dead-args-optimization by default (intel#3004)
  [SYCL][L0][Plugin] Add support for batching copy commands (intel#5155)
  [CI] Add cache checkout script to docker containers (intel#5184)
  [SYCL][Doc] Add HIP backend to the filter selector (intel#5176)
  [Doc] Add documentation for Docker images (intel#4778)
  [LIBCLC] Add functionality for in-kernel asserts for CUDA backend (intel#5174)
  Force opt to use new pass manager in exponential-deferred-inlining test after a8c2ba1
  [SYCL] Add vec and marray support to known_identity type trait (intel#5163)
  Correctly resolve merge conflicts
  Update SPV_INTEL_hw_thread_queries to rev 2
  ...
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.

None yet

4 participants