[webkit-reviews] review granted: [Bug 171269] [JSC] Math unary functions should be handled by DFG : [Attachment 308410] Patch

[bugzilla-daemon at webkit.org]

Saam Barati <sbarati at apple.com> has granted Yusuke Suzuki
<utatane.tea at gmail.com>'s request for review:
Bug 171269: [JSC] Math unary functions should be handled by DFG
https://bugs.webkit.org/show_bug.cgi?id=171269

Attachment 308410: Patch

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




--- Comment #19 from Saam Barati <sbarati at apple.com> ---
Comment on attachment 308410
  --> https://bugs.webkit.org/attachment.cgi?id=308410
Patch

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

> Source/JavaScriptCore/ChangeLog:12
> +	   optimization phase. Actually, ArithLog is effective in kraken.

what do you mean by effective here? We effectively optimize it in Kraken?

> Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:2186
> +	   if (argumentCountIncludingThis == 1) {
> +	       insertChecks();
> +	       set(VirtualRegister(resultOperand), addToGraph(JSConstant,
OpInfo(m_constantNaN)));
> +	       return true;
> +	   }

Do we have tests for all of these?

> Source/JavaScriptCore/dfg/DFGOperations.cpp:406
> +    double a = op1.toNumber(exec); \

nit: Lets pick a better name than "a".

> Source/JavaScriptCore/runtime/MathCommon.cpp:528
> +    if (!value)

Nit: the older version had ==0 here, I think that's more readable.