[webkit-reviews] review granted: [Bug 213191] Add DFG/FTL fast path for GetPrototypeOf based on OverridesGetPrototype flag : [Attachment 402422] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Jun 23 20:04:28 PDT 2020


Yusuke Suzuki <ysuzuki at apple.com> has granted Alexey Shvayka
<shvaikalesh at gmail.com>'s request for review:
Bug 213191: Add DFG/FTL fast path for GetPrototypeOf based on
OverridesGetPrototype flag
https://bugs.webkit.org/show_bug.cgi?id=213191

Attachment 402422: Patch

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




--- Comment #3 from Yusuke Suzuki <ysuzuki at apple.com> ---
Comment on attachment 402422
  --> https://bugs.webkit.org/attachment.cgi?id=402422
Patch

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

r=me

> Source/JavaScriptCore/bytecode/AccessCase.cpp:1287
> +	   jit.emitLoadPrototype(vm, valueGPR, scratch2GPR, scratchGPR,
failAndIgnore);

#if USE(JSVALUE64)
JSValueRegs resultRegs(scratch2GPR);
#else
JSValueRegs resultRegs(scratchGPR, scratch2GPR);
#endif

jit.emitLoadPrototype(vm, valueGPR, resultRegs, scratchGPR, failAndIgnore);

> Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:13952
> +	   m_jit.emitLoadPrototype(vm(), objectGPR, tempGPR, temp2GPR,
slowCases);

m_jit.emitLoadPrototype(vm(), objectGPR, resultRegs, temp2GPR, slowCases);

> Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:13969
> +	   m_jit.emitLoadPrototype(vm(), valueGPR, tempGPR, temp2GPR,
slowCases);

m_jit.emitLoadPrototype(vm(), valueGPR, resultRegs, temp2GPR, slowCases);

> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:4450
> +	       m_out.appendTo(slowPath, continuation);

m_out.appendTo(slowPath, fastPath);

> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:4470
> +	       m_out.appendTo(isObjectPath, continuation);

m_out.appendTo(isObjectPath, slowPath);

> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:4478
> +	       m_out.appendTo(slowPath, continuation);

m_out.appendTo(slowPath, fastPath);

> Source/JavaScriptCore/jit/AssemblyHelpers.cpp:395
> +void AssemblyHelpers::emitLoadPrototype(VM& vm, RegisterID source,
RegisterID dest, RegisterID scratch, JumpList& slowPath)

Let's accept `GPRReg objectGPR, JSValueRegs resultRegs, GPRReg scratchGPR`
instead.
Changing the meaning of scratch to part of resultRegs inside function looks
weird. We should get JSValueRegs resultRegs in this function's parameter.
And let's note that scratch can be a resultRegs.tagGPR().
You can insert,

ASSERT(resultRegs.payloadGPR() != objectGPR);
ASSERT(resultRegs.payloadGPR() != scratchGPR);
ASSERT(objectGPR != scratchGPR);

> Source/JavaScriptCore/jit/AssemblyHelpers.cpp:397
> +    emitLoadStructure(vm, source, dest, scratch);

emitLoadStructure(vm, objectGPR, resultRegs.payloadGPR(), scratchGPR);

> Source/JavaScriptCore/jit/AssemblyHelpers.cpp:408
> +#if USE(JSVALUE64)
> +    JSValueRegs resultRegs(dest);
> +#else
> +    JSValueRegs resultRegs(scratch, dest);
> +#endif

This is not necessary.

> Source/JavaScriptCore/jit/AssemblyHelpers.cpp:410
> +    loadValue(MacroAssembler::Address(dest, Structure::prototypeOffset()),
resultRegs);

loadValue(MacroAssembler::Address(resultRegs.payloadGPR(),
Structure::prototypeOffset()), resultRegs);

> Source/JavaScriptCore/jit/AssemblyHelpers.cpp:412
> +    loadValue(MacroAssembler::Address(source,
offsetRelativeToBase(knownPolyProtoOffset)), resultRegs);

loadValue(MacroAssembler::Address(objectGPR,
offsetRelativeToBase(knownPolyProtoOffset)), resultRegs);

> Source/JavaScriptCore/jit/JITOpcodes.cpp:1753
> +    JSValueRegs resultRegs(regT2);

Add
GPRReg scratchGPR = regT3;

> Source/JavaScriptCore/jit/JITOpcodes.cpp:1756
> +    JSValueRegs resultRegs(regT3, regT2);

Add
GPRReg scratchGPR = regT1;
ASSERT(valueRegs.tagGPR() == scratchGPR);

> Source/JavaScriptCore/jit/JITOpcodes.cpp:1762
> +    slowCases.append(branchIfNotObject(regT0));

Let's use valueRegs.payloadGPR().

> Source/JavaScriptCore/jit/JITOpcodes.cpp:1764
> +    emitLoadPrototype(vm(), regT0, regT2, regT3, slowCases);

Let's use it to something like this.

`
emitLoadPrototype(vm(), valueRegs.payloadGPR(), resultRegs, scratchGPR,
slowCases);
`


More information about the webkit-reviews mailing list