[mono][interp] Update mono struct lowering to use MonoMemoryManager #102486
Conversation
kotlarmilos
requested review from
lambdageek,
steveisok and
thaystg
as code owners
|
Tagging subscribers to this area: @BrzVlad, @kotlarmilos |
There was a problem hiding this comment.
Thanks a lot for the quick fix. The failing test was caused by my PR #102143 and the fact that at the moment when the CI ran on my PR the SwiftErrorHandling tests were disabled so it didn't get caught before merging.
src/mono/mono/mini/method-to-ir.c
Outdated
| @@ -7585,7 +7585,7 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b | |||
| } | |||
|
|
|||
| // Create a new dummy signature with the lowered arguments | |||
| fsig = mono_metadata_signature_dup_new_params (cfg->mempool, fsig, new_param_count, (MonoType**)new_params->data); | |||
| fsig = mono_metadata_signature_dup_new_params (cfg->mem_manager, fsig, new_param_count, (MonoType**)new_params->data); | |||
There was a problem hiding this comment.
Is this really needed though ? While interpreter requires the signature to be alive after the code is compiled, I don't think jit code has this constraint. This change prevents the signature from being freed as it was done before.
There was a problem hiding this comment.
We could use a similar approach as with mono_metadata_signature_dup_internal
runtime/src/mono/mono/metadata/metadata.c
Line 2429 in b1b3e85
mempool for JIT and mem_manager for interpreter.
…r a MonoMethodSignature based on the provided parameters
src/mono/mono/metadata/metadata.c
Outdated
| @@ -2559,13 +2576,13 @@ mono_metadata_signature_dup_delegate_invoke_to_target (MonoMethodSignature *sig) | |||
| * @return the new \c MonoMethodSignature structure. | |||
| */ | |||
| MonoMethodSignature* | |||
| mono_metadata_signature_dup_new_params (MonoMemPool *mp, MonoMethodSignature *sig, uint32_t num_params, MonoType **new_params) | |||
| mono_metadata_signature_dup_new_params (MonoImage *image, MonoMemPool *mp, MonoMemoryManager *mem_manager, MonoMethodSignature *sig, uint32_t num_params, MonoType **new_params) | |||
There was a problem hiding this comment.
Looks like we can drop the image arg here.
There was a problem hiding this comment.
Yes, I added it for future extensibility, but not needed at this point.
…otnet#102486) * Update mono_metadata_signature_dup_new_params to use MonoMemoryManager * Refactor mono_metadata_signature_dup_new_params to allocate memory for a MonoMethodSignature based on the provided parameters * Remove unused param from the mono_metadata_signature_dup_new_params
…otnet#102486) * Update mono_metadata_signature_dup_new_params to use MonoMemoryManager * Refactor mono_metadata_signature_dup_new_params to allocate memory for a MonoMethodSignature based on the provided parameters * Remove unused param from the mono_metadata_signature_dup_new_params
Description
This PR updates
mono_metadata_signature_dup_new_paramsto useMonoMemoryManagerinstead ofMonoMemPool. The mempool can be detroyed after method is compiled.Fixes #102478