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

Check object alignment at runtime. #60

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

Conversation

giuliomoro
Copy link
Contributor

This will throw at runtime when the allocator didn't respect the object's alignment. This can only happen with heap allocation when using a standard earlier than c++17 . See discussion in #54

@falkTX
Copy link
Contributor

falkTX commented Aug 9, 2022

if using the code in a plugin context, throwing is quite bad. also impossible to catch and recover if using C (for which hvcc has bindings for afaik)
IMO it would be best to make this deactivate the module with an error of sorts (printing to console?)

@giuliomoro
Copy link
Contributor Author

Fair point, but this check is to detect an alignment issue at allocation time. Operating on misaligned memory would cause a SIGILL to be delivered to your process when later on you try to execute SIMD assembly on unaligned memory. So it's much better - imo - to detect the issue at allocation time. It's C++ convention that a failed object construction should throw, so it should be up to the calling code to look out for this as appropriate. The provided C wrapper is already safe as it is* because it ensures alignment before calling new();

  • at least it guarantees alignment to a 16 byte boundary

@dromer dromer added the help wanted Extra attention is needed label Oct 4, 2022
@dromer dromer added this to Review in Core Improvements Jul 26, 2023
@dromer dromer moved this from Review to Doing in Core Improvements Jul 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants