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.

Sign up for GitHub

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

Jump to bottom

Remaining test failures with wasm-backend pthreads #8718

Closed
kripken opened this issue May 31, 2019 · 2 comments · Fixed by #8842
Closed

Remaining test failures with wasm-backend pthreads #8718

kripken opened this issue May 31, 2019 · 2 comments · Fixed by #8842

Comments

@kripken
Copy link
Member

kripken commented May 31, 2019

Some of these look quite worrying, e.g. flakiness on test_pthread_64bit_atomics (happens in chrome and firefox, so not a browser bug).

I'm marking them with TODO - fix final pthreads tests and a link to this issue.
@kripken kripken mentioned this issue May 31, 2019
Merged
kripken added a commit that referenced this issue Jun 4, 2019
@kripken
Enable some pthreads tests on the wasm backend path (#8715)
30eb07a 
Build more system libs with -mt.
Test fixes.
Misc minor JS glue fixes.
Some tests still fail on odd things, I marked them as TODO - fix final pthreads tests,  opened #8718 to track that. There may be some serious issues there.
@kripken
Copy link
Member Author

kripken commented Jun 6, 2019

The wasm backend flakiness on pthreads tests seems to be in tests where we synchronize between threads. It's as if a thread doesn't always see changes that it should. (I wonder if this wasn't noticed earlier because we ignored all tests with fences anyhow, while waiting for that to land.)

@aheejin @dschuff is there a difference between the wasm backend and fastcomp that can explain this? do we emit different synchronization code? The one difference I am aware of is in the emscripten repo's pthread library code,

https://github.com/emscripten-core/emscripten/blob/incoming/system/lib/pthread/library_pthread_asmjs.c

vs

https://github.com/emscripten-core/emscripten/blob/incoming/system/lib/pthread/library_pthread_wasm.c

The wasm backend uses c11 methods. Perhaps we implement them differently in the wasm backend?

This was referenced Jun 17, 2019
Closed
Merged
@kripken
Copy link
Member Author

kripken commented Jun 18, 2019

I checked the atomics we emit for those c11 things with @tlively and we didn't see anything odd.

Meanwhile #8811 has been opened, and it looks like it fixes this! With the establishStackSpace fix there, the problem goes from 20% of runs of the 64-bit atomics test fail to 0%, and I ran many many tests.

@kripken kripken mentioned this issue Jun 21, 2019
Merged
kripken added a commit that referenced this issue Jun 22, 2019
@kripken
Wasm backend pthreads testing improvements (#8842)
16edbe5 
 *   After #8811 landed a lot more tests can pass, this enables all those that can.
 *   Remove unnecessary stuff in test_pthread_gcc_atomic_fetch_and_op.
 *   Add more runtime assertions for the stack position.

There are still a few tests that don't pass, so this doesn't close #8718. But they are quite few at this point (and look unrelated to stack issues - something else going on there).
This was referenced Jul 3, 2019
Closed
Merged
kripken added a commit that referenced this issue Jul 10, 2019
@kripken
Fix REPORT_RESULT with multiple files (#8918)
1a1c816 
If multiple files existed, we defined the functions in each one, which ended up causing link errors in the upstream backend. Instead, split out the header from the source, and add the source as an extra input. 

This fixes a few of the final wasm backend pthreads errors #8718
belraquib pushed a commit to belraquib/emscripten that referenced this issue Dec 23, 2020
@kripken @belraquib
Enable some pthreads tests on the wasm backend path (emscripten-core#…
39caadd 
…8715)

Build more system libs with -mt.
Test fixes.
Misc minor JS glue fixes.
Some tests still fail on odd things, I marked them as TODO - fix final pthreads tests,  opened emscripten-core#8718 to track that. There may be some serious issues there.
belraquib pushed a commit to belraquib/emscripten that referenced this issue Dec 23, 2020
@kripken @belraquib
Wasm backend pthreads testing improvements (emscripten-core#8842)
d943d30 
 *   After emscripten-core#8811 landed a lot more tests can pass, this enables all those that can.
 *   Remove unnecessary stuff in test_pthread_gcc_atomic_fetch_and_op.
 *   Add more runtime assertions for the stack position.

There are still a few tests that don't pass, so this doesn't close emscripten-core#8718. But they are quite few at this point (and look unrelated to stack issues - something else going on there).
belraquib pushed a commit to belraquib/emscripten that referenced this issue Dec 23, 2020
@kripken @belraquib
Fix REPORT_RESULT with multiple files (emscripten-core#8918)
68b71ca 
If multiple files existed, we defined the functions in each one, which ended up causing link errors in the upstream backend. Instead, split out the header from the source, and add the source as an extra input. 

This fixes a few of the final wasm backend pthreads errors emscripten-core#8718
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Wasm backend pthreads testing improvements emscripten-core/emscripten
1 participant
@kripken