Bug 156072

Summary:

[JSC] Add truncate operation (rounding to zero)

Product:

WebKit

Reporter:

Yusuke Suzuki <ysuzuki>

Component:

JavaScriptCore

Assignee:

Yusuke Suzuki <ysuzuki>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

commit-queue, keith_miller, mark.lam, msaboff, ossy, saam

Priority:

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

156160

Bug Blocks:

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none
Patch	none

Yusuke Suzuki

Reported 2016-03-31 11:06:38 PDT

Add Math.trunc's TruncIntrinsic and DFG ArithTrunc. And use @trunc in toInteger. (This is the same in JSValue::toInteger) Currently, even if we know the given value is Int32, due to @abs and negation, we always emit some code. But if we add it, we can convert them to Identity in DFG Fixup phase.

Attachments
Patch (48.22 KB, patch) 2016-03-31 14:46 PDT, Yusuke Suzuki	no flags	Details Formatted Diff Diff
Patch (52.91 KB, patch) 2016-03-31 15:59 PDT, Yusuke Suzuki	no flags	Details Formatted Diff Diff
Patch (54.69 KB, patch) 2016-04-01 04:59 PDT, Yusuke Suzuki	no flags	Details Formatted Diff Diff
Patch (54.96 KB, patch) 2016-04-02 00:34 PDT, Yusuke Suzuki	no flags	Details Formatted Diff Diff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

Yusuke Suzuki

Comment 1 2016-03-31 11:15:13 PDT

Let's use B3 Patchpoint instead of adding new B3 / Air IR!

Yusuke Suzuki

Comment 2 2016-03-31 14:46:30 PDT

Created attachment 275331 [details] Patch WIP

Yusuke Suzuki

Comment 3 2016-03-31 15:59:08 PDT

Created attachment 275347 [details] Patch WIP2

Yusuke Suzuki

Comment 4 2016-04-01 04:59:42 PDT

Created attachment 275394 [details] Patch

Yusuke Suzuki

Comment 5 2016-04-01 05:01:24 PDT

Added micro benchmarks that use trunc. 92/92 Generating benchmark report at /Users/yusukesuzuki/dev/WebKit/Baseline_Trunc_JSRegress_192-168-22-121_20160401_2101_report.txt And raw data at /Users/yusukesuzuki/dev/WebKit/Baseline_Trunc_JSRegress_192-168-22-121_20160401_2101.json Benchmark report for JSRegress on 192-168-22-121 (MacBookPro10,1). VMs tested: "Baseline" at /Users/yusukesuzuki/dev/WebKit/WebKitBuild/trunc-master/Release/jsc "Trunc" at /Users/yusukesuzuki/dev/WebKit/WebKitBuild/trunc/Release/jsc Collected 10 samples per benchmark/VM, with 10 VM invocations per benchmark. Emitted a call to gc() between sample measurements. Used 1 benchmark iteration per VM invocation for warm-up. Used the jsc-specific preciseTime() function to get microsecond-level timing. Reporting benchmark execution times with 95% confidence intervals in milliseconds. Baseline Trunc many-foreach-calls 22.2353+-1.0961 ^ 18.1958+-0.3896 ^ definitely 1.2220x faster math-trunc 42.7834+-0.9172 ^ 23.8203+-1.0271 ^ definitely 1.7961x faster <geometric> 30.8220+-0.7449 ^ 20.8044+-0.3740 ^ definitely 1.4815x faster

WebKit Commit Bot

Comment 6 2016-04-01 05:02:18 PDT

Attachment 275394 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/ftl/FTLOutput.cpp:155: Place brace on its own line for function definitions. [whitespace/braces] [4] Total errors found: 1 in 45 files If any of these errors are false positives, please file a bug against check-webkit-style.

Yusuke Suzuki

Comment 7 2016-04-02 00:34:11 PDT

Created attachment 275462 [details] Patch

WebKit Commit Bot

Comment 8 2016-04-02 00:35:30 PDT

Attachment 275462 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/ftl/FTLOutput.cpp:155: Place brace on its own line for function definitions. [whitespace/braces] [4] Total errors found: 1 in 45 files If any of these errors are false positives, please file a bug against check-webkit-style.

Yusuke Suzuki

Comment 9 2016-04-02 00:38:39 PDT

Related benchmark results. Benchmark report for JSRegress on dandelion (MacBookPro10,1). VMs tested: "Baseline" at /Users/yusukesuzuki/dev/WebKit/WebKitBuild/trunc-master/Release/jsc "Trunc" at /Users/yusukesuzuki/dev/WebKit/WebKitBuild/trunc/Release/jsc Collected 20 samples per benchmark/VM, with 20 VM invocations per benchmark. Emitted a call to gc() between sample measurements. Used 1 benchmark iteration per VM invocation for warm-up. Used the jsc-specific preciseTime() function to get microsecond-level timing. Reporting benchmark execution times with 95% confidence intervals in milliseconds. Baseline Trunc array-prototype-every 81.9896+-1.8672 ^ 77.5264+-1.0458 ^ definitely 1.0576x faster array-prototype-forEach 78.7342+-0.7214 77.1840+-0.9078 might be 1.0201x faster array-prototype-map 80.3581+-0.6846 ^ 77.7265+-0.6281 ^ definitely 1.0339x faster array-prototype-reduce 74.9866+-0.6510 ? 75.3615+-0.7255 ? array-prototype-reduceRight 92.0309+-0.7503 ^ 89.1449+-0.6684 ^ definitely 1.0324x faster array-prototype-some 80.5732+-0.8079 ^ 76.9317+-0.5362 ^ definitely 1.0473x faster many-foreach-calls 22.7338+-1.0990 ^ 18.6803+-0.6081 ^ definitely 1.2170x faster math-trunc 8.8024+-0.2148 ^ 6.2823+-0.1497 ^ definitely 1.4011x faster string-repeat-arith 26.1573+-0.7163 25.6345+-0.4013 might be 1.0204x faster string-repeat-not-resolving-fixed 3.6999+-0.1178 3.5125+-0.0818 might be 1.0534x faster string-repeat-not-resolving-no-inline 5.4666+-0.1887 5.3437+-0.1892 might be 1.0230x faster string-repeat-not-resolving 5.5505+-0.2369 ^ 4.8133+-0.1615 ^ definitely 1.1532x faster string-repeat-resolving-fixed 26.5824+-0.4180 26.3695+-0.3880 string-repeat-resolving-no-inline 34.5224+-0.6110 ? 34.9351+-0.5973 ? might be 1.0120x slower string-repeat-resolving 34.1329+-0.4977 34.0148+-0.3377 string-repeat-single-not-resolving 9.6244+-0.4989 ? 9.7663+-0.6325 ? might be 1.0147x slower string-repeat-single-resolving 9.6858+-0.4403 ? 9.7531+-0.6080 ? string-repeat-small-not-resolving 4.2266+-0.1217 4.0773+-0.1245 might be 1.0366x faster string-repeat-small-resolving 34.8371+-0.6112 ? 34.9999+-0.4296 ? <geometric> 22.9358+-0.1689 ^ 21.8019+-0.1372 ^ definitely 1.0520x faster

Yusuke Suzuki

Comment 10 2016-04-02 00:45:21 PDT

Comment on attachment 275462 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=275462&action=review Add comments. > Source/JavaScriptCore/dfg/DFGFixupPhase.cpp:369 > + case ArithTrunc: { This handling can eliminate @trunc in @toInteger in almost all the cases. > Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4513 > + return; Currently, we take relatively simple solution. An alternative implementation for Int32 results is using branchTruncateDoubleToInt32. But in such implementation, we should take care about INT32_MIN case (for x86 case). So, as a first step, now we just use the round instruction here. > Source/JavaScriptCore/ftl/FTLOutput.cpp:160 > + } Implement Trunc IR as a Patchpoint.

Yusuke Suzuki

Comment 11 2016-04-03 00:45:17 PDT

Comment on attachment 275462 [details] Patch thanks

Yusuke Suzuki

Comment 12 2016-04-03 00:45:29 PDT

thanks

WebKit Commit Bot

Comment 13 2016-04-03 01:37:23 PDT

Comment on attachment 275462 [details] Patch Clearing flags on attachment: 275462 Committed r198981: <http://trac.webkit.org/changeset/198981>

WebKit Commit Bot

Comment 14 2016-04-03 01:37:26 PDT

All reviewed patches have been landed. Closing bug.