-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
[release/7.0][sgen] Remove two dead fields; fix wasm-mt AOT compilation #75509
[release/7.0][sgen] Remove two dead fields; fix wasm-mt AOT compilation #75509
Conversation
They were only initialized, but never read. But what they did do is change the SgenClientInfo field offsets between the threaded and non-threaded wasm runtimes (because emscripten only has HAVE_PTHREAD_KILL in the threaded build). As a result the offsets for tlab_start, tlab_next and tlab_temp_end were different for the AOT compilers targeting threaded vs non-threaded WASM. This PR removes the two unused fields which will make the two wasm configurations match again, allowing us to ship a single AOT cross compiler for both threaded and non-threaded wasm.
Tagging subscribers to this area: @BrzVlad Issue DetailsThey were only initialized, but never read. But what they did do is change the SgenClientInfo field offsets between the threaded and non-threaded wasm runtimes (because emscripten only has HAVE_PTHREAD_KILL in the threaded build). As a result the offsets for tlab_start, tlab_next and tlab_temp_end were different for the AOT compilers targeting threaded vs non-threaded WASM. This PR removes the two unused fields which will make the two wasm configurations match again, allowing us to ship a single AOT cross compiler for both threaded and non-threaded wasm. Fixes #75474
|
/cc @lewing |
Manually verified that the wasm AOT compiler produced by this PR for threaded and non-threaded wasm generate identical offsets. And also can AOT compile the raytracer demo. (and hte demo runs correctly) |
/backport to main |
Started backporting to main: https://github.com/dotnet/runtime/actions/runs/3042510478 |
ping @marek-safar |
/azp run runtime-wasm |
Azure Pipelines successfully started running 1 pipeline(s). |
@carlossanlop this one is done baking |
Approved, signed off, CI green. This is perfectly crunchy and ready to |
They were only initialized, but never read.
But what they did do is change the SgenClientInfo field offsets between the threaded and non-threaded wasm runtimes (because emscripten only has HAVE_PTHREAD_KILL in the threaded build). As a result the offsets for tlab_start, tlab_next and tlab_temp_end were different for the AOT compilers targeting threaded vs non-threaded WASM.
This PR removes the two unused fields which will make the two wasm configurations match again, allowing us to ship a single AOT cross compiler for both threaded and non-threaded wasm.
Fixes #75474