[webkit-reviews] review granted: [Bug 226755] Optimize compareStrictEq when neither side is a double and at least one is not a BigInt : [Attachment 435649] Patch

[bugzilla-daemon at webkit.org]

Yusuke Suzuki <ysuzuki at apple.com> has granted Robin Morisset
<rmorisset at apple.com>'s request for review:
Bug 226755: Optimize compareStrictEq when neither side is a double and at least
one is not a BigInt
https://bugs.webkit.org/show_bug.cgi?id=226755

Attachment 435649: Patch

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




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

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

r=me with comments.

> Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:7470
> +void SpeculativeJIT::emitBitwiseJSValueEquality(JSValueOperand& left,
JSValueOperand& right, GPRTemporary& result)

Let's just take `JSValueRegs left` and `JSValueRegs right` instead of
`JSValueOperand&`. And let's just take GPRReg for result.

> Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:7482
> +void SpeculativeJIT::emitBranchOnBitwiseJSValueEquality(JSValueOperand&
left, JSValueOperand& right, BasicBlock* taken, BasicBlock* notTaken)

Ditto.

> Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:7523
> +    emitBitwiseJSValueEquality(left, right, result);

Let's just pass leftRegs, rightRegs, and resultGPR.

> Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:7543
> +    emitBranchOnBitwiseJSValueEquality(left, right, taken, notTaken);

Lef's just pass leftRegs and rightRegs.

> Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp:356
> +    GPRReg leftGPR = leftRegs.payloadGPR();
> +    GPRReg rightGPR = rightRegs.payloadGPR();

I think using `leftRegs.payloadGPR()` / `rightRegs.payloadGPR()` is easier to
read since we do not need to remember leftGPR is a part of leftRegs.

> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:9749
> +	   m_out.branch(isCell(leftValue,
provenType(neitherDoubleNorHeapBigIntEdge)), unsure(leftIsCellEqualCase),
unsure(returnTrueBlock));

We can remove Numbers from provenType.
(provenType(neitherDoubleNorHeapBigIntEdge) & ~SpecFullNumber)

> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:9763
> +	   m_out.branch(isNotCell(leftValue, leftValueType),
unsure(continuation), unsure(leftIsCell));

We can remove doubles from the provenType. 
(provenType(neitherDoubleNorHeapBigIntEdge) & ~SpecFullDouble)

> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:9767
> +	   m_out.branch(isNotString(leftValue, leftValueType),
unsure(continuation), unsure(leftIsString));

We can remove non-cell and HeapBigInt from provenType. 
(provenType(neitherDoubleNorHeapBigIntEdge) & SpecCell & ~SpecHeapBigInt).

> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:9770
> +	   m_out.branch(isNotCell(rightValue, rightValueType),
unsure(continuation), unsure(rightIsCell));

We can remove doubles from the provenType. (provenType(notDoubleEdge) &
~SpecFullDouble).

> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:9773
> +	   m_out.branch(isNotString(rightValue, rightValueType),
unsure(continuation), unsure(rightIsString));

We can remove non-cells from the provenType. (provenType(notDoubleEdge) &
SpecCell).

> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:18876
> +	   m_out.branch(isCell(value, provenType(edge)), unsure(isCellBlock),
unsure(continuation));

We can remove Int32 (but we can also remove Numbers) from the provenType. 
(provenType(edge) & ~SpecFullNumber)