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

Reduce string duplication from assertion messages #1338

Merged
merged 3 commits into from
Jul 26, 2024

Conversation

sethrj
Copy link
Member

@sethrj sethrj commented Jul 26, 2024

Inspired by @esseivaju 's example in #1325 , this defines external constant strings for "runtime error" types to both reduce library size and to avoid "magic constants" (i.e. requiring that special string values in one file match string values in another).

HIP/frontier (ORANGE ndebug):

Library Before After
libcorecel.a 1613066 1606148
libgeocel.a 902558 898838
liborange.a 4122526 4095566
libceleritas.a 11385000 11295664
libaccel.a 869100 863828

CUDA/wildstyle (ORANGE reldeb):

Library Before After
libcorecel.a 17431736 17415056
libgeocel.a 10777392 10776984
liborange.a 54714576 54682904
libceleritas.a 182853472 181899976
libaccel.a 21292144 21290984

Unfortunately I'm not able to use static inline constexpr for the CUDA/HIP printf statements:

/home/s3j/Code/celeritas/src/corecel/Assert.hh(516): error: An inline __device__/__constant__/__managed__ variable must have internal linkage when the program is compiled in whole program mode (-rdc=false)

see this llvm bug report for details.

@sethrj sethrj added core Software engineering infrastructure minor Minor internal changes or fixes labels Jul 26, 2024
@sethrj sethrj requested a review from esseivaju July 26, 2024 14:35
@esseivaju
Copy link
Contributor

Unfortunately I'm not able to use static inline constexpr for the CUDA/HIP printf statements:

They also mention it here, maybe that will be supported one day

@sethrj sethrj merged commit 1800cc2 into celeritas-project:develop Jul 26, 2024
29 checks passed
@sethrj sethrj deleted the inline-constexpr-char branch July 26, 2024 18:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Software engineering infrastructure minor Minor internal changes or fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants