[Mono-dev] Proposed Patch - Google Native Client

[Elijah Taylor]

Great, thanks for the help in looking over the changes and merging them in.
 I'll try to grab the latest to test our changes from there and try to
rebase our changes a bit.

Expect a 64-bit patch coming soon.  The changes for Native Client are a bit
more extensive for amd64 but I'll work on reducing the patch as much as
possible before sending it here to the list.

-Elijah


On Mon, Aug 16, 2010 at 10:32 PM, Zoltan Varga <vargaz at gmail.com> wrote:

> Hi,
>
>   Committed the rest of the patch with the following changes:
> - use configure machinery to disable building of ikvm-native instead of
> #ifdef-ing the whole file.
> - define a DISABLE_SOCKETS define and use that to disable building of
> sockets.c and
>   socket-io.c.
>
>                        Zoltan
>
>
> On Mon, Aug 9, 2010 at 5:46 PM, Zoltan Varga <vargaz at gmail.com> wrote:
>
>> Hi,
>>
>>   Committed the codegen parts of the patch with the following changes:
>> - changed the indentation etc. to conform to the mono coding conventions.
>> Sorry if this
>>   causes conflicts.
>> - Added a NACL_SIZE() macro to reduce the amount of #ifdefs a bit.
>> - Fixed a few warnings in x86-codegen.h.
>>
>>                                                      Zoltan
>>
>>
>> On Mon, Aug 2, 2010 at 8:34 PM, Zoltan Varga <vargaz at gmail.com> wrote:
>>
>>> Hi,
>>>
>>>   Those are not always mutually exclusive, they sometimes signal
>>> sub-target stuff, like
>>> TARGET_PS3 is a sub-target of TARGET_POWERPC. I'm fine with writing
>>> #ifdef <default>
>>> #elif <nacl>
>>> #endif
>>>
>>> just need to add another define for <default> but thats not a big
>>> problem.
>>>
>>>                            Zoltan
>>>
>>>
>>> On Mon, Aug 2, 2010 at 8:05 PM, Elijah Taylor <elijahtaylor at google.com>wrote:
>>>
>>>> Regarding TARGET_NACL, I'm not sure if that's quite the same as the
>>>> other TARGET_ defines.  AFAIK all of the TARGET_ defines are generally
>>>> mutually exclusive in Mono, but for NaCl we rely on TARGET_X86 or
>>>> TARGET_AMD64 to be defined along with our __native_client_codegen__ in order
>>>> to determine which architecture we're targetting (particularly because
>>>> codegen macros are shared between the two, but also because of #ifdef'd code
>>>> in aot-compiler.c, etc).  Also, if we went with TARGET_NACL we're still in
>>>> the same boat as before where we'd have code like this:
>>>>
>>>> #ifndef TARGET_NACL
>>>> // default
>>>> #else
>>>> // NaCl
>>>> #endif
>>>>
>>>>
>>>> On Mon, Aug 2, 2010 at 10:51 AM, Zoltan Varga <vargaz at gmail.com> wrote:
>>>>
>>>>> Hi,
>>>>>
>>>>> Yes, we've been trying to do that for future modifications in amd64.  I
>>>>>> can address these issues with x86 with an alternate patch if you'd like, let
>>>>>> me know.
>>>>>>
>>>>>
>>>>> I can do it before checking the changes in, when that happens.
>>>>>
>>>>>
>>>>>>
>>>>>> It looks like you're trying to optimize for as few preprocessor
>>>>>> defines as possible.  If that's the case, we'll try to carry that sentiment
>>>>>> over to our new changes too.
>>>>>>
>>>>>>
>>>>> We are trying to move away from ifdefs if possible, they lead to ugly
>>>>> looking code.
>>>>>
>>>>>
>>>>>> One thing we've been talking about locally is what to do in amd64
>>>>>> (where we're having to make more changes than in x86) when we have divergent
>>>>>> behavior between normal mono and NaCl.  Trying to be good guests in the Mono
>>>>>> codebase, we started putting the mono codeblocks first, like so:
>>>>>>
>>>>>> #if defined(__default_codegen__)
>>>>>> //normal mono behavior
>>>>>> #elif defined(__native_client_codegen__)
>>>>>> //new nacl behavior
>>>>>> #else (if needed)
>>>>>> //optional default case
>>>>>> #endif
>>>>>>
>>>>>> And then defining __default_codegen__ for non-nacl builds.  Obviously
>>>>>> this is a little messy as it adds a new define for every other build, but we
>>>>>> thought it made it clear that it is the default path, rather than using
>>>>>> "#ifndef __native_client_codegen__".  I find that it's easier to parse code
>>>>>> with a lot of ifdefs rather than a mixture of ifndefs and ifdefs.
>>>>>>
>>>>>>
>>>>> Either approach is fine by me. We already have a bunch of defines for
>>>>> the target platform, i.e.
>>>>> TARGET_X86/TARGET_AMD64, you can add a TARGET_NACL, and use that
>>>>> instead of
>>>>> __native_client_codegen__.
>>>>>
>>>>>                    Zoltan
>>>>>
>>>>>  Does anyone have any strong opinions one way or another about how we
>>>>>> do this?
>>>>>>
>>>>>>
>>>>>>                              Zoltan
>>>>>>>
>>>>>>>
>>>>>>> On Fri, Jul 16, 2010 at 1:30 AM, Elijah Taylor <
>>>>>>> elijahtaylor at google.com> wrote:
>>>>>>>
>>>>>>>> Hi, here's an updated patch with your feedback addressed.  I
>>>>>>>> re-based the diff closer to head revision (r160382) to include the other
>>>>>>>> changes of ours that already landed, as well as make sure we're still
>>>>>>>> compatible with current Mono development.
>>>>>>>>
>>>>>>>> In general this diff should have a smaller impact on the .c files:
>>>>>>>> mini-x86.c, exceptions-x86.c, tramp-x86.c specifically, and the Native
>>>>>>>> Client changes are a little more grouped together rather than spread out.
>>>>>>>>
>>>>>>>> A couple of points separate from the feedback:
>>>>>>>> 1) I fixed a bug in my implementation of genmdesc.pl changes, so
>>>>>>>> that will be different from the previous patch
>>>>>>>> 2) There's a small typo at head revision in mono/mini/tramp-x86.c
>>>>>>>> which says "rethow" instead of "reth*r*ow" for your rethrow
>>>>>>>> exception trampoline.  This is also fixed in my patch.
>>>>>>>>
>>>>>>>> As always feedback is appreciated from everyone.
>>>>>>>>
>>>>>>>>
>>>>>>>> -Elijah
>>>>>>>>
>>>>>>>>
>>>>>>>> On Tue, Jul 6, 2010 at 6:35 AM, Zoltan Varga <vargaz at gmail.com>wrote:
>>>>>>>>
>>>>>>>>> Hi,
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>> One possibility is to pad out all x86_prefix instructions to the
>>>>>>>>>> nearest 32-byte boundary, but that could really bloat things depending on
>>>>>>>>>> how often they're used.  Do you have any idea of the prefix to non-prefix
>>>>>>>>>> instruction ratio?  It seems like it'd be pretty low based on looking at the
>>>>>>>>>> code but I haven't looked at any actual metrics.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>> I think that would be ok, they are seldom used.
>>>>>>>>>
>>>>>>>>>                                  Zoltan
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.ximian.com/pipermail/mono-devel-list/attachments/20100817/c8c779c8/attachment-0001.html