[Mono-dev] Proposed Patch - Google Native Client

Zoltan Varga vargaz at gmail.com
Tue Aug 17 01:32:24 EDT 2010


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/12a41116/attachment-0001.html 


More information about the Mono-devel-list mailing list