[webkit-reviews] review granted: [Bug 225397] [JSC] Abolish using resolved scope as |this| value when calling functions : [Attachment 429993] Patch

[bugzilla-daemon at webkit.org]

Filip Pizlo <fpizlo at apple.com> has granted Alexey Shvayka
<shvaikalesh at gmail.com>'s request for review:
Bug 225397: [JSC] Abolish using resolved scope as |this| value when calling
functions
https://bugs.webkit.org/show_bug.cgi?id=225397

Attachment 429993: Patch

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




--- Comment #11 from Filip Pizlo <fpizlo at apple.com> ---
Comment on attachment 429993
  --> https://bugs.webkit.org/attachment.cgi?id=429993
Patch

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

> Source/JavaScriptCore/ChangeLog:8
> +	   Reviewed by NOBODY (OOPS!).
> +
> +	   * API/APICallbackFunction.h:

Can you write some stuff about what you did?

> Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:13497
> +    addSlowPathGenerator(slowPathCall(slowCases, this, operationToObject,
valueGPR, TrustedImmPtr::weakPointer(m_graph, globalObject), valueRegs,
TrustedImmPtr(nullptr)));

What are some example programs where the slow path would be taken?

> Source/JavaScriptCore/jit/JITOpcodes.cpp:2097
> +void JIT::emit_op_get_this_value_for_call(const Instruction*
currentInstruction)
> +{
> +    auto bytecode = currentInstruction->as<OpGetThisValueForCall>();
> +#if USE(JSVALUE64)
> +    JSValueRegs valueRegs(regT0);
> +#else
> +    JSValueRegs valueRegs(regT1, regT0);
> +#endif
> +    emitGetVirtualRegister(bytecode.m_scope, valueRegs);
> +
> +    Jump isWithScope = branchIfNotType(valueRegs.payloadGPR(), JSTypeRange {
FirstJSScopeType, LastJSScopeType });
> +    moveTrustedValue(jsUndefined(), valueRegs);
> +    isWithScope.link(this);
> +
> +    emitPutVirtualRegister(bytecode.m_dst, valueRegs);
> +}

I really like this.