[Mono-dev] Proposed Patch - Google Native Client
vargaz at gmail.com
Mon Aug 2 08:00:21 EDT 2010
Some further comments:
- it would be nice to follow the mono coding conventions for the changes,
put a space before '(' in calls, use tabs in files where tabs are used for
- some of the #ifdefs could be avoided by adding a NACL_SIZE() macro:
#if defined(__native_client_codegen__) || defined(__native_client__)
#define NACL_SIZE(a, b) (b)
#define NACL_SIZE(a, b) (a)
and using it for things like the trampoline buffer sizes:
+ tramp_size = 128;
tramp_size = 64;
tramp_size = NACL_SIZE (64, 128);
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.
> On Tue, Jul 6, 2010 at 6:35 AM, Zoltan Varga <vargaz at gmail.com> wrote:
>>> 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.
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the Mono-devel-list