[mono][aot] Enable direct call transformation for MONO_PATCH_INFO_METHOD #82711
Conversation
kotlarmilos
requested review from
vargaz,
lambdageek and
SamMonoRT
as code owners
src/mono/mono/mini/aot-compiler.c
Outdated
| // Enable direct call transformation where caller's class requires no initialization | ||
| if (!m_class_has_static_refs (method->klass)) { | ||
| // Enable direct call transformation where caller's class requires no initialization and callee can specialize | ||
| if (!m_class_has_static_refs (method->klass) && mono_aot_can_specialize (cmethod)) { |
There was a problem hiding this comment.
has_static_refs () means the class has static reference fields, not that it has an initializer.
There was a problem hiding this comment.
The assumption is that if a caller method has static reference fields it requires initialization. Is there a better way to check if the caller method has to be initialized?
There was a problem hiding this comment.
m_class_has_cctor (method->klass) will return whenever the class has a static ctor.
There was a problem hiding this comment.
Why is this condition required ?
There was a problem hiding this comment.
This PR enables direct call transformation for MONO_PATCH_INFO_METHOD. If a caller requires initialization but got transformed, it fails with SIGSEGV as static fields are null. If a called method is invoked by reflection or can be invoked externally, it can't be transformed. m_class_has_cctor checks if the caller requires no initialization, while mono_aot_can_specialize checks if the called method isn't called externally or by using reflection.
Does that make sense?
There was a problem hiding this comment.
Actually it might be good to add it to is_direct_callable.
kotlarmilos
changed the title
|
Failures shouldn't be related. |
| MonoCompile *callee_cfg = (MonoCompile *)g_hash_table_lookup (acfg->method_to_cfg, cmethod); | ||
|
|
||
| // Don't compile inflated methods if we're doing dedup | ||
| if (acfg->aot_opts.dedup && !mono_aot_can_dedup (cmethod)) { | ||
| if (!m_class_has_cctor (method->klass) && mono_aot_can_specialize (callee_cfg->method)) { | ||
| char *name = mono_aot_get_mangled_method_name (cmethod); |
There was a problem hiding this comment.
Looked into why this was not enabled before, and i don't think this is going to work. The callee method might use GOT slots etc. which are not initialized if the method is directly called. So before this can be enabled, the AOTed code needs to be changed to init itself, the same way the llvm compiled code does.
There was a problem hiding this comment.
Good point! Let's first try to enable method initialization during the invocation before proceeding with this and other direct call transformations.
|
This issue is blocked by #83377. Closing it until the prolog method initialization is implemented. |
ghost
locked as resolved and limited conversation to collaborators
This PR contributes to #82224 by enabling direct call transformation for
MONO_PATCH_INFO_METHODwhere caller requires no initialization.