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

Add a DEAL_II_ASSUME macro. #16393

Closed
wants to merge 1 commit into from
Closed

Add a DEAL_II_ASSUME macro. #16393

wants to merge 1 commit into from

Conversation

bangerth
Copy link
Member

C++23 has a nice feature, [[assume(expr)]] that allows giving compilers hints about invariants that must be satisfied at a specific point -- see https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2022/p1774r8.pdf. I wanted to see whether we can perhaps use that in the "NO-DEBUG" definition of Assert to give the compiler a hint, but it turns out that I do not have access to a compiler that supports any of the various incompatible ways compilers have provided this sort of feature pre-C++23.

Let's see what the testers have to say. I would be curious to see what happens if anyone looks at the release mode library size without and with this patch, with a compiler that supports any of this.

(Separate question: We could of course also add the assumption after the if in the DEBUG version of the Assert macro. Opinions?)

@masterleinad
Copy link
Member

(Separate question: We could of course also add the assumption after the if in the DEBUG version of the Assert macro. Opinions?)

In that case, the compiler might just compile the assert away, right?

@bangerth
Copy link
Member Author

bangerth commented Jan 1, 2024

That's why I was only suggesting to do it in release mode where the assertion isn't there to begin with. But even in debug mode, I believe that such an assumption would only hold downstream, not upstream, or would it? If not, we'd place a call to a function the compiler can't look through (a barrier) between the assertion and the assume statement, to make sure the compiler can't know anything about upstream because it could have been changed in the opaque call.

@masterleinad
Copy link
Member

In my opinion, it mostly makes sense in Release mode and I would rather not risk that some compiler would just discard the assert.

@masterleinad
Copy link
Member

In terms of library size, I am seeing with clang:
new: 129246112
old: 129870368

@bangerth
Copy link
Member Author

bangerth commented Jan 8, 2024

Oh, interesting! It's only half a percent, but if that translates to half a percent performance gain, then that ain't bad :-)

@bangerth
Copy link
Member Author

This is now superseded by #16437.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants