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

Remove -fno-exceptions #832

Merged
merged 2 commits into from
Mar 27, 2025

Conversation

AvivBenchorin
Copy link
Contributor

Removes the -fno-exceptions C++ compiler flags. NCCL currently does not build with the flag, so removing it here should not have a performance impact and will make it easier to handle errors from constructors/deconstructors and STL libraries.

Also removes checking for NULL after allocating with new. Since (std::nothrow) was not being used to specify non-throwing allocators, errors in the constructor would just throw an exception rather than return NULL, making the NULL checks unnecessary. With -fno-exceptions removed, the exception will now be propagated up the stack.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Remove the -fno-exceptions C++ compiler flags.
NCCL currently does not build with the flag,
so removing it here should not have a performance
impact and will make it easier to handle errors
from constructors/deconstructors and
STL libraries.

Signed-off-by: Aviv Benchorin <benchori@amazon.com>
Removes checking for NULL after allocating with
`new`. Since `(std::nothrow)` was not used to
specify non-throwing allocators, errors in the
constructor would just throw an exception rather
than return NULL, making the NULL checks
unnecessary.

Signed-off-by: Aviv Benchorin <benchori@amazon.com>
Copy link
Contributor

@bwbarrett bwbarrett left a comment

Choose a reason for hiding this comment

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

NIT: for the body of commit messages, you get 72 characters. You're word-wrapping pretty aggressively.

@bwbarrett bwbarrett merged commit 53911e2 into aws:master Mar 27, 2025
23 checks passed
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