-
Notifications
You must be signed in to change notification settings - Fork 469
Prefer aligned_alloc over posix_memalign #805
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
6a0827a
to
e8c2f76
Compare
@compnerd Can we please do a test? |
Should we not have a fallback path here? |
are we not compiling with c11? |
There is no guarantee that the libc is going to be complete (e.g. Windows). Yes, I know that there is a separate Windows path, but this is already a difficult library to port, lets not complicate it further. |
Windows is literally the only libc with this issue. I just checked. |
That's literally the point of this PR. And let's not assume that the code already assumes you are compiling for C11, as there are C99 and C11 features in this codebase like 0 length arrays. |
Also, the PR I did for swift literally only checks for windows, and then the STDC version, not a complete libc. I don't see why we would need to find the one libc that doesn't have this but somehow has posix_memalign |
73817e0
to
d325a26
Compare
aligned_alloc is more cleaner and portable.
@swift-ci please test |
@swift-ci please test Windows platform |
FYI for anyone else running into this: this change makes libdispatch require a minimum Android SDK version of 28 (Android 9 Pie) as noted here, because earlier versions don’t have Building with an earlier
Is there any chance to add an Android target to CI to be able to spot things like this sooner? |
I will look into it. In the meantime, I have submitted a fallback, #811. |
Thank you @finagolfin! 🙏 |
aligned_alloc is more cleaner and portable.