-
Notifications
You must be signed in to change notification settings - Fork 285
Provide cuda::ptr_in_range
#6086
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
Conversation
|
next step is to aggregate memory/pointer utilities in |
This comment has been minimized.
This comment has been minimized.
|
@davebayer @miscco open question, we don't do anything special in |
|
I was also tempted to optimize the function in the same way of |
|
pre-commit.ci autofix |
|
/ok to test 829f9e0 |
This comment has been minimized.
This comment has been minimized.
|
/ok to test a506e84 |
This comment has been minimized.
This comment has been minimized.
|
I got scared enough by https://quuxplusone.github.io/blog/2019/01/20/std-less-nightmare/ to use a safer version of the function. |
| [[nodiscard]] _CCCL_API bool __ptr_in_range_host(_Tp* __ptr, _Tp* __start, _Tp* __end) noexcept | ||
| { | ||
| _CCCL_ASSERT(::std::greater_equal<>{}(__end, __start), "__ptr_in_range_host: __end must be greater than __start"); | ||
| return ::std::greater_equal<>{}(__ptr, __start) && ::std::less<>{}(__ptr, __end); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just out of curiosity, is our common practice to use ::std::* within HOST_COMPILATION instead of universally relying on ::cuda::std::less (in this case) to do the host/device dispatching?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this case we really rely on a special blessing of std::greater_equal etc. in the C++ standard that allows pointers to be compared and establishes a total order. operator< on raw pointers only yields a partial order, which the compiler abuses in some situations. E.g.:
int a[10];
int b[10];
int* p = a +5;
if (a < p && p < a + 10) // this is legal
if (a < p && p < b) // this is NOT legal, pointers from different arrays cannot be compared. invokes UB
if (std::less{}(a, p) && std::less{}(p, b)) // this IS legal, pointer addresses are numerically comparedDon't ask me why :D I really don't know.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here is the wording: https://eel.is/c++draft/comparisons#general-2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I read about it before reviewing here. My questions was why we don't use cuda::std::* instead of just std::. Not why we do not use the < operator. But I guess you answered by saying "special blessing of std::greater_equal" which kinda implies that the cuda::std counterpart does not partake of that blessing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, correct. The host compiler recognizes std::greater_equal but not cuda::std::greater_equal
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just:
Co-authored-by: Giannis Gonidelis <[email protected]>
|
/ok to test ddd33e5 |
|
/ok to test d164dd5 |
This comment has been minimized.
This comment has been minimized.
🥳 CI Workflow Results🟩 Finished in 3h 43m: Pass: 100%/242 | Total: 3d 02h | Max: 2h 00m | Hits: 97%/304639See results here. |
Description
Checking if a pointer is in a given range is very common. The utility has already been proposed in WG21 P3234. It is already provided by Boost Core and part of libc++/libstdc++ as internal function.