-
Notifications
You must be signed in to change notification settings - Fork 3.4k
[WiP] Initial changes to support wasm64 in emcc #12658
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
Thank you for submitting a pull request! If this is your first PR, make sure to add yourself to AUTHORS. |
Wow! Is this really your first emscripten change! How exciting for you :) |
This is unfinished, but useful to maybe have a quick look to see what I am all doing wrong ;) In particular, the libc changes probably shouldn't land as-is, but they indicate what was required to make this work. |
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.
Looks mostly great!
What do we think about that name "memory64" vs "wasm64" (which is used in the llvm world)?
@sbc100 in the future, we're going to have a world where a single Wasm module can potentially access both a 32-bit and 64-bit memory at the same time, and load/store operations just match the memory they access. Thus, from a Wasm perspective, this is a memory property, not an "architecture". We have already used the term "memory64" thru-out the WABT and Binaryen implementations. To verify wether a load/store is correct, these tools look up the associated memory. Of course, LLVM doesn't understand such a concept, and thinks in terms of architectures that hold for the entire program, so there "wasm64" is more appropriate. I personally like this term better also, since 99% of programmers are not going to understand the subtleties around Wasm memories. But it is a useful distinction for us. |
sgtm! |
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.
Exciting stuff!
a0b3f68
to
4e65d88
Compare
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.
lgtm minus minor comments.
1151c37
to
329cb70
Compare
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.
lgtm % a couple more comments!
No description provided.