[Mono-dev] Proposed Patch - Google Native Client

Zoltan Varga vargaz at gmail.com
Mon Aug 9 11:46:23 EDT 2010


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/20100809/48eebbba/attachment.html 


More information about the Mono-devel-list mailing list