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

dev(ustring.h): string literal operator for ustring and ustringhash #3939

Merged
merged 2 commits into from
Aug 21, 2023

Conversation

lgritz
Copy link
Collaborator

@lgritz lgritz commented Aug 3, 2023

  • Add string literal operators for ustring and ustringhash so you can write: "foo"_us "bar"_ush Note that on Cuda (but not on the host), this ustringhash construction is constexpr.

  • Add a new ustringhash(const char*, size_t) constructor, which is also constexpr on the device side.

  • Mark the existing ustringhash(string_view) constructor as constexpr on the device side.

We can mark those things as constexpr on the device side because on the Cuda side, the implementation only takes the hash (which is constexpr), and does not actually add a ustring to the table (which cannot be constexpr). To make such declarations easy, add a new macro to platform.h, OIIO_DEVICE_CONSTEXPR which is constexpr for the device, merely inline for the host.

Copy link
Contributor

@jessey-git jessey-git left a comment

Choose a reason for hiding this comment

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

Seems straightforward and LGTM.

@@ -815,23 +815,32 @@ class OIIO_UTIL_API ustringhash {
}

/// Construct a ustringhash from a null-terminated C string (char *).
OIIO_HOSTDEVICE explicit ustringhash(const char* str)
OIIO_DEVICE_CONSTEXPR explicit ustringhash(const char* str)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe add a comment on why the CPU side can't be constexpr.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

Copy link
Collaborator

@AlexMWells AlexMWells left a comment

Choose a reason for hiding this comment

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

LGTM, like the string literals.

* Add string literal operators for ustring and ustringhash so you can
  write:
  ```
      "foo"_us
      "bar"_ush
  ```
  Note that on Cuda (but not on the host), this ustringhash construction
  is constexpr.

* Add a new `ustringhash(const char*, size_t)` constructor, which is also
  constexpr on the device side.

* Mark the existing `ustringhash(string_view)` constructor as constexpr
  on the device side.

We can mark those things as constexpr on the device side because on
the Cuda side, the implementation only takes the hash (which is
constexpr), and does not actually add a ustring to the table (which
cannot be constexpr). To make such declarations easy, add a new macro
to platform.h, `OIIO_DEVICE_CONSTEXPR` which is constexpr for the
device, merely inline for the host.

Signed-off-by: Larry Gritz <lg@larrygritz.com>
Signed-off-by: Larry Gritz <lg@larrygritz.com>
@lgritz lgritz merged commit 629b205 into AcademySoftwareFoundation:master Aug 21, 2023
22 of 23 checks passed
@lgritz lgritz deleted the lg-ustringliteral branch August 21, 2023 01:13
lgritz added a commit to lgritz/OpenImageIO that referenced this pull request Aug 25, 2023
…cademySoftwareFoundation#3939)

* Add string literal operators for ustring and ustringhash so you can
write: ``` "foo"_us "bar"_ush ``` Note that on Cuda (but not on the
host), this ustringhash construction is constexpr.

* Add a new `ustringhash(const char*, size_t)` constructor, which is
also constexpr on the device side.

* Mark the existing `ustringhash(string_view)` constructor as constexpr
on the device side.

We can mark those things as constexpr on the device side because on the
Cuda side, the implementation only takes the hash (which is constexpr),
and does not actually add a ustring to the table (which cannot be
constexpr). To make such declarations easy, add a new macro to
platform.h, `OIIO_DEVICE_CONSTEXPR` which is constexpr for the device,
merely inline for the host.

---------

Signed-off-by: Larry Gritz <lg@larrygritz.com>
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