-
Notifications
You must be signed in to change notification settings - Fork 3.5k
fix: Big-endian support does not work with optimizations #25797
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
base: main
Are you sure you want to change the base?
fix: Big-endian support does not work with optimizations #25797
Conversation
ce7bfd0 to
156bee8
Compare
|
Sorry if I've asked you this already, but what is you use case for bigendian suport @slavek-kucera ? Are you running in production on actual BE machines? |
|
Yes, the output targets s390x running z/os. |
|
As for testing, I guess we could add bigendian3 test suite (assuming @juj still runs it in the private pipeline). |
I totally do. (e.g. http://clbri.com:8010/#/builders/11/builds/1275 - ignore lots of red for other failures, but there's a passing Posted PR #25820 to complete the testing matrix. After that lands, I can add the remaining bigendian suites onto the test runner list. |
What engine are you running the emscripten output on? i.e. Are these servers running Nodejs I guess? |
|
Yes, the engine is nodejs. |
Add the rest of the bigendian test matrix to help test #25797
|
@slavek-kucera My Linux box now completed run of all the bigendian suites. It looks like there are some failures in -O2 and -Os modes: http://clbri.com:8010/#/builders/11/builds/1278 |
|
I think those are all false positive build failures due to closure compiler. Maybe the function needs an annotation just like |
156bee8 to
c0ade81
Compare
While migrating to 4.0.19 I discovered that enabling optimizations removes
ParenthesizedExpressionnode from AST causing thelittleEndianHeapto not perform the transform correctly.