-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[IRGen] Fix misalignment of conditional invertible requirement counts #73277
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
Conversation
@swift-ci please smoke test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice find
@swift-ci please smoke test Windows |
1 similar comment
@swift-ci please smoke test Windows |
@swift-ci please smoke test |
5209398
to
2a2d68f
Compare
@swift-ci please smoke test |
2a2d68f
to
517c05a
Compare
@swift-ci please smoke test |
add include Define a popcount util Update MathUtils.h
517c05a
to
852ef0b
Compare
@swift-ci please smoke test |
@Azoy I haven't precisely reproduced but this PR might have caused this build error on windows ARM64 (it is fine on X86_64):
It seems like I think we could use this from llvm rather than rolling our own:
|
@hjyamauchi that header isn't available to the runtime unfortunately. We can just manually define our own popcount like I did here: #73384 |
@Azoy Alejandro, gotcha. Thanks for the fix! |
[IRGen] Fix misalignment of conditional invertible requirement counts
The conditional invertible protocol set is alone as a 16 bit slot in the generic context, but items here are expected to be 4 bytes large, so some padding needs to accompany this value. If the number of invertible protocols that we're conditional on is even, we can stick a 0 after all of the requirement counts. Also, we were using
countBitsUsed
to determine how many requirement counts there were, but this is really testing the first bit in [1, 64] that is turned on instead of actually saying how many bits are turned on.