[Mono-dev] Proposed Patch - Google Native Client

[Zoltan Varga]

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/20100802/c6be6277/attachment-0001.html