[Mono-dev] [PATCH] more support for Google Native Client
Zoltan Varga
vargaz at gmail.com
Wed Jan 5 18:51:14 EST 2011
Hi,
I think the current code looks ok, and we should think about how to merge
it into mono trunk. As a first step, could you rebase your master branch on
top of master to fix the few conflicts which has surfaced due to changes to
mono master ?
Zoltan
On Wed, Jan 5, 2011 at 8:23 PM, Elijah Taylor <elijahtaylor at google.com>wrote:
> Hi Zoltan,
>
> I've addressed all of the issues you pointed out (minus genmdesc.c:
> __nacl_suspend_thread_if_needed, but that doesn't need to be merged in at
> this time, it can remain in my local repository only). Please take another
> look at your earliest convenience and let me know if there's anything else
> you need from me.
>
> -Elijah
>
>
> On Tue, Jan 4, 2011 at 10:55 AM, Elijah Taylor <elijahtaylor at google.com>wrote:
>
>> Replies inline:
>>
>> On Tue, Jan 4, 2011 at 10:30 AM, Zoltan Varga <vargaz at gmail.com> wrote:
>>
>>> Hi,
>>>
>>> Some comments:
>>> - the patch changes IMT_REG to AMD64_R11 in the non-nacl case, I'm not
>>> sure thats
>>> intentional.
>>>
>>
>> Has this changed in the last six months on the Mono side? IIRC I didn't
>> mean to change anything like this. The reason I made explicit defines was
>> so code in aot-compiler and mini-amd64 could share defines over which reg
>> was the one we jump through and which was a scratch reg. I'll diff vs Mono
>> head revision and make it correct.
>>
>>
>>> - you could define __mono_ilp32__ in the nacl/amd64 case, and use that
>>> instead of
>>> defined(__native_client_codegen__) && defined(TARGET_AMD64) in a few
>>> places.
>>>
>>
>> That sounds reasonable. I'm assuming you mean non-arch specific areas
>> like mini.c, aot-*.c, method-to-ir.c, etc? Are there any other major
>> consequences to defining __mono_ilp32__ ?
>>
>>
>>> - it would be better to define nacl_global_codeman_validate () as a no-op
>>> in the non-nacl case, so its callers wouldn't need #ifdefs.
>>>
>>
>> I'll fix this.
>>
>>
>>> - genmdesc.c contains this change, which is probably not needed:
>>> +void __nacl_suspend_thread_if_needed() {}
>>> +
>>>
>>
>> It is needed temporarily due to a preliminary GC implementation, we don't
>> have to submit it this way. Eventually (soon) we won't need it at all.
>>
>>
>>> - you could use sizeof(mgreg_t) instead of SIZEOF_REGISTER to be
>>> consistent with
>>> the usage of sizeof(gpointer).
>>>
>>
>> Sounds good. I'll try to use sizeof for all compiled code and only use
>> SIZEOF_REGISTER/SIZEOF_VOID_P for pre-processor directives only.
>>
>>
>>> Other than these, I think the changes look fine, they aren't that
>>> disruptive, since they don't
>>> change the non-nacl behavior at all.
>>>
>>>
>> Great! I was worried just based on LOC changed that it might get more
>> resistance. In truth I'm more worried about future Mono changes
>> accidentally breaking NaCl behavior. I'm planning on getting some automated
>> testing implemented soon to combat this though.
>>
>>
>>
>>
>>> On Tue, Dec 21, 2010 at 9:12 PM, Elijah Taylor <elijahtaylor at google.com>wrote:
>>>
>>>> Greetings Mono developers!
>>>>
>>>>
>>>> *[tl;dr very large patch for Native Client<http://www.chromium.org/nativeclient> support
>>>> hosted here <https://github.com/elijahtaylor/mono>, would love feedback
>>>> and many eyes to look at it]
>>>> *
>>>>
>>>>
>>>> I'm back with another round of changes for supporting Google's Native
>>>> Client (NaCl), including support for amd64, JIT compilation, and Garbage
>>>> Collection. It's a large set of changes, forked on Dec 14 in github @
>>>> https://github.com/elijahtaylor/mono. I would appreciate feedback on
>>>> these changes... to facilitate this, I'll try to explain the largest changes
>>>> by feature (please email if clarification is needed):
>>>>
>>>> *1) amd64 codegen*
>>>>
>>>> - Rules located here:
>>>> http://www.chromium.org/nativeclient/design-documents/nacl-sfi-model-on-x86-64-systems
>>>> - Removed %r15 from register allocation, LMF save/restore, etc.
>>>> (r15 is special and not modifiable by untrusted code)
>>>> - Sandbox all data access through membase address mode. If not
>>>> %rsp or %rbp relative, re-write as clearing upper 32-bits + memindex
>>>> addressing
>>>> - align functions, call sites
>>>> - Sandbox returns and all indirect jumps (need to be 32-byte
>>>> aligned, cleared upper 32-bits)
>>>> - Never omit frame pointer as general operations to rbp aren't
>>>> allowed
>>>>
>>>> *2) NaCl x86-64 is ILP32 (this is the largest set of changes and may
>>>> make some mono devs unhappy)*
>>>>
>>>> - Set SIZEOF_REGISTER == 8 while sizeof(gpointer) == 4 for NaCl
>>>> amd64 (we can use 8-byte instructions, but pointers are 4-bytes)
>>>> - Re-write large portions of mini-amd64.c, tramp-amd64.c,
>>>> exceptions-amd64.c, mini.c, method-to-ir.c to use appropriate sizes
>>>> (SIZEOF_REGISTER, sizeof(gpointer), literal '8'). *These changes
>>>> are disruptive, but ultimately they should be more correct than what was
>>>> there before. *It's our opinion that these changes actually improve
>>>> Mono despite their impact.
>>>> - We only generate NaCl amd64 code from an ILP32 machine (either a
>>>> 32-bit application for AOT code, or NaCl runtime JIT), so we may not have
>>>> caught all of the [8 <--> SIZEOF_REGISTER] conversions, but we likely caught
>>>> most of the [sizeof(gpointer) <--> SIZEOF_REGISTER] and [8 <-->
>>>> sizeof(gpointer)] changes that are necessary.
>>>> - Change atomic operations and default pointer directives to use
>>>> 32-bit instructions (long instead of quad)
>>>> - Change default operations to use 32-bit integers/pointers (eg,
>>>> OP_LOAD_MEMBASE uses 4-bytes instead of 8)
>>>>
>>>> *3) JIT support for NaCl*
>>>>
>>>> - Since we're unable to emit code directly in its final executable
>>>> location, we instead:
>>>> - reserve a buffer on the heap
>>>> - create a hash table entry mapping the temp location and final
>>>> location
>>>> - modify all non-local patches relative to the final location
>>>> - request the NaCl runtime to install the created code in the
>>>> final location
>>>> - See mono/utils/mono-codeman.c changes for more detail.
>>>> - For every codeman *reserve*, we must add a codeman *validate* call
>>>> in order to install the method/trampoline/blob in the final location (as
>>>> well as validate it for NaCl, pad it out, etc)
>>>> - We don't delete or reuse code (we can, but it's icky and the
>>>> benefits don't outweigh the cost)
>>>> - Backpatching changed to use NaCl syscalls to modify existing
>>>> dynamic code
>>>>
>>>> *4) GC support for NaCl (boehm only)*
>>>>
>>>> - NaCl compiler and Mono code generator both emit instrumentation at
>>>> GC "safe points" (back branches and function prologs), for cooperative
>>>> thread parking (we're not allowed to send and receive signals)
>>>> - Added new opcode OP_NACL_GC_SAFE_POINT to handle mono
>>>> instrumentation
>>>> - modified pthread_stop_world.c and pthread_support.c somewhat
>>>> extensively to support this new way of stopping the world
>>>> - wrapped pthread_exit because NaCl doesn't support pthread cleanup
>>>> functions
>>>> - added machine type NACL to libgc with machine specific defines
>>>>
>>>>
>>>> *5) Misc bug fixes (not NaCl-specific)*
>>>>
>>>> - fix *x86_memindex_emit* when disp is 32-bit
>>>> - properly exclude code in libgc/gc_dlopen.c when DYNAMIC_LOADING
>>>> not defined
>>>> - properly exclude code based on DISABLE_SOCKETS by including
>>>> config.h before checking define
>>>> - clean up calculation of offset for amd64 AOT specific trampoline
>>>> args
>>>> - fix bug in *mono_bblock_insert_before_ins* when trying to insert
>>>> an instruction to the beginning of an existing basic block.
>>>> - fix small typo bug in genmdesc.pl which kept amd64 from being able
>>>> to be a target of cross compiling
>>>> - fix struct passing in amd64 with sizeof(struct) == 16 when fields
>>>> aren't 8-byte aligned (eg, first field is 12 bytes, second field is 4
>>>> bytes), pass on stack instead of in registers (mini-amd64.c:*
>>>> add_valuetype*)
>>>> - add extra checks to mini-amd64.c:*mono_arch_emit_exceptions* to
>>>> keep exception/R4/R8 emitting from overflowing a buffer silently
>>>> - fix bugs in *new_codechunk* and *mono_code_manager_reserve_align*which allowed unaligned code to be allocated.
>>>>
>>>>
>>>> I know we're close to holidays so I don't have any delusions that these
>>>> changes will get in by the end of the year :) Please feel free to pick
>>>> apart these changes and let me know if there are things that should be
>>>> changed.
>>>>
>>>> -Elijah
>>>>
>>>> _______________________________________________
>>>> Mono-devel-list mailing list
>>>> Mono-devel-list at lists.ximian.com
>>>> http://lists.ximian.com/mailman/listinfo/mono-devel-list
>>>>
>>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.ximian.com/pipermail/mono-devel-list/attachments/20110106/d98cdeb9/attachment.html
More information about the Mono-devel-list
mailing list