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

Fixes Heap Allocations for Large alignMask #238

Closed
wants to merge 5 commits into from
Closed

Fixes Heap Allocations for Large alignMask #238

wants to merge 5 commits into from

Conversation

manavgabhawala
Copy link
Contributor

Uses the posix_memalign C function from the stdlib instead of malloc so that in case the alignMask is larger than what the system (or malloc) supports (on OS X malloc aligns memory on 16 byte boundaries so for larger values) we do not get unnecessary crashes.

Uses the aligned_alloc C function from the stdlib instead of malloc so that incase the alignMask is larger than what the system supports we do not get unnecessary crashes.
@gparker42
Copy link
Contributor

The alignMask used by swift::slowAlloc() and the alignment used by posix_memalign() are not compatible. You need to convert them. (I think alignment = alignMask+1 works.)

Calling posix_memalign() for every allocation may be slow. You should measure performance. You may need to call malloc directly when alignMask is within the system's guaranteed alignment.

@manavgabhawala
Copy link
Contributor Author

I could not find anything that showed that swift::slowAlloc() requires an alignment mask + 1 maybe I am missing something, but that should be an easy enough fix. However, we should make sure this is true because posix_memalign requires the alignMask to be a power of 2, or else will fail.

You are right, posix_memalign incurs a significant overhead in order to align memory, so we can use it only if the alignMask exceeds the system's guaranteed alignment. However, after a fair amount of research I've found that on Darwin, malloc promises a 16 byte alignment whereas on other posix systems the glibc only promises 8 byte alignment. There probably is a clean way to work around the platform dependent system size, any suggestions?

@jckarter
Copy link
Contributor

jckarter commented Dec 5, 2015

The compiler could emit more specific function calls when it knows the alignment needed for an allocation, and only fall back to slowAlloc when the alignment is dynamic. That's how @davezarzycki's original allocator entry points were planned to work.

Fixes a segmentation fault where the IRGen module would crash when a
generic (empty) struct was part of the source code because it tried to
call std::next on an end iterator.
dabelknap pushed a commit to dabelknap/swift that referenced this pull request May 3, 2019
Changes needed to build/test on Linux.
freak4pc pushed a commit to freak4pc/swift that referenced this pull request Sep 28, 2022
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