FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Change `callvirt` into `calli` for virtual delegates by LeVladIonescu · Pull Request #83461 · dotnet/runtime · GitHub

  1. dotnet
  2. runtime

Repository navigation

Change callvirt into calli for virtual delegates - #83461

Merged
LeVladIonescu merged 27 commits into
dotnet:mainfrom
LeVladIonescu:delegates_issue
Apr 26, 2023
ConversationCommits (27)ChecksFiles changed
Merged

Change callvirt into calli for virtual delegates#83461
LeVladIonescu merged 27 commits into
dotnet:mainfrom
LeVladIonescu:delegates_issue

Conversation

LeVladIonescu commented Mar 15, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

Related to #83329.

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 <vlad-alexandruionescu@Vlads-MacBook-Pro-2.local>
Merge branch 'dotnet:main' into main
f9ed58c
Fixed typo
a381c06 
Signed-off-by: Vlad - Alexandru Ionescu <vlad-alexandruionescu@Vlads-MacBook-Pro-2.local>
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 <vlad-alexandruionescu@Vlads-MacBook-Pro-4.local>
Merge branch 'dotnet:main' into delegates_issue
afd58dd
Merge branch 'dotnet:main' into delegates_issue
45f4845
ghost assigned LeVladIonescu Mar 15, 2023
This was referenced Mar 15, 2023
Open
Closed
Closed
Closed
lambdageek reviewed Mar 15, 2023
View reviewed changes
Comment thread src/mono/mono/metadata/marshal-lightweight.c Outdated
LeVladIonescu requested a review from SamMonoRT March 16, 2023 10:20
LeVladIonescu marked this pull request as ready for review March 16, 2023 13:21
vargaz reviewed Mar 16, 2023
View reviewed changes
Comment thread src/tests/JIT/Delegate/Tests.cs
vargaz reviewed Mar 16, 2023
View reviewed changes
Comment thread src/tests/JIT/Delegate/Tests.csproj
SamMonoRT requested a review from trylek March 16, 2023 19:14
This was referenced Mar 31, 2023
Closed
Open
vargaz reviewed Mar 31, 2023
View reviewed changes
Comment thread src/mono/mono/metadata/marshal.c Outdated
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));

vargaz Mar 31, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

LeVladIonescu Apr 3, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

vargaz Apr 3, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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.

vargaz reviewed Mar 31, 2023
View reviewed changes
Comment thread src/mono/mono/mini/mini-trampolines.c Outdated
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 <vlad-alexandruionescu@Vlads-MacBook-Pro-4.local>
Merge branch 'dotnet:main' into delegates_issue
e7e8913

LeVladIonescu commented Apr 3, 2023

Copy link
Copy Markdown
Contributor Author

/azp run runtime-extra-platforms

azure-pipelines Bot commented Apr 3, 2023

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

This was referenced Apr 3, 2023
Closed
Closed
Closed
Vlad - Alexandru Ionescu added 2 commits April 13, 2023 02:43
Added boxing for valuetype ref
d6e1d3d 
Signed-off-by: Vlad - Alexandru Ionescu <vlad-alexandruionescu@Vlads-MacBook-Pro-4.local>
Added RequiresProcessIsolation property for tests in merged directory
0074bea 
Signed-off-by: Vlad - Alexandru Ionescu <vlad-alexandruionescu@Vlads-MacBook-Pro-4.local>
build-analysis Bot mentioned this pull request Apr 13, 2023
Closed

kg commented Apr 13, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

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 commented Apr 13, 2023

Copy link
Copy Markdown
Contributor

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 commented Apr 13, 2023

Copy link
Copy Markdown
Contributor

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

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 <vlad-alexandruionescu@Vlads-MacBook-Pro-4.local>
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 <vlad-alexandruionescu@Vlads-MacBook-Pro-5.local>

LeVladIonescu commented Apr 24, 2023

Copy link
Copy Markdown
Contributor Author

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

SamMonoRT commented Apr 25, 2023

Copy link
Copy Markdown
Member

Let's merge this after Preview4 snap.

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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

lambdageek lambdageek approved these changes
SamMonoRT Awaiting requested review from SamMonoRT
thaystg Awaiting requested review from thaystg thaystg is a code owner
trylek Awaiting requested review from trylek
+1 more reviewer
vargaz vargaz approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

LeVladIonescu

Labels

area-Codegen-JIT-mono

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

Back | FazBrowse Home | New Git URL