[Webkit-unassigned] [Bug 198604] Refactoring of architectural Register Information

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=198604

Keith Miller <keith_miller at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #373008|review?                     |review-
              Flags|                            |

--- Comment #60 from Keith Miller <keith_miller at apple.com> ---
Comment on attachment 373008
  --> https://bugs.webkit.org/attachment.cgi?id=373008
Post-5 Review Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=373008&action=review

Sorry about the delay. I thought this had already been review and committed for some reason... :/

I have a couple more style changes for you but I think it's almost there! Thanks again for the patch.

> Source/JavaScriptCore/assembler/RegisterInfo.h:59
> +#  include "X86Registers.h"
> +#elif CPU(X86_64)
> +#  include "X86_64Registers.h"
> +#elif CPU(MIPS)
> +#  include "MIPSRegisters.h"
> +#elif CPU(ARM_THUMB2)
> +#  include "ARMv7Registers.h"
> +#elif CPU(ARM64)
> +#  include "ARM64Registers.h"

These includes should not have spaces between the # and the "include"

> Source/JavaScriptCore/assembler/X86_64Registers.h:92
> +    macro(xmm10,"xmm10", 0, 0)                  \
> +    macro(xmm11,"xmm11", 0, 0)                  \
> +    macro(xmm12,"xmm12", 0, 0)                  \
> +    macro(xmm13,"xmm13", 0, 0)                  \
> +    macro(xmm14,"xmm14", 0, 0)                  \
> +    macro(xmm15,"xmm15", 0, 0)

Can you add a space after these register identifiers? That would match webkit style.

> Source/JavaScriptCore/jit/RegisterSet.cpp:52
> +    if (isReserved) result.set(RegisterNames::id);

I think the body of the if should be on its own line and be indented.

> Source/JavaScriptCore/jit/RegisterSet.cpp:116
> +    result.set(RegisterNames::id);

Can you fix the indentation here?

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20190701/1f2e0c0c/attachment-0001.html>