[webkit-reviews] review denied: [Bug 47121] need way to measure size of JITed ARM code : [Attachment 69946] conditionally include ARM assembly dump via CPU(ARM_THUMB2)

[bugzilla-daemon at webkit.org]

Geoffrey Garen <ggaren at apple.com> has denied David Goodwin
<david_goodwin at apple.com>'s request for review:
Bug 47121: need way to measure size of JITed ARM code
https://bugs.webkit.org/show_bug.cgi?id=47121

Attachment 69946: conditionally include ARM assembly dump via CPU(ARM_THUMB2)
https://bugs.webkit.org/attachment.cgi?id=69946&action=review

------- Additional Comments from Geoffrey Garen <ggaren at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=69946&action=review

Great patch, but I once again have some minutia about WebKit's coding style.

Here's the full list of guidelines for future reference:
http://webkit.org/coding/coding-style.html.

> JavaScriptCore/assembler/LinkBuffer.h:289
> +    static void dumpLinkStats(void *code, size_t initialSize, size_t
finalSize)

Should be "void* code".

> JavaScriptCore/assembler/LinkBuffer.h:298
> +	   printf("link %p: orig %u, compact %u (delta %u, %.2f%%)\n", 
> +		  code, (unsigned)initialSize, (unsigned)finalSize,
(unsigned)(initialSize - finalSize),
> +		  100.0 * (float)(initialSize - finalSize) / initialSize);

Should be static_cast<x> instead of (x).

> JavaScriptCore/assembler/LinkBuffer.h:301
> +	   printf("\ttotal %u: orig %u, compact %u (delta %u, %.2f%%)\n", 
> +		  linkCount, totalInitialSize, totalFinalSize, totalInitialSize
- totalFinalSize,
> +		  100.0 * (float)(totalInitialSize - totalFinalSize) /
totalInitialSize);

Ditto.

> JavaScriptCore/assembler/LinkBuffer.h:306
> +    static void dumpCode(void *code, size_t size)

Should be "void* code".

> JavaScriptCore/assembler/LinkBuffer.h:314
> +	   unsigned short *tcode = (unsigned short *)code;

Should be "unsigned short* tcode" and "static_cast<unsigned short*>(code)".