Skip to content

[interp] Add x86 support#19730

Merged
BrzVlad merged 2 commits intomono:masterfrom
BrzVlad:feature-interp-x86
May 8, 2020
Merged

[interp] Add x86 support#19730
BrzVlad merged 2 commits intomono:masterfrom
BrzVlad:feature-interp-x86

Conversation

@BrzVlad
Copy link
Member

@BrzVlad BrzVlad commented May 7, 2020

This mainly consists of a trampoline that is used when calling from interp into native code, as well as the call convention logic that initializes the CallContext structure (which is passed to the trampoline). On other platforms we also have a single trampoline used when entering interpreter code. That trampoline works in a similar way and its purpose is to avoid compilation of all possible interp entry signatures, which is problematic on full aot scenarios. Given the interp x86 support is currently provided for android, we can avoid using that trampoline for now and use the per signature jit compiled interp_in_wrappers instead.

The few disabled interp tests are the same as on amd64.

We only support pinvoke into cdecl functions.

BrzVlad added 2 commits May 7, 2020 15:42
On x86 linux, double doesn't guarantee 8 byte alignment, which we need since all objects on the vt stack need to be 8 byte aligned. Use padding instead to achieve this.
This mainly consists of a trampoline that is used when calling from interp into native code, as well as the call convention logic that initializes the CallContext structure (which is passed to the trampoline). On other platforms we also have a single trampoline used when entering interpreter code. That trampoline works in a similar way and its purpose is to avoid compilation of all possible interp entry signatures, which is problematic on full aot scenarios. Given the interp x86 support is currently provided for android, we can avoid using that trampoline for now and use the per signature jit compiled interp_in_wrappers instead.

The few disabled interp tests are the same as on amd64.

We only support pinvoke into cdecl functions.
#if SIZEOF_VOID_P == 4
/* Align data field to MINT_VT_ALIGNMENT */
gint32 pad;
#endif
Copy link
Contributor

Choose a reason for hiding this comment

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

I assume this is because double is 4 byte aligned on x86 ?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes. And mono_64bitaligned_t doesn't work. We probably get alignment correctly with it by chance

void
mono_arch_set_native_call_context_args (CallContext *ccontext, gpointer frame, MonoMethodSignature *sig)
{
CallInfo *cinfo = get_call_info (NULL, sig);
Copy link
Contributor

Choose a reason for hiding this comment

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

Would be nice to add some kind of per-signature caching for this stuff, this looks very expensive.

Copy link
Member Author

Choose a reason for hiding this comment

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

That is a pending optimization for a while now. This pinvoke path is only used when having weird signatures (like ones receiving value types), and we have fastpaths using do_icall for most common signatures (interp_icall_op_for_sig). Given these special signatures didn't really show up yet in a real application, the improvement still remains pending ...

@BrzVlad
Copy link
Member Author

BrzVlad commented May 8, 2020

@monojenkins build failed

@BrzVlad BrzVlad merged commit 798f223 into mono:master May 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants