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

[wasi] Implement TODO in mono-threads-wasi.S #101337

Merged
merged 8 commits into from
Jul 4, 2024
Merged

[wasi] Implement TODO in mono-threads-wasi.S #101337

merged 8 commits into from
Jul 4, 2024

Conversation

akoeplinger
Copy link
Member

@akoeplinger akoeplinger commented Apr 20, 2024 •
edited
Loading

We're using an LLVM version that contains support for __stack_low and __stack_high so we can use it to get the wasm stack bounds.

@akoeplinger
[wasi] Implement TODO in mono-threads-wasi.S
2e26221 
Noticed this TODO and since we're using an LLVM version that contains this change we can fix it.
This was referenced Apr 20, 2024
Closed
Closed
Open
Closed
@akoeplinger
Copy link
Member Author

looks like this is breaking something so marking as no-merge for now:

[17:59:58] info: [MONO] error: * Assertion at /__w/1/s/src/mono/mono/utils/mono-threads-wasm.c:235, condition `(guint8*)&tmp > *staddr' not met

@akoeplinger akoeplinger added the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Apr 20, 2024
@pavelsavara pavelsavara self-assigned this Apr 22, 2024
@lewing lewing requested a review from radekdoulik April 22, 2024 14:36
This was referenced Jun 17, 2024
Closed
Open
Closed
Closed
Closed
Open
@akoeplinger akoeplinger force-pushed the fix-wasi-todo branch 2 times, most recently from ac8295a to 3c3aec5 Compare June 17, 2024 14:16
This was referenced Jun 17, 2024
Closed
Open
@build-analysis build-analysis bot mentioned this pull request Jun 17, 2024
Closed
@pavelsavara
Copy link
Member

pavelsavara commented Jun 18, 2024 •
edited
Loading 

 Wasm.Build.Tests.WasmTemplateTests.ConsoleBuildAndRunDefault(config: "Debug", relinking: True) [FAIL]
[MONO] * Assertion at /__w/1/s/src/mono/mono/utils/mono-threads-wasm.c:48, condition `<disabled>' not met

https://helixre107v0xdcypoyl9e7f.blob.core.windows.net/dotnet-runtime-refs-pull-101337-merge-396c837070724d6abb/Workloads-ST-Wasm.Build.Tests.WasmTemplateTests/1/console.1a3f5f65.log?helixlogtype=result

This was referenced Jun 18, 2024
Open
Open
@pavelsavara pavelsavara removed the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Jun 20, 2024
pavelsavara
pavelsavara approved these changes Jun 20, 2024
View reviewed changes
src/mono/mono/utils/CMakeLists.txt Outdated Show resolved Hide resolved
@build-analysis build-analysis bot mentioned this pull request Jun 20, 2024
Open
@pavelsavara
Copy link
Member

It seems that some of the System.Threading.Tests.SemaphoreSlimTests and System.Threading.Tasks.Tests.TaskContinueWithTests are deadlocking. I'm quite sure it's unrelated to stack size and that we could merge.

@radekdoulik do you see same issues on emscripten bump ?

@pavelsavara
Copy link
Member

/ba-g I opened #104418 for the deadlocks

@pavelsavara pavelsavara merged commit 2a72651 into dotnet:main Jul 4, 2024
76 of 79 checks passed
@akoeplinger akoeplinger deleted the fix-wasi-todo branch July 4, 2024 13:15
@github-actions github-actions bot locked and limited conversation to collaborators Aug 4, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@lambdageek lambdageek lambdageek left review comments

@pavelsavara pavelsavara pavelsavara approved these changes

@radekdoulik radekdoulik Awaiting requested review from radekdoulik

@steveisok steveisok Awaiting requested review from steveisok steveisok is a code owner

Assignees

@pavelsavara pavelsavara

@akoeplinger akoeplinger

Labels
area-VM-meta-mono
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@akoeplinger @pavelsavara @lambdageek @lewing