[webkit-reviews] review granted: [Bug 184702] Templatize CodePtr/Refs/FunctionPtrs with PtrTags. : [Attachment 338152] proposed patch rebased.
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Apr 17 15:13:33 PDT 2018
Filip Pizlo <fpizlo at apple.com> has granted Mark Lam <mark.lam at apple.com>'s
request for review:
Bug 184702: Templatize CodePtr/Refs/FunctionPtrs with PtrTags.
https://bugs.webkit.org/show_bug.cgi?id=184702
Attachment 338152: proposed patch rebased.
https://bugs.webkit.org/attachment.cgi?id=338152&action=review
--- Comment #5 from Filip Pizlo <fpizlo at apple.com> ---
Comment on attachment 338152
--> https://bugs.webkit.org/attachment.cgi?id=338152
proposed patch rebased.
View in context: https://bugs.webkit.org/attachment.cgi?id=338152&action=review
r=me. The suggestions about using alternate tags can be addressed in another
bug.
> Source/JavaScriptCore/bytecode/CallLinkInfo.h:332
> + CodeLocationLabel<JSEntryPtrTag> m_callReturnLocationOrPatchableJump;
> + CodeLocationLabel<JSEntryPtrTag> m_hotPathBeginOrSlowPathStart;
> + CodeLocationNearCall<JSEntryPtrTag> m_hotPathOther;
I think these should be JSInternalPtrTag, since these are not being used as
targets for a JS call. They are being used to point at some patchable stuff in
the call IC.
> Source/JavaScriptCore/ftl/FTLLazySlowPath.h:88
> + CodeLocationLabel<JSEntryPtrTag> m_done;
I think this should be JSInternalPtrTag. It's not meant to point to the call
entrypoint of anything.
> Source/JavaScriptCore/jit/JITMathIC.h:67
> + CodeLocationLabel<JSEntryPtrTag> doneLocation() { return
m_inlineStart.labelAtOffset(m_inlineSize); }
> + CodeLocationLabel<JSEntryPtrTag> slowPathStartLocation() { return
m_inlineStart.labelAtOffset(m_deltaFromStartToSlowPathStart); }
> + CodeLocationCall<JSEntryPtrTag> slowPathCallLocation() { return
m_inlineStart.callAtOffset(m_deltaFromStartToSlowPathCallLocation); }
I think these should be JSInternal, since none of these are meant to call to
the JS call entrypoint of any function.
> Source/JavaScriptCore/jit/JITMathIC.h:250
> + CodeLocationLabel<JSEntryPtrTag> m_inlineStart;
ISInternal, because this does not for doing JS calls
> Source/JavaScriptCore/llint/LLIntData.cpp:78
> + JSEntryPtrTag, // llint_program_prologue
> + JSEntryPtrTag, // llint_eval_prologue
> + JSEntryPtrTag, // llint_module_program_prologue
> + JSEntryPtrTag, // llint_function_for_call_prologue
> + JSEntryPtrTag, // llint_function_for_construct_prologue
> + JSEntryPtrTag, // llint_function_for_call_arity_check
> + JSEntryPtrTag, // llint_function_for_construct_arity_check
> + JSEntryPtrTag, // llint_generic_return_point
> BytecodePtrTag, // llint_throw_from_slow_path_trampoline
> ExceptionHandlerPtrTag, // llint_throw_during_call_trampoline
> - CodePtrTag, // llint_native_call_trampoline
> - CodePtrTag, // llint_native_construct_trampoline
> - CodePtrTag, // llint_internal_function_call_trampoline
> - CodePtrTag, // llint_internal_function_construct_trampoline
> + JSEntryPtrTag, // llint_native_call_trampoline
> + JSEntryPtrTag, // llint_native_construct_trampoline
> + JSEntryPtrTag, // llint_internal_function_call_trampoline
> + JSEntryPtrTag, // llint_internal_function_construct_trampoline
It would be cool if we could give these JSInternal since these aren't meant to
be used as targets of JS calls.
More information about the webkit-reviews
mailing list