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 configure logic for choosing how to perform CT value barriers #4447

Merged
merged 1 commit into from
Nov 27, 2024

Conversation

randombit
Copy link
Owner

Related to #4444 and #4445

@randombit randombit requested a review from reneme November 26, 2024 22:00
@coveralls
Copy link

coveralls commented Nov 26, 2024

Coverage Status

coverage: 91.249% (+0.004%) from 91.245%
when pulling e30c68d on jack/config-ct-barrier
into e54502e on master.

Copy link
Collaborator

@reneme reneme left a comment

Choose a reason for hiding this comment

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

Nice! Thanks for setting this up. There's definitely value in allowing to configure this. Also, I can confirm that this is fixing the ICEs reported in #4444.

I feel that a bit more documentation would be good, though. Not necessarily user-facing; perhaps just briefly discussing the idea of each alternative in a doxygen for the value_barrier.

configure.py Outdated Show resolved Hide resolved
configure.py Show resolved Hide resolved
src/build-data/cc/gcc.txt Show resolved Hide resolved
src/lib/utils/ct_utils.h Outdated Show resolved Hide resolved
Also restrict CT::value_barrier to only unsigned integral types.
Any other usage is probably incorrect.

Related to #4444 and #4445
@randombit randombit force-pushed the jack/config-ct-barrier branch from 7e5c5c4 to e30c68d Compare November 27, 2024 15:03
@randombit randombit merged commit 04b5a8d into master Nov 27, 2024
38 checks passed
@randombit randombit deleted the jack/config-ct-barrier branch November 27, 2024 15:48
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