[wasm64] Fix and run all pthread browser tests #20527
Conversation
sbc100
force-pushed
the
browser64_pthreads
branch
3 times, most recently
from
f3a32d7 to
18af054
Compare
sbc100
changed the title
sbc100
force-pushed
the
browser64_pthreads
branch
2 times, most recently
from
6bf7a50 to
ceec2c0
Compare
| @@ -57,7 +57,7 @@ void *WaitingThread(void *arg) | |||
|
|
|||
| int main() | |||
| { | |||
| for(int i = 0; i < NUM_THREADS; ++i) | |||
| for(intptr_t i = 0; i < NUM_THREADS; ++i) | |||
There was a problem hiding this comment.
Surely the number of threads fits in an int? 😄
lgtm regardless, but I'm curious if this fixed a specific issue? Maybe that the higher bits were undefined?
There was a problem hiding this comment.
The specific issue here and elsewhere is that i is cast to a void* and passed to pthread_create as the argument to the thread start function.
Casting an int to void* is find on wasm32 but generates a warning on wasm64.
| @@ -129,7 +129,7 @@ int main() | |||
| fetch_and_and_data = HILO(65536 + (1<<(NUM_THREADS+1))-1, (1<<(NUM_THREADS+1))-1); | |||
| for(int i = 0; i < NUM_THREADS; ++i) | |||
| { | |||
| threadArg[i] = DUP(~(1UL<<i)); | |||
| threadArg[i] = DUP(~(1u<<i)); | |||
There was a problem hiding this comment.
Doesn't this change us from an unsigned long to plain unsigned? We seem to store this to uint64_t, so isn't the former better, if it makes any difference?
There was a problem hiding this comment.
Yes. DUP() here is supposed to take an i32 and use HILO() to construct an i64 where the low and high parts are the same.
The current code which uses UL works for wasm32 because UL is i32 but on wasm64 UL is i64 which breaks DUP / HILO macros.
I verified this was broken on 64-bit linux too... and this was the fix there.
sbc100
force-pushed
the
browser64_pthreads
branch
from
ceec2c0 to
608f5d2
Compare
test/test_browser.py
Outdated
| @@ -3875,6 +3877,7 @@ def test_pthread_gcc_atomic_fetch_and_op(self): | |||
| self.btest_exit('pthread/test_pthread_gcc_atomic_fetch_and_op.cpp', args=args + ['-sINITIAL_MEMORY=64MB', '-pthread', '-sPTHREAD_POOL_SIZE=8']) | |||
|
|
|||
| # 64 bit version of the above test. | |||
| @no_wasm64("XXX") | |||
sbc100
force-pushed
the
browser64_pthreads
branch
from
608f5d2 to
d75f024
Compare
I accidentally duplicated this in emscripten-core#20527
I accidentally duplicated this in #20527
Split out from #20516