Skip to content

[SYCL][NFC] Refactor common helpers #19044

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

Open
wants to merge 1 commit into
base: sycl
Choose a base branch
from

Conversation

AlexeySachkov
Copy link
Contributor

  • Dropped barely used __SYCL_ASSERT macro
  • Moved __SYCL_UR_ERROR_REPORT macro and corresponding helper function to library
  • Moved helpers thare are used in a single header to that header to reduce their impact on other headers
  • Moved some helpers into the library

- Dropped barely used `__SYCL_ASSERT` macro
- Moved `__SYCL_UR_ERROR_REPORT` macro and corresponding helper function
  to library
- Moved helpers thare are used in a single header to that header to
  reduce their impact on other headers
Comment on lines +116 to +117


Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change

#include <sycl/detail/common.hpp> // for NDLoop, __SYCL_ASSERT
#include <sycl/__spirv/spirv_types.hpp> // for Scope, __ocl_event_t
#include <sycl/access/access.hpp> // for decorated, mode, addr...
#include <sycl/detail/common.hpp>
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this #include used by group.hpp?

@@ -59,6 +59,29 @@ class custom_selector : public device_selector {
}
};

inline std::string_view get_backend_name_no_vendor(backend Backend) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the idea of having this function in DPC++ headers is to make sure that sycl-ls prints the same values as DPC++ runtime uses for the env. variables. Now, we need to make sure that they are in sync. How do we do that?
Can we move this function to separate "public" header, which is not included by sycl.hpp, but included by DPC++ runtime and sycl-ls tool?

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