Bug 156072

Summary: [JSC] Add truncate operation (rounding to zero)
Product: WebKit Reporter: Yusuke Suzuki <ysuzuki>
Component: JavaScriptCoreAssignee: Yusuke Suzuki <ysuzuki>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, keith_miller, mark.lam, msaboff, ossy, saam
Priority: P2    
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

Description Yusuke Suzuki 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.
Comment 1 Yusuke Suzuki 2016-03-31 11:15:13 PDT
Let's use B3 Patchpoint instead of adding new B3 / Air IR!
Comment 2 Yusuke Suzuki 2016-03-31 14:46:30 PDT
Created attachment 275331 [details]
Patch

WIP
Comment 3 Yusuke Suzuki 2016-03-31 15:59:08 PDT
Created attachment 275347 [details]
Patch

WIP2
Comment 4 Yusuke Suzuki 2016-04-01 04:59:42 PDT
Created attachment 275394 [details]
Patch
Comment 5 Yusuke Suzuki 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
Comment 6 WebKit Commit Bot 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.
Comment 7 Yusuke Suzuki 2016-04-02 00:34:11 PDT
Created attachment 275462 [details]
Patch
Comment 8 WebKit Commit Bot 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.
Comment 9 Yusuke Suzuki 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
Comment 10 Yusuke Suzuki 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.
Comment 11 Yusuke Suzuki 2016-04-03 00:45:17 PDT
Comment on attachment 275462 [details]
Patch

thanks
Comment 12 Yusuke Suzuki 2016-04-03 00:45:29 PDT
thanks
Comment 13 WebKit Commit Bot 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>
Comment 14 WebKit Commit Bot 2016-04-03 01:37:26 PDT
All reviewed patches have been landed.  Closing bug.
Comment 15 Csaba Osztrogonác 2016-04-04 01:53:35 PDT
(In reply to comment #13)
> Committed r198981: <http://trac.webkit.org/changeset/198981>

... and the ARM buildfix landed in http://trac.webkit.org/changeset/198999
Comment 16 Yusuke Suzuki 2016-04-04 02:07:35 PDT
(In reply to comment #15)
> (In reply to comment #13)
> > Committed r198981: <http://trac.webkit.org/changeset/198981>
> 
> ... and the ARM buildfix landed in http://trac.webkit.org/changeset/198999

Oops, thanks!