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.

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

[mono] Fix LLVMArgWasmVtypeAsScalar when a method doesn't set its return value #101299

Merged

Conversation

kotlarmilos
Copy link
Member

@kotlarmilos kotlarmilos commented Apr 19, 2024

Description

This PR should fix an assertion in cases when a method doesn't set its return value. Additionally, it should fix trimming-related issues introduced in #101104 (comment).

Fixes #101104

@kotlarmilos kotlarmilos added arch-wasm WebAssembly architecture area-Codegen-JIT-mono labels Apr 19, 2024
@kotlarmilos kotlarmilos added this to the 9.0.0 milestone Apr 19, 2024
@kotlarmilos kotlarmilos self-assigned this Apr 19, 2024
@kotlarmilos
Copy link
Member Author

/azp run runtime-extra-platforms

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@kotlarmilos
Copy link
Member Author

/azp run runtime-extra-platforms-wasm

Copy link

No pipelines are associated with this pull request.

@kotlarmilos
Copy link
Member Author

/azp run runtime-wasm

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@kotlarmilos
Copy link
Member Author

/azp run runtime-wasm

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

radekdoulik and others added 2 commits April 22, 2024 12:57
I think we should handle it here, the same way as `LLVMArgVtypeAsScalar`. They differ only in the return type, where the wasm specific one is returning type of the element instead of `int`.
@kotlarmilos
Copy link
Member Author

/azp run runtime-wasm

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@kotlarmilos kotlarmilos marked this pull request as ready for review April 22, 2024 12:46
@kotlarmilos
Copy link
Member Author

/azp run runtime-wasm

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@kotlarmilos
Copy link
Member Author

/azp run runtime-extra-platforms

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@kotlarmilos kotlarmilos changed the title [mono] Fix an assertion when a method doesn't set its return value [mono] Fix LLVMArgWasmVtypeAsScalar when a method doesn't set its return value Apr 22, 2024
kotlarmilos and others added 2 commits April 23, 2024 08:42
Co-authored-by: Aleksey Kliger (λgeek) <akliger@gmail.com>
@kotlarmilos
Copy link
Member Author

/azp run runtime-wasm

@kotlarmilos
Copy link
Member Author

/azp run runtime-extra-platforms

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

1 similar comment
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

…nally include TrimmerRootDescriptor on Apple mobile platforms
@kotlarmilos
Copy link
Member Author

/azp run runtime-wasm,runtime-extra-platforms

Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@kotlarmilos
Copy link
Member Author

According to the build analysis, the failures are known and shouldn't be related. The wasm-extra-platforms failures seem to be the same as in https://dev.azure.com/dnceng-public/public/_build?definitionId=151&_a=summary.

I've created a tracking issue: #101482

@kotlarmilos kotlarmilos merged commit 43ba3a8 into dotnet:main Apr 25, 2024
159 of 177 checks passed
matouskozak pushed a commit to matouskozak/runtime that referenced this pull request Apr 30, 2024
…eturn value (dotnet#101299)

* Update mini-llvm.c

I think we should handle it here, the same way as `LLVMArgVtypeAsScalar`. They differ only in the return type, where the wasm specific one is returning type of the element instead of `int`.

* Include TrimmerRootDescriptor only on Apple mobile platforms

* Update src/mono/mono/mini/mini-llvm.c
---------

Co-authored-by: Radek Doulik <radek.doulik@gmail.com>
Co-authored-by: Aleksey Kliger (λgeek) <akliger@gmail.com>
michaelgsharp pushed a commit to michaelgsharp/runtime that referenced this pull request May 9, 2024
…eturn value (dotnet#101299)

* Update mini-llvm.c

I think we should handle it here, the same way as `LLVMArgVtypeAsScalar`. They differ only in the return type, where the wasm specific one is returning type of the element instead of `int`.

* Include TrimmerRootDescriptor only on Apple mobile platforms

* Update src/mono/mono/mini/mini-llvm.c
---------

Co-authored-by: Radek Doulik <radek.doulik@gmail.com>
Co-authored-by: Aleksey Kliger (λgeek) <akliger@gmail.com>
@github-actions github-actions bot locked and limited conversation to collaborators May 25, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-wasm WebAssembly architecture area-Codegen-JIT-mono
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[browser] Wasm linux/windows HighResource LibrariestTests AOT are failing on build
3 participants