Change `callvirt` into `calli` for virtual delegates #83461

LeVladIonescu · 2023-03-15T15:00:29Z

Related to #83329.

…ntations Signed-off-by: Vlad - Alexandru Ionescu <[email protected]>

Signed-off-by: Vlad - Alexandru Ionescu <[email protected]>

src/mono/mono/metadata/marshal-lightweight.c

src/tests/JIT/Delegate/Tests.cs

src/tests/JIT/Delegate/Tests.csproj

vargaz · 2023-03-31T22:09:32Z

src/mono/mono/metadata/marshal.c

@@ -6287,7 +6283,7 @@ mono_marshal_free_dynamic_wrappers (MonoMethod *method)
 	if (image->wrapper_caches.runtime_invoke_method_cache)
 		clear_runtime_invoke_method_cache (image->wrapper_caches.runtime_invoke_method_cache, method);
 	if (image->wrapper_caches.delegate_abstract_invoke_cache)
-		g_hash_table_foreach_remove (image->wrapper_caches.delegate_abstract_invoke_cache, signature_pointer_pair_matches_pointer, method);
+		g_hash_table_remove (image->wrapper_caches.delegate_abstract_invoke_cache, mono_method_signature_internal (method));


This is not be needed anymore since the wrappers are no longer associated with the method.

And how is that cache going to be freed if we remove it from here?

Its freed already in marshal.c, we just don't need to handle it here, since these wrappers will no longer depend on dynamic methods.

src/mono/mono/mini/mini-trampolines.c

Signed-off-by: Vlad - Alexandru Ionescu <[email protected]>

LeVladIonescu · 2023-04-03T17:58:12Z

/azp run runtime-extra-platforms

azure-pipelines · 2023-04-03T17:58:32Z

Azure Pipelines successfully started running 1 pipeline(s).

Signed-off-by: Vlad - Alexandru Ionescu <[email protected]>

kg · 2023-04-13T17:13:49Z

I can reproduce the browser System.Runtime.Tests failure in console with XUNIT_RANDOM_ORDER_SEED=875207244. It occurs in AOT but not in interp mode, and occurs even if jiterpreter is disabled. It seems to be some sort of problem with the AOT<->interp transitions that are happening when most of the test suite is AOT'd.

kg · 2023-04-13T19:36:06Z

Error info from a modified debug build:

* Assertion at /home/kate/Projects/dotnet-runtime-wasm/src/mono/mono/mini/interp/interp.c:1874, condition `ftndesc->method' not met
at mono_wasm_stringify_as_error_with_stack (/home/kate/Projects/dotnet-runtime-wasm/artifacts/bin/System.Runtime.Tests/Debug/net8.0-browser/browser-wasm/AppBundle/dotnet.js:810:22)
at mono_wasm_trace_logger (/home/kate/Projects/dotnet-runtime-wasm/artifacts/bin/System.Runtime.Tests/Debug/net8.0-browser/browser-wasm/AppBundle/dotnet.js:829:31)
at wasm_trace_logger (wasm_trace_logger (wasm://wasm/1fdbaf7e:wasm-function[109418]:0x672e272))
at eglib_log_adapter (eglib_log_adapter (wasm://wasm/1fdbaf7e:wasm-function[93132]:0x61719b4))
at monoeg_g_logstr (monoeg_g_logstr (wasm://wasm/1fdbaf7e:wasm-function[92989]:0x616175b))
at monoeg_g_logv_nofree (monoeg_g_logv_nofree (wasm://wasm/1fdbaf7e:wasm-function[92987]:0x61615c4))
at monoeg_assertion_message (monoeg_assertion_message (wasm://wasm/1fdbaf7e:wasm-function[92991]:0x616183c))
at mono_assertion_message (mono_assertion_message (wasm://wasm/1fdbaf7e:wasm-function[92992]:0x61618ea))
at mono_interp_exec_method (mono_interp_exec_method (wasm://wasm/1fdbaf7e:wasm-function[92646]:0x612c6a5))
at interp_entry (interp_entry (wasm://wasm/1fdbaf7e:wasm-function[92719]:0x6131807))
at interp_entry_instance_ret_1 (interp_entry_instance_ret_1 (wasm://wasm/1fdbaf7e:wasm-function[92733]:0x6131eb5))
at aot_instances_aot_wrapper_gsharedvt_in_sig_obj_this_bii (aot_instances_aot_wrapper_gsharedvt_in_sig_obj_this_bii (wasm://wasm/1fdbaf7e:wasm-function[68462]:0x45f40a3))
at System_Runtime_Tests_System_Tests_CreateDelegateTests_CreateDelegate10_Nullable_Method (System_Runtime_Tests_System_Tests_CreateDelegateTests_CreateDelegate10_Nullable_Method (wasm://wasm/1fdbaf7e:wasm-function[40702]:0x2094aad))

kg · 2023-04-13T19:48:27Z

That assertion is in ftnptr_to_imethod, which means it's either in the implementation of CALLI or interp_delegate_ctor.

Signed-off-by: Vlad - Alexandru Ionescu <[email protected]>

LeVladIonescu · 2023-04-24T12:10:49Z

This version is ready to be merged
cc @vargaz @lambdageek @SamMonoRT

SamMonoRT · 2023-04-25T13:00:55Z

Let's merge this after Preview4 snap.

Vlad - Alexandru Ionescu and others added 13 commits November 24, 2022 12:33

Changed how NaN values are compared due to their multiple bit represe…

a175351

…ntations Signed-off-by: Vlad - Alexandru Ionescu <[email protected]>

Merge branch 'dotnet:main' into main

f9ed58c

Fixed typo

a381c06

Signed-off-by: Vlad - Alexandru Ionescu <[email protected]>

Merge branch 'dotnet:main' into main

d91a78f

Merge branch 'dotnet:main' into main

1291fe3

Merge branch 'dotnet:main' into main

9a58bb3

Merge branch 'dotnet:main' into main

8db950e

Merge branch 'dotnet:main' into main

f587d24

Merge branch 'dotnet:main' into main

f5dbeaf

Merge branch 'dotnet:main' into main

287419e

Work to make JIT virtual delegates not depend on the target_method

fbc1526

Signed-off-by: Vlad - Alexandru Ionescu <[email protected]>

Merge branch 'dotnet:main' into delegates_issue

afd58dd

Merge branch 'dotnet:main' into delegates_issue

45f4845

LeVladIonescu added the area-Codegen-JIT-mono label Mar 15, 2023

ghost assigned LeVladIonescu Mar 15, 2023

lambdageek reviewed Mar 15, 2023

View reviewed changes

src/mono/mono/metadata/marshal-lightweight.c Outdated Show resolved Hide resolved

Switched order of how MonoObject is passed to the icall

91c3920

LeVladIonescu requested a review from SamMonoRT March 16, 2023 10:20

Initialise pointer for build warning

81973db

LeVladIonescu marked this pull request as ready for review March 16, 2023 13:21

LeVladIonescu requested review from vargaz and thaystg as code owners March 16, 2023 13:21

vargaz reviewed Mar 16, 2023

View reviewed changes

src/tests/JIT/Delegate/Tests.cs Outdated Show resolved Hide resolved

vargaz reviewed Mar 16, 2023

View reviewed changes

src/tests/JIT/Delegate/Tests.csproj Outdated Show resolved Hide resolved

Check return value in the test

3b0d6cb

SamMonoRT requested a review from trylek March 16, 2023 19:14

This was referenced Mar 31, 2023

WasmTestOnBrowser-System.* test failures in CI #83655

Closed

[6.0] Android test failure: Failed to install mobile app #83422

Open

vargaz reviewed Mar 31, 2023

View reviewed changes

src/mono/mono/mini/mini-trampolines.c Outdated Show resolved Hide resolved

vargaz approved these changes Mar 31, 2023

View reviewed changes

Vlad - Alexandru Ionescu and others added 2 commits April 3, 2023 19:56

Added test for lazy fetch trampoline (rgctx)

7b65607

Signed-off-by: Vlad - Alexandru Ionescu <[email protected]>

Merge branch 'dotnet:main' into delegates_issue

e7e8913

vargaz approved these changes Apr 3, 2023

View reviewed changes

This was referenced Apr 3, 2023

Wasm debugger test timing out #83847

Closed

[wasm] interpreter timeouts when WebSocket closes unexpectedly #84101

Closed

CI System.Text.RegularExpressions.Tests failure #84323

Closed

Vlad - Alexandru Ionescu added 2 commits April 13, 2023 02:43

Added boxing for valuetype ref

d6e1d3d

Signed-off-by: Vlad - Alexandru Ionescu <[email protected]>

Added RequiresProcessIsolation property for tests in merged directory

0074bea

Signed-off-by: Vlad - Alexandru Ionescu <[email protected]>

build-analysis bot mentioned this pull request Apr 13, 2023

WasmTestOnBrowser-System.Text.Json.Tests.WorkItemExecution timing out #84434

Closed

Vlad - Alexandru Ionescu and others added 4 commits April 19, 2023 16:33

Using get_ftnptr callback and moving boxing inside the icall

be6daed

Signed-off-by: Vlad - Alexandru Ionescu <[email protected]>

Merge branch 'dotnet:main' into delegates_issue

596837e

Merge branch 'dotnet:main' into delegates_issue

414f451

Rremoved add_delegate_trampoline since it's not used anymore

1d2c4fb

Signed-off-by: Vlad - Alexandru Ionescu <[email protected]>

lambdageek approved these changes Apr 24, 2023

View reviewed changes

LeVladIonescu merged commit 2677eb2 into dotnet:main Apr 26, 2023

LeVladIonescu deleted the delegates_issue branch April 26, 2023 06:27

ghost locked as resolved and limited conversation to collaborators May 26, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Change `callvirt` into `calli` for virtual delegates #83461

Change `callvirt` into `calli` for virtual delegates #83461

LeVladIonescu commented Mar 15, 2023 •

edited

Loading

vargaz Mar 31, 2023

LeVladIonescu Apr 3, 2023

vargaz Apr 3, 2023

LeVladIonescu commented Apr 3, 2023

azure-pipelines bot commented Apr 3, 2023

kg commented Apr 13, 2023 •

edited

Loading

kg commented Apr 13, 2023

kg commented Apr 13, 2023

LeVladIonescu commented Apr 24, 2023

SamMonoRT commented Apr 25, 2023

Change callvirt into calli for virtual delegates #83461

Change callvirt into calli for virtual delegates #83461

Conversation

LeVladIonescu commented Mar 15, 2023 • edited Loading

vargaz Mar 31, 2023

Choose a reason for hiding this comment

LeVladIonescu Apr 3, 2023

Choose a reason for hiding this comment

vargaz Apr 3, 2023

Choose a reason for hiding this comment

LeVladIonescu commented Apr 3, 2023

azure-pipelines bot commented Apr 3, 2023

kg commented Apr 13, 2023 • edited Loading

kg commented Apr 13, 2023

kg commented Apr 13, 2023

LeVladIonescu commented Apr 24, 2023

SamMonoRT commented Apr 25, 2023

Change `callvirt` into `calli` for virtual delegates #83461

Change `callvirt` into `calli` for virtual delegates #83461

LeVladIonescu commented Mar 15, 2023 •

edited

Loading

kg commented Apr 13, 2023 •

edited

Loading