Bug 167736 – [JSC] Add operationToInt32SensibleSlow to optimize kraken pbkdf2 and sha256

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Yusuke Suzuki]

[JSC] Add operationToInt32Fallback to optimize kraken pbkdf2

[Yusuke Suzuki]

Created attachment 300414 [details]
Patch

WIP

[Yusuke Suzuki]

Still considering about the edge cases.

[Yusuke Suzuki]

CAUTION: Currently, this is only valid in x64 (sensible double to int32 case in DFG and FTL). And still I'm now considering about edge cases.
BTW, the latest ARM64 has super fast double to int32 operation. If this instruction becomes available, we can optimize this in ARM64.

[Yusuke Suzuki]

Pf course, later, we should make DFG prediction better not to emit ValueToInt32 in such a case.
But I think optimizing this sensitive double to int32 is worth doing.

Note that kraken pbkdf2 consumes 1/3 time for operationToInt32. ;)

[Yusuke Suzuki]

This also improves kraken sha256 iterative.

[Yusuke Suzuki]

Comment on attachment 300414 [details]
Patch

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

> Source/JavaScriptCore/runtime/MathCommon.cpp:481
> +    // If exponent < 0 there will be no bits to the left of the decimal point
> +    // after rounding; if the exponent is > 83 then no bits of precision can be
> +    // left in the low 32-bit range of the result (IEEE-754 doubles have 52 bits
> +    // of fractional precision).
> +    // Note this case handles 0, -0, and all infinite, NaN, & denormal value.
> +    if (exp < 0 || exp > 83)
> +        return 0;

And I think this check is also unnecessary.
1. If exp < 0, truncate should succeed.
2. If exp > 83 (exp >= 84), the following code produces 0 as a result. (exp > 52, so static_cast<int32_t>(bits << (84 - 52)) -> 0)

[Yusuke Suzuki]

Created attachment 300518 [details]
Patch

[Saam Barati]

Comment on attachment 300518 [details]
Patch

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

r=me

> Source/JavaScriptCore/ChangeLog:63
> +            1ff:   66 48 0f 7e c0          movq   %xmm0,%rax

I wonder why this doesn't do a mov rdx, rax

> Source/JavaScriptCore/runtime/MathCommon.cpp:470
> +int32_t JIT_OPERATION operationToInt32SensibleSlow(double number)

IEE754 is fun

[Yusuke Suzuki]

Comment on attachment 300518 [details]
Patch

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

>> Source/JavaScriptCore/ChangeLog:63
>> +            1ff:   66 48 0f 7e c0          movq   %xmm0,%rax
> 
> I wonder why this doesn't do a mov rdx, rax

Yeah, that's storage. But it's due to the GCC6's decision.

>> Source/JavaScriptCore/runtime/MathCommon.cpp:470
>> +int32_t JIT_OPERATION operationToInt32SensibleSlow(double number)
> 
> IEE754 is fun

Yeah :)

[Yusuke Suzuki]

Committed r211670: <http://trac.webkit.org/changeset/211670>

[Saam Barati]

I've confirmed that this is a 1-1.5% progression on Mac Kraken.

[Radar WebKit Bug Importer]

<rdar://problem/30364482>