Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Arbitrary alignment #32

Merged
merged 5 commits into from
Mar 6, 2018
Merged

Arbitrary alignment #32

merged 5 commits into from
Mar 6, 2018

Conversation

fitzgen
Copy link
Member

@fitzgen fitzgen commented Mar 5, 2018

This fixes #29; see each commit for details.

fitzgen added 4 commits March 2, 2018 10:28
This threads the requested alignment through all the allocation functions, and
makes them take the requested alignment into account.

Size classes are not used for alignments greater than the size of a word, since
that means we wouldn't be able to pack their memory like `[header, data, header,
data, ...]`. Instead, we would need to introduce padding like `[header, padding,
data, header, padding, data, ...]`.

Fixes #29
This should help exercise the alignment code paths.
The CI issues we have been hitting are fixed in binaryen master.
@fitzgen fitzgen requested a review from pepyakin March 5, 2018 23:39
.travis.yml Outdated
@@ -25,7 +25,7 @@ before_script:
- which cargo-readme || cargo install cargo-readme
- test -d binaryen || git clone https://github.com/WebAssembly/binaryen.git
- export PATH="$PATH:$(pwd)/binaryen/bin"
- which wasm-opt || (cd binaryen && cmake . && make -j4 && cd -)
- which wasm-opt || (cd binaryen && git pull origin master && cmake . && make -j4 && cd -)
Copy link
Member Author

Choose a reason for hiding this comment

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

Hrm... this didn't seem to do the trick.

@fitzgen fitzgen mentioned this pull request Mar 5, 2018
@fitzgen fitzgen force-pushed the arbitrary-alignment branch from e4137ff to 1046396 Compare March 6, 2018 00:02
@fitzgen
Copy link
Member Author

fitzgen commented Mar 6, 2018

Ok, just made binaryen optional, and it looks like CI is working again.

Copy link
Member

@pepyakin pepyakin left a comment

Choose a reason for hiding this comment

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

👍 looks great!

@fitzgen
Copy link
Member Author

fitzgen commented Mar 6, 2018

Thanks for review! 😸

@fitzgen fitzgen merged commit 9e3b96b into master Mar 6, 2018
@fitzgen fitzgen deleted the arbitrary-alignment branch March 6, 2018 17:09
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support allocations with alignment greater than a word
2 participants