Skip to content

[SYCL] Follow rule of three in sycl headers #16080

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

Merged
merged 5 commits into from
Nov 21, 2024
Merged

[SYCL] Follow rule of three in sycl headers #16080

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
4b9584b
Follow rule of three in sycl headers
ayylol Nov 13, 2024
570661a
clang format
ayylol Nov 18, 2024
44ae82a
delete copy constructor for `tls_code_loc_t`
ayylol Nov 19, 2024
434ad03
Replace deletion of copy constructor with comment
ayylol Nov 19, 2024
1f9120c
delete copy ctor inside preview breaking changes
ayylol Nov 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions sycl/include/sycl/detail/common.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,13 @@ class __SYCL_EXPORT tls_code_loc_t {
/// @brief Iniitializes TLS with CodeLoc if a TLS entry not present
/// @param CodeLoc The code location information to set up the TLS slot with.
tls_code_loc_t(const detail::code_location &CodeLoc);

#ifdef __INTEL_PREVIEW_BREAKING_CHANGES
// Used to maintain global state (GCodeLocTLS), so we do not want to copy
tls_code_loc_t(const tls_code_loc_t &) = delete;
tls_code_loc_t &operator=(const tls_code_loc_t &) = delete;
#endif // __INTEL_PREVIEW_BREAKING_CHANGES

/// If the code location is set up by this instance, reset it.
~tls_code_loc_t();
/// @brief Query the information in the TLS slot
Expand Down
2 changes: 2 additions & 0 deletions sycl/include/sycl/detail/util.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ template <typename T> struct TempAssignGuard {
TempAssignGuard(T &fld, T tempVal) : field(fld), restoreValue(fld) {
field = tempVal;
}
TempAssignGuard(const TempAssignGuard<T> &) = delete;
TempAssignGuard operator=(const TempAssignGuard<T> &) = delete;
Comment on lines +45 to +46
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Whole point of this class is to use the constructor to temporary assign a value. Since the copy assignment/copy constructor cant take an extra assignment for the temp value to assign these functions are not useful.

~TempAssignGuard() { field = restoreValue; }
};

Expand Down
4 changes: 2 additions & 2 deletions sycl/include/sycl/ext/intel/experimental/pipes.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@ namespace experimental {
class pipe_base {

protected:
pipe_base();
~pipe_base();
pipe_base() = default;
~pipe_base() = default;
Comment on lines +46 to +47
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Exists so derived classes can instantiate pipe_base, so just marking as default explicitly.


__SYCL_EXPORT static std::string get_pipe_name(const void *HostPipePtr);
__SYCL_EXPORT static bool wait_non_blocking(const event &E);
Expand Down
3 changes: 3 additions & 0 deletions sycl/include/sycl/ext/oneapi/bindless_images_memory.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@ class image_mem_impl {
const context &syclContext);
__SYCL_EXPORT ~image_mem_impl();

image_mem_impl(const image_mem_impl &) = delete;
image_mem_impl &operator=(const image_mem_impl &) = delete;

raw_handle_type get_handle() const { return handle; }
const image_descriptor &get_descriptor() const { return descriptor; }
sycl::device get_device() const { return syclDevice; }
Expand Down
1 change: 1 addition & 0 deletions sycl/include/sycl/ext/oneapi/experimental/cuda/barrier.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ class barrier {
barrier(barrier &&other) noexcept = delete;
barrier &operator=(const barrier &other) = delete;
barrier &operator=(barrier &&other) noexcept = delete;
~barrier() = default;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this class only has a state member variable, so the default destructor should be fine.


void initialize(uint32_t expected_count) {
#ifdef __SYCL_DEVICE_ONLY__
Expand Down
Loading