[webkit-reviews] review granted: [Bug 217362] [JSC] More consistent PtrTagging for code types : [Attachment 410612] Patch

[bugzilla-daemon at webkit.org]

Mark Lam <mark.lam at apple.com> has granted Yusuke Suzuki <ysuzuki at apple.com>'s
request for review:
Bug 217362: [JSC] More consistent PtrTagging for code types
https://bugs.webkit.org/show_bug.cgi?id=217362

Attachment 410612: Patch

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




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

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

r=me with fixes.

> Source/JavaScriptCore/llint/LLIntExceptions.cpp:76
> +MacroAssemblerCodeRef<ExceptionHandlerPtrTag> catcher(OpcodeSize size)

Can we call this handleCatch (verb) instead of catcher (noun)?

> Source/JavaScriptCore/llint/LLIntExceptions.cpp:80
> +	   return catcherThunk(size);

Ditto: handleCatchThunk?

> Source/JavaScriptCore/llint/LLIntThunks.cpp:152
> +	      
codeRef.construct(generateThunkWithJumpTo<JITThunkPtrTag>(wasm_function_prologu
e, "function for call"));

Is there any reason this shouldn't also be using JSEntryPtrTag (and obsoleting
JITThunkPtrTag)?  Looks like this is the last use of it.  We used to use
JITThunkPtrTag for certain pointers based on where they point to.  Now, you've
changed this to based on where the pointers will be used.  Based on where the
pointer is used, it sounds like wasm_function_prologue should be using
JSEntryPtrTag too.

You don't have to do this in this patch, but maybe it warrants a look later.