[webkit-reviews] review granted: [Bug 237624] Rename LLInt macros using return to destination : [Attachment 454168] Patch

[bugzilla-daemon at webkit.org]

Mark Lam <mark.lam at apple.com> has granted Keith Miller
<keith_miller at apple.com>'s request for review:
Bug 237624: Rename LLInt macros using return to destination
https://bugs.webkit.org/show_bug.cgi?id=237624

Attachment 454168: Patch

https://bugs.webkit.org/attachment.cgi?id=454168&action=review




--- Comment #9 from Mark Lam <mark.lam at apple.com> ---
Comment on attachment 454168
  --> https://bugs.webkit.org/attachment.cgi?id=454168
Patch

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

r=me.  This patch is huge, and it's very difficult to review this by
eyeballing, and be confident that it was done right.  It's too easy to miss
something, and perhaps introduce a subtle change.  Just for a sanity check (if
you haven't already done so), please generate a LLIntAssembly.h before and
after your patch, and diff the 2 LLIntAssembly.h files.  If there are no
errors, they should be identical.

> Source/JavaScriptCore/llint/LowLevelInterpreter.asm:529
>      llintOpWithMetadata(opcodeName, opcodeStruct, macro(size, get, dispatch,
metadata, return)

/return/setDestinationAndDispatch/ to be consistent.

> Source/JavaScriptCore/llint/LowLevelInterpreter64.asm:2557
> +	       # prepareCall in LLInt does not untag setDestinationAndDispatch
address. So we need to untag that in the trampoline separately.

/setDestinationAndDispatch address/the return address/

> Source/JavaScriptCore/llint/LowLevelInterpreter64.asm:2795
> -    macro returnConstantScope()
> +    macro setDestinationAndDispatchConstantScope()

In this case, `returnConstantScope` would be a better name if not for the fact
that there's no actual "return" operation.  How about
`setDestinationAndDispatchWithConstantScope` instead? 
`setDestinationAndDispatchConstantScope` just doesn't read like English.