Bug 151680 – Snippefy bitwise operators for the baseline JIT.

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Mark Lam]

Working on it.

[Radar WebKit Bug Importer]

<rdar://problem/23696526>

[Mark Lam]

Created attachment 266558 [details]
proposed patch.

Tests and benchmarks are still being run.

[WebKit Commit Bot]

Attachment 266558 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/jit/JITBitOrGenerator.h:39:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/jit/JITBitXorGenerator.h:39:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/jit/JITBitwiseBinaryOpGenerator.h:40:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/jit/JITBitwiseBinaryOpGenerator.h:41:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/jit/JITBitwiseBinaryOpGenerator.h:42:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/jit/JITBitwiseBinaryOpGenerator.h:43:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/jit/JITBitwiseBinaryOpGenerator.h:44:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/jit/JITBitwiseBinaryOpGenerator.h:45:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/jit/JITBitAndGenerator.h:39:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
Total errors found: 9 in 19 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Geoffrey Garen]

Comment on attachment 266558 [details]
proposed patch.

r=me

[Mark Lam]

This patch has passed the JSC tests on x86 and x86_64.  It has passed the layout tests on x86_64.

Benchmarks with the DFG and FTL enable shows perf to be neutral for both x86 and x86_64.  Still waiting for benchmark results with no DFG.

[Mark Lam]

Created attachment 266651 [details]
patch 2: fixed 32-bit regression in baseline JIT

The only difference in this patch is the addition of this conditional in emitBitwiseBinaryOpFastPath():

#if USE(JSVALUE32_64)
        emitStoreInt32(result, resultRegs.payloadGPR(), op1 == result || op2 == result);
#else
        emitPutVirtualRegister(result, resultRegs);
#endif

On x86, the emitStoreInt32() results in setting the result IntTag value using an immediate instead of getting it from another register which is already known to have the IntTag value.  The 32-bit bitwise code was previous setting the result this way.  This change basically restores the setting of the result to the way it was done before.

This difference accounted for some noticeable perf difference on x86 when running without the DFG.

[Mark Lam]

Created attachment 266652 [details]
x86_64 benchmark result.

[WebKit Commit Bot]

Attachment 266651 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/jit/JITBitOrGenerator.h:39:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/jit/JITBitXorGenerator.h:39:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/jit/JITBitwiseBinaryOpGenerator.h:40:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/jit/JITBitwiseBinaryOpGenerator.h:41:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/jit/JITBitwiseBinaryOpGenerator.h:42:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/jit/JITBitwiseBinaryOpGenerator.h:43:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/jit/JITBitwiseBinaryOpGenerator.h:44:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/jit/JITBitwiseBinaryOpGenerator.h:45:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/jit/JITBitAndGenerator.h:39:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
Total errors found: 9 in 19 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Mark Lam]

Created attachment 266653 [details]
x86 benchmark result.

[Mark Lam]

Created attachment 266654 [details]
x86_64 benchmark result without DFG.

[Mark Lam]

Created attachment 266655 [details]
x86 benchmark result without DFG.

[Mark Lam]

Perf is neutral with the DFG on.  Perf is also mostly neutral with just the baseline JIT.  

On x86_64 baseline only, AsmBench tests shows some progression possibly because the snippet forms of bitor and bitxor has added support for constant operands (inherited from the 32-bit version).

On x86 baseline only, JSRegress' is-function-fold does appear to have a regression.  However, inspection of the test code shows that it doesn't even use bitand, bitor, or bitxor operators.  A dump of the disassemble JIT code also shows no use of the operators.  I'll count this one as being due to cache line effects, and ignore it.

[Geoffrey Garen]

Comment on attachment 266651 [details]
patch 2: fixed 32-bit regression in baseline JIT

r=me

[Mark Lam]

Thanks for the review.  Landed in r193471: <http://trac.webkit.org/r193471>.