-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
Big Endian Support (using extra acorn AST pass) #13413
Conversation
Thank you for submitting a pull request! If this is your first PR, make sure to add yourself to AUTHORS. |
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.
In general the direction looks good!
Please add a test for this. See for example how unsignPointers
is tested in tests/test_other.py
.
5f795bc
to
841e180
Compare
@kripken Please take a look! On LE machine with SUPPORT_BIG_ENDIAN=0 it does not cause any additional test failures (over core tests). On LE machine with SUPPORT_BIG_ENDIAN=1 it breaks a few tests that deal with runtime code size. On BE machine there are still problems, but Emscripten works in most cases (for "wasm0", "wasm1", "wasm2" and "other" test suites approximately 600 out of 700 tests pass). |
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 great!
Looks like all the test failures here are unrelated issues on |
} | ||
case 'HEAPF64': { | ||
// change "name[idx]" to "LE_HEAP_LOAD_F64(idx*8)" | ||
makeCallExpression(node, 'LE_HEAP_LOAD_F64', [multiply(idx, 8)]); |
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.
Nice work overall!
Instead of doing the multiplies here, I would recommend placing the multiplies inside the LE_HEAP_LOAD_
and LE_HEAP_STORE_
functions for a nice code size win. (only one multiply per function versus once for each call) - unless there is some reason they need to be here?
Related to previous discussion about supporting Big Endian architectures:
#12387
#12780