Bug 125519 - Make the different flavors of integer arithmetic more explicit, and don't rely on (possibly stale) results of the backwards propagator to decide integer arithmetic semantics
Summary: Make the different flavors of integer arithmetic more explicit, and don't rel...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Filip Pizlo
URL:
Keywords:
Depends on: 125523
Blocks: 125433
  Show dependency treegraph
 
Reported: 2013-12-10 09:08 PST by Filip Pizlo
Modified: 2014-01-06 20:49 PST (History)
12 users (show)

See Also:


Attachments
work in progress (38.00 KB, patch)
2014-01-03 11:22 PST, Filip Pizlo
no flags Details | Formatted Diff | Diff
more correcter (53.68 KB, patch)
2014-01-03 20:12 PST, Filip Pizlo
no flags Details | Formatted Diff | Diff
more (63.12 KB, patch)
2014-01-06 13:47 PST, Filip Pizlo
no flags Details | Formatted Diff | Diff
the patch (71.09 KB, patch)
2014-01-06 16:00 PST, Filip Pizlo
ggaren: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Filip Pizlo 2013-12-10 09:08:29 PST
For example, the DFG works with the following kinds of addition:

- Not type-inferred addition-or-strcat.  This is represented as a ValueAdd prior to FixupPhase and as a ValueAdd with UntypedUse after FixupPhase.

- Not yet type-inferred addition.  This is represented as an ArithAdd prior to FixupPhase and doesn't exist after FixupPhase.

- Double addition.  ValueAdd or ArithAdd with NumberUses.

- Integer addition with overflow check.  ValueAdd or ArithAdd with Int32Use and !bytecodeCanTruncateInteger(node->arithModeFlags())

- Integer addition without overflow check.  ValueAdd or ArithAdd with Int32Use and bytecodeCanTruncateInteger(node->arithModeFlags())

- Machine int addition with overflow check.  ValueAdd or ArithAdd with MachineIntUse.

It seems like it would be better if after FixupPhase, ValueAdd always meant addition-or-strcat - so FixupPhase with turn ValueAdd to ArithAdd if it's a numeric addition.  It also seems like it would be better to have an IntAdd node, that means integer addition without overflow check.  ArithAdd with Int32Use's always implies overflow checking.  It also seems like ArithAdd should be called DoubleAdd so that we remember that semantically this is always a double addition, even if we've strength-reduced it to an int addition.

This will mean we have the following opcodes, with the following meanings:

ValueAdd: not type-inferred addition-or-strcat.
DoubleAdd: a numerical addition that is semantically required to behave like a double addition, but can be speculated to various types depending on useKind (NumberUse, MachineIntUse, Int32Use).  Because this is required to provide double-equivalent semantics, the integer versions will need overflow checks.
Int32Add: an integer addition.

We can do similar things for other arithmetic operations.
Comment 1 Filip Pizlo 2013-12-10 09:11:55 PST
This blocks https://bugs.webkit.org/show_bug.cgi?id=125433 because I want to be able to insert various kinds of arithmetic into the graph.  This is currently awkward to do because if you add a ValueAdd or ArithAdd, you have to make sure that the NodeFlags are set *just right* or you'll get the wrong semantics.  I don't want NodeFlags to factor into semantics for arithmetic, at least not after FixupPhase.
Comment 2 Filip Pizlo 2013-12-10 09:13:53 PST
Actually, I think I'll keep the name "ArithAdd" rather than using the term "DoubleAdd".  This is desirable because we sometimes use the word "Double" in DFG node types to mean that we're doing something with a value's *representation*, which isn't what I want here.
Comment 3 Filip Pizlo 2014-01-03 11:22:41 PST
Created attachment 220323 [details]
work in progress
Comment 4 Filip Pizlo 2014-01-03 20:12:19 PST
Created attachment 220365 [details]
more correcter
Comment 5 Filip Pizlo 2014-01-06 13:47:34 PST
Created attachment 220453 [details]
more
Comment 6 Filip Pizlo 2014-01-06 16:00:33 PST
Created attachment 220466 [details]
the patch
Comment 7 WebKit Commit Bot 2014-01-06 16:02:40 PST
Attachment 220466 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/CMakeLists.txt', u'Source/JavaScriptCore/ChangeLog', u'Source/JavaScriptCore/GNUmakefile.list.am', u'Source/JavaScriptCore/JavaScriptCore.vcxproj/JavaScriptCore.vcxproj', u'Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj', u'Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h', u'Source/JavaScriptCore/dfg/DFGArithMode.cpp', u'Source/JavaScriptCore/dfg/DFGArithMode.h', u'Source/JavaScriptCore/dfg/DFGCSEPhase.cpp', u'Source/JavaScriptCore/dfg/DFGConstantFoldingPhase.cpp', u'Source/JavaScriptCore/dfg/DFGFixupPhase.cpp', u'Source/JavaScriptCore/dfg/DFGGraph.cpp', u'Source/JavaScriptCore/dfg/DFGNode.h', u'Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp', u'Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h', u'Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp', u'Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp', u'Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp', '--commit-queue']" exit_code: 1
ERROR: Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h:661:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
Total errors found: 1 in 17 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 8 Geoffrey Garen 2014-01-06 16:43:40 PST
Comment on attachment 220466 [details]
the patch

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

r=me

> Source/JavaScriptCore/ChangeLog:3
> +        Make the different flavors of integer arithmetic more explicit, and don't rely on (possibly stale) results of the backwards propagator to decide integer arithmetic semantis

"semantics"

> Source/JavaScriptCore/dfg/DFGArithMode.h:38
> +    Unchecked, // Don't check anything and just do an integer operation.

I think this might be clearer as "Int" or "Hardware". "Unchecked" doesn't really specify what *is* required, only what's *not* required.

> Source/JavaScriptCore/dfg/DFGArithMode.h:41
> +    DoOverflow // Even though the inputs are integers, up-convert them to doubles and return a double.

I think this might be clearer as "Double" or "OverflowToDouble". At first, I read "DoOverflow" as "allow the CPU to overflow in int space".

> Source/JavaScriptCore/dfg/DFGArithMode.h:45
> +inline bool doesOverflow(Arith::Mode mode)

I think this might be clearer as "isDouble" or "shouldOverflowToDouble".
Comment 9 Filip Pizlo 2014-01-06 19:09:01 PST
(In reply to comment #8)
> (From update of attachment 220466 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=220466&action=review
> 
> r=me
> 
> > Source/JavaScriptCore/ChangeLog:3
> > +        Make the different flavors of integer arithmetic more explicit, and don't rely on (possibly stale) results of the backwards propagator to decide integer arithmetic semantis
> 
> "semantics"
> 
> > Source/JavaScriptCore/dfg/DFGArithMode.h:38
> > +    Unchecked, // Don't check anything and just do an integer operation.
> 
> I think this might be clearer as "Int" or "Hardware". "Unchecked" doesn't really specify what *is* required, only what's *not* required.

I still prefer "Unchecked".  The actual semantics of the arithmetic operation is determined by the input type first, and the ArithMode second.  Using the input type as the primary mode selector is in line with operations that doesn't have ArithMode and never will, like CompareEq/CompareLess/etc.  So, for ArithAdd, you might have an input type like Int32Use and then an ArithMode like Unchecked.  Making the ArithMode be "IntUnchecked" would be tautological - the input type already told you that you're getting ints as inputs and producing an int as an output.  Making the ArithMode be "Int" would be both tautological and too vague.  Both CheckOverflow and CheckOverflowAndNegativeZero have to do with integer operations as well, and both also take ints as inputs and produce ints as outputs.  So "Int" doesn't tell me anything.  I already knew that I was dealing with integers.  The *relevant* thing is that I'm doing an unchecked arithmetic operation.  I also don't like "Hardware" because overflow checking is a hardware thing - all of the hardware that we care about has hardware overflow bits and overflow branch instructions.

Some processors have an "overflow bit" on arithmetic operations, that determines whether the operation sets the overflow flag.  I think PPC worked like this.  Think of ArithMode as an overflow-bit-on-stereoids.  The "bit" is either false (Unchecked) or true (CheckOverflow), but it can explicitly tell you if the decision hasn't been made or is not applicable (NotSet), it can tell you if to also check negative zero (CheckOverflowAndNegativeZero), and DoOverflow is a special case for now (only UInt32ToNumber uses it).

I *almost* decided to have DoOverflow be part of a different enumeration - but that enumeration would optimally just have "NotSet", "CheckOverflow", and "DoOverflow".  The overlap made me think that just making it part of this enumeration would be easiest.

> 
> > Source/JavaScriptCore/dfg/DFGArithMode.h:41
> > +    DoOverflow // Even though the inputs are integers, up-convert them to doubles and return a double.
> 
> I think this might be clearer as "Double" or "OverflowToDouble". At first, I read "DoOverflow" as "allow the CPU to overflow in int space".

It should be DoOverflow because it really means: overflow to something bigger than the input was.  So, optimally, Int32 would overflow to Int52 rather than overflowing to Double and there is a bug for this.

My goal is to use terms that are overloadable so that we don't have to have separate enumerations for each kind of arithmetic operation, and to avoid having to change the name of the enumeration elements when we implement optimizations.  So, when we do get around to fixing Uint32ToNumber so that it returns an Int52 on JSVALUE64, and still returns a double on JSVALUE32_64 (since JSVALUE32_64 doesn't have Int52), we won't have to add a second enumeration element.  DoesOverflow will mean:

JSVALUE64: overflow to Int52
JSVALUE32_64: overflow to double

More formally, it means "overflow to the smallest type that the implementation allows for that soundly represents all possible results given the input type".

You could argue that we should apply YAGNI to the IR and call it OverflowToDouble for now, and then change the name later.  Applying YAGNI in order to make the names of things overly specific is a dangerous thing to do because it's all-too-tempting to then make OverflowToDouble be implemented as overflow-to-int52 and not change the name.  That's why I'd rather the IR's terminology be a bit vague in cases like this.

> 
> > Source/JavaScriptCore/dfg/DFGArithMode.h:45
> > +inline bool doesOverflow(Arith::Mode mode)
> 
> I think this might be clearer as "isDouble" or "shouldOverflowToDouble".

I think it's too specific.
Comment 10 Filip Pizlo 2014-01-06 19:13:31 PST
Performance results:


Benchmark report for SunSpider, LongSpider, V8Spider, Octane, Kraken, and JSRegress on oldmac (MacPro4,1).

VMs tested:
"TipOfTree" at /Volumes/Data/pizlo/OpenSource/WebKitBuild/Release/jsc (r161244)
"SimpleMath" at /Volumes/Data/fromMiniMe/tertiary/OpenSource/WebKitBuild/Release/jsc (r161244)

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.

                                                        TipOfTree                 SimpleMath                                    
SunSpider:
   3d-cube                                            7.3425+-0.0811            7.3417+-0.0404        
   3d-morph                                           8.9055+-0.0678     ?      8.9131+-0.0951        ?
   3d-raytrace                                        9.1552+-0.0965            9.0630+-0.1288          might be 1.0102x faster
   access-binary-trees                                2.1929+-0.0938            2.1309+-0.0195          might be 1.0291x faster
   access-fannkuch                                    8.1003+-0.0619            8.0401+-0.0590        
   access-nbody                                       4.2811+-0.0076            4.2800+-0.0104        
   access-nsieve                                      4.9721+-0.0287     ?      5.0034+-0.0386        ?
   bitops-3bit-bits-in-byte                           1.8857+-0.0055            1.8836+-0.0051        
   bitops-bits-in-byte                                7.1930+-0.0799            7.1784+-0.0747        
   bitops-bitwise-and                                 2.9788+-0.0310     ?      2.9969+-0.0108        ?
   bitops-nsieve-bits                                 5.2451+-0.0106     ?      5.2472+-0.0116        ?
   controlflow-recursive                              3.1489+-0.0213            3.1365+-0.0343        
   crypto-aes                                         5.6358+-0.0743     ?      5.7304+-0.1404        ? might be 1.0168x slower
   crypto-md5                                         3.4062+-0.0660     ?      3.4250+-0.0165        ?
   crypto-sha1                                        3.0481+-0.0415            3.0376+-0.0353        
   date-format-tofte                                 12.4049+-0.2140     ^     11.8731+-0.2954        ^ definitely 1.0448x faster
   date-format-xparb                                  8.7091+-0.1368     ?      8.7171+-0.0614        ?
   math-cordic                                        4.2580+-0.0119     ?      4.2991+-0.0926        ?
   math-partial-sums                                 10.1553+-0.0585     !     10.2879+-0.0686        ! definitely 1.0131x slower
   math-spectral-norm                                 2.7913+-0.0083            2.7879+-0.0077        
   regexp-dna                                        12.9906+-0.1229     ?     13.1385+-0.1640        ? might be 1.0114x slower
   string-base64                                      5.8424+-0.0391            5.7925+-0.0236        
   string-fasta                                      10.5025+-0.1097     ?     10.5457+-0.1221        ?
   string-tagcloud                                   15.8941+-0.2660           15.5233+-0.1265          might be 1.0239x faster
   string-unpack-code                                31.2701+-0.1607     !     31.6960+-0.1320        ! definitely 1.0136x slower
   string-validate-input                              6.9808+-0.0402     !      7.0822+-0.0402        ! definitely 1.0145x slower

   <arithmetic> *                                     7.6650+-0.0180            7.6597+-0.0163          might be 1.0007x faster
   <geometric>                                        6.1451+-0.0141            6.1379+-0.0111          might be 1.0012x faster
   <harmonic>                                         5.0439+-0.0170            5.0349+-0.0104          might be 1.0018x faster

                                                        TipOfTree                 SimpleMath                                    
LongSpider:
   3d-cube                                         2150.4546+-12.3694        2143.2926+-8.6607        
   3d-morph                                        1501.2196+-1.2220         1500.7383+-1.0006        
   3d-raytrace                                     1520.8118+-4.8756     ^   1499.3996+-7.0579        ^ definitely 1.0143x faster
   access-binary-trees                             2463.1347+-33.8171        2456.9344+-11.4935       
   access-fannkuch                                  656.6345+-0.6609     ^    650.0602+-1.7042        ^ definitely 1.0101x faster
   access-nbody                                    1496.3347+-0.7425         1495.8375+-0.6824        
   access-nsieve                                   1551.3519+-4.1101     ?   1552.0666+-4.3783        ?
   bitops-3bit-bits-in-byte                         126.1881+-0.2789          126.1290+-0.1878        
   bitops-bits-in-byte                              601.8212+-4.0344          600.2003+-3.7405        
   bitops-nsieve-bits                              1153.1025+-14.1138        1146.5311+-1.3145        
   controlflow-recursive                           1470.6700+-0.6894     ?   1471.6143+-2.5266        ?
   crypto-aes                                      1667.1826+-5.7312     ^   1658.4624+-2.8335        ^ definitely 1.0053x faster
   crypto-md5                                      1231.7262+-1.5940     ^   1218.8118+-1.7421        ^ definitely 1.0106x faster
   crypto-sha1                                     1603.9392+-3.0255     ?   1604.2062+-2.1367        ?
   date-format-tofte                               1250.0394+-3.4309     ^   1212.0723+-7.8669        ^ definitely 1.0313x faster
   date-format-xparb                               1450.7481+-26.3399    ?   1465.7784+-10.3369       ? might be 1.0104x slower
   math-cordic                                     1736.1817+-1.6681     ?   1760.5905+-49.1727       ? might be 1.0141x slower
   math-partial-sums                               1302.5680+-1.6240     ?   1303.7958+-0.6356        ?
   math-spectral-norm                              1826.3318+-0.6865     ?   1826.8048+-0.7584        ?
   string-base64                                    590.1951+-2.4668     ^    580.3344+-2.9405        ^ definitely 1.0170x faster
   string-fasta                                     993.3437+-8.7243     ?   1010.8606+-26.5763       ? might be 1.0176x slower
   string-tagcloud                                  389.6089+-1.8526     ?    392.6470+-2.4831        ?

   <arithmetic>                                    1306.0722+-2.2980         1303.5076+-2.6050          might be 1.0020x faster
   <geometric> *                                   1125.0802+-1.3403         1122.4359+-1.9801          might be 1.0024x faster
   <harmonic>                                       819.4074+-0.4652     ^    817.7756+-1.1369        ^ definitely 1.0020x faster

                                                        TipOfTree                 SimpleMath                                    
V8Spider:
   crypto                                            79.4730+-0.1209           79.3952+-0.0880        
   deltablue                                        100.1944+-0.5830          100.1612+-0.6105        
   earley-boyer                                      74.1640+-0.3562           73.9167+-0.3499        
   raytrace                                          45.8946+-0.5237     ?     46.0260+-0.6167        ?
   regexp                                           101.3243+-1.4558          100.3402+-0.3020        
   richards                                         131.4362+-1.4707          131.1863+-1.1325        
   splay                                             46.0918+-0.1863     ?     46.5947+-0.3447        ? might be 1.0109x slower

   <arithmetic>                                      82.6540+-0.2719           82.5172+-0.1998          might be 1.0017x faster
   <geometric> *                                     77.3924+-0.2172           77.3649+-0.1788          might be 1.0004x faster
   <harmonic>                                        72.1502+-0.2256     ?     72.2434+-0.2266        ? might be 1.0013x slower

                                                        TipOfTree                 SimpleMath                                    
Octane and V8v7:
   encrypt                                           0.46858+-0.00651          0.46560+-0.00046       
   decrypt                                           8.58239+-0.01687          8.57521+-0.00892       
   deltablue                                x2       0.57146+-0.00687          0.56440+-0.00239         might be 1.0125x faster
   earley                                            0.90484+-0.00371    ?     0.90559+-0.00555       ?
   boyer                                            12.56958+-0.05931         12.56145+-0.14887       
   raytrace                                 x2       4.34413+-0.05762          4.32033+-0.05700       
   regexp                                   x2      32.79526+-0.09794    ?    32.80306+-0.10300       ?
   richards                                 x2       0.43281+-0.00682    ?     0.43476+-0.00553       ?
   splay                                    x2       0.63767+-0.00311    ?     0.63830+-0.00427       ?
   navier-stokes                            x2      10.70620+-0.00557         10.70471+-0.00400       
   closure                                           0.43741+-0.00089    ?     0.43900+-0.00205       ?
   jquery                                            6.35606+-0.00996    ?     6.39220+-0.02826       ?
   gbemu                                    x2      71.44439+-0.63168         71.21440+-0.51572       
   mandreel                                 x2     135.33489+-0.83721        135.18832+-0.57543       
   pdfjs                                    x2     101.76514+-0.38193    !   102.69376+-0.31672       ! definitely 1.0091x slower
   box2d                                    x2      34.82981+-0.16813         34.69142+-0.16835       

V8v7:
   <arithmetic>                                      7.59378+-0.01213          7.58994+-0.01479         might be 1.0005x faster
   <geometric> *                                     2.51994+-0.00810          2.51500+-0.00645         might be 1.0020x faster
   <harmonic>                                        1.03871+-0.00586          1.03644+-0.00497         might be 1.0022x faster

Octane including V8v7:
   <arithmetic>                                     31.34778+-0.08313    ?    31.37869+-0.07010       ? might be 1.0010x slower
   <geometric> *                                     6.97741+-0.01673          6.97192+-0.01090         might be 1.0008x faster
   <harmonic>                                        1.44707+-0.00718          1.44509+-0.00620         might be 1.0014x faster

                                                        TipOfTree                 SimpleMath                                    
Kraken:
   ai-astar                                          494.853+-0.345            494.652+-0.218         
   audio-beat-detection                              223.912+-0.451            223.754+-0.504         
   audio-dft                                         290.551+-1.035      ?     291.150+-2.381         ?
   audio-fft                                         130.804+-0.100      ^     130.531+-0.126         ^ definitely 1.0021x faster
   audio-oscillator                                  244.273+-0.464            244.153+-0.438         
   imaging-darkroom                                  289.398+-8.910      ?     293.894+-17.339        ? might be 1.0155x slower
   imaging-desaturate                                158.371+-0.162      ?     158.461+-0.440         ?
   imaging-gaussian-blur                             364.181+-1.244            363.477+-0.121         
   json-parse-financial                               81.917+-0.885      ?      83.271+-1.295         ? might be 1.0165x slower
   json-stringify-tinderbox                          104.325+-0.275      ?     104.726+-0.436         ?
   stanford-crypto-aes                                91.386+-0.613             91.002+-0.274         
   stanford-crypto-ccm                               101.869+-1.156             99.707+-1.294           might be 1.0217x faster
   stanford-crypto-pbkdf2                            261.848+-1.035      ^     257.222+-1.848         ^ definitely 1.0180x faster
   stanford-crypto-sha256-iterative                  114.908+-2.010            114.097+-0.507         

   <arithmetic> *                                    210.900+-0.694            210.721+-1.348           might be 1.0008x faster
   <geometric>                                       181.173+-0.512            180.920+-0.962           might be 1.0014x faster
   <harmonic>                                        156.826+-0.491            156.603+-0.739           might be 1.0014x faster

                                                        TipOfTree                 SimpleMath                                    
JSRegress:
   adapt-to-double-divide                            22.7884+-0.1218           22.7712+-0.0571        
   aliased-arguments-getbyval                         0.9983+-0.0041            0.9944+-0.0027        
   allocate-big-object                                3.1212+-0.0547            3.1010+-0.0141        
   arity-mismatch-inlining                            0.9552+-0.0040     ?      0.9621+-0.0107        ?
   array-access-polymorphic-structure                 9.9876+-0.1310            9.8635+-0.1075          might be 1.0126x faster
   array-nonarray-polymorhpic-access                 58.3292+-0.2212           58.2338+-0.1803        
   array-with-double-add                              5.7706+-0.0604     ?      5.7805+-0.0625        ?
   array-with-double-increment                        4.3656+-0.0181     ?      4.3730+-0.0203        ?
   array-with-double-mul-add                          6.8944+-0.0428            6.8841+-0.0810        
   array-with-double-sum                              8.0334+-0.0780     ?      8.0614+-0.0869        ?
   array-with-int32-add-sub                          10.5633+-0.1056           10.4389+-0.0839          might be 1.0119x faster
   array-with-int32-or-double-sum                     8.0307+-0.0746     ?      8.0694+-0.0392        ?
   ArrayBuffer-DataView-alloc-large-long-lived   
                                                    117.8314+-0.7376     !    119.9535+-0.7459        ! definitely 1.0180x slower
   ArrayBuffer-DataView-alloc-long-lived             30.4649+-0.1837     !     30.8697+-0.1244        ! definitely 1.0133x slower
   ArrayBuffer-Int32Array-byteOffset                  6.0260+-0.0776            5.9533+-0.0800          might be 1.0122x faster
   ArrayBuffer-Int8Array-alloc-huge-long-lived   
                                                    214.4856+-2.8123     ?    215.6500+-2.1912        ?
   ArrayBuffer-Int8Array-alloc-large-long-lived-fragmented   
                                                    166.3358+-0.8034     ?    167.1229+-1.0014        ?
   ArrayBuffer-Int8Array-alloc-large-long-lived   
                                                    118.6200+-0.7279     !    120.5706+-0.9981        ! definitely 1.0164x slower
   ArrayBuffer-Int8Array-alloc-long-lived-buffer   
                                                     48.5788+-0.2236     ?     48.8760+-0.2518        ?
   ArrayBuffer-Int8Array-alloc-long-lived            30.1490+-0.2096     !     30.6587+-0.1432        ! definitely 1.0169x slower
   ArrayBuffer-Int8Array-alloc                       26.2832+-0.4061     ?     26.5486+-0.2032        ? might be 1.0101x slower
   asmjs_bool_bug                                     9.2583+-0.1214            9.2363+-0.0526        
   basic-set                                         19.9854+-0.1107           19.8786+-0.1194        
   big-int-mul                                        5.5285+-0.0681     ?      5.5438+-0.0572        ?
   boolean-test                                       4.4320+-0.0213            4.4228+-0.0207        
   branch-fold                                        4.8935+-0.0557     ?      4.9137+-0.0068        ?
   by-val-generic                                    12.7673+-0.2038           12.5942+-0.1587          might be 1.0137x faster
   captured-assignments                               0.5824+-0.0059     ?      0.5868+-0.0054        ?
   cast-int-to-double                                12.4218+-0.1214     ?     12.4799+-0.0937        ?
   cell-argument                                     15.7493+-0.4327     ?     16.1805+-0.4144        ? might be 1.0274x slower
   cfg-simplify                                       3.9800+-0.0059     ?      3.9819+-0.0061        ?
   chain-custom-getter                              159.8061+-4.9434     ?    160.2669+-5.5892        ?
   chain-getter-access                              488.6087+-1.7341     ^    482.2610+-4.4660        ^ definitely 1.0132x faster
   cmpeq-obj-to-obj-other                            11.9984+-0.6281           11.8646+-0.4436          might be 1.0113x faster
   constant-test                                      8.9675+-0.1273            8.8831+-0.0853        
   DataView-custom-properties                       125.7240+-1.0698     ?    126.5701+-0.9567        ?
   delay-tear-off-arguments-strictmode                3.6212+-0.0103     ?      3.6252+-0.0177        ?
   destructuring-arguments-length                   172.8433+-1.8695     !    177.0964+-1.5504        ! definitely 1.0246x slower
   destructuring-arguments                            8.8240+-0.0303     ?      8.9178+-0.0939        ? might be 1.0106x slower
   destructuring-swap                                 8.6313+-0.1415            8.5639+-0.0683        
   direct-arguments-getbyval                          0.8696+-0.0130            0.8593+-0.0123          might be 1.0120x faster
   double-get-by-val-out-of-bounds                    7.4724+-0.0798            7.4666+-0.0634        
   double-pollution-getbyval                         11.0400+-0.0824     ?     11.0613+-0.1112        ?
   double-pollution-putbyoffset                       6.0900+-0.1195            6.0347+-0.0291        
   double-to-int32-typed-array-no-inline              2.6200+-0.0060     ?      2.6423+-0.0379        ?
   double-to-int32-typed-array                        2.2867+-0.0209     ?      2.2963+-0.0541        ?
   double-to-uint32-typed-array-no-inline             2.7285+-0.0060     ?      2.7293+-0.0048        ?
   double-to-uint32-typed-array                       2.4924+-0.0152     ?      2.4985+-0.0076        ?
   empty-string-plus-int                             10.9727+-0.0864     !     11.1901+-0.0962        ! definitely 1.0198x slower
   emscripten-cube2hash                              55.4718+-0.3479     ?     55.7702+-0.2664        ?
   emscripten-memops                               7042.6414+-35.6712    ?   7057.4156+-1.3689        ?
   external-arguments-getbyval                        2.1054+-0.0287            2.0644+-0.0180          might be 1.0198x faster
   external-arguments-putbyval                        3.1261+-0.0258            3.1063+-0.0218        
   fixed-typed-array-storage-var-index                1.4134+-0.0222            1.3993+-0.0065          might be 1.0100x faster
   fixed-typed-array-storage                          0.9992+-0.0327            0.9992+-0.0276        
   Float32Array-matrix-mult                           6.5657+-0.0908            6.5226+-0.0484        
   Float32Array-to-Float64Array-set                  92.7674+-0.3990     ?     92.9431+-0.6426        ?
   Float64Array-alloc-long-lived                    103.6343+-0.4517          103.2556+-0.7384        
   Float64Array-to-Int16Array-set                   117.0049+-0.5453     ?    117.3267+-1.7771        ?
   fold-double-to-int                                20.6007+-0.1023           20.5781+-0.2159        
   for-of-iterate-array-entries                       8.7196+-0.0929     ?      8.7280+-0.1776        ?
   for-of-iterate-array-keys                          3.4521+-0.0369            3.4234+-0.0536        
   for-of-iterate-array-values                        2.9749+-0.0558     ?      2.9761+-0.0539        ?
   function-dot-apply                                 3.1086+-0.0041     ?      3.1196+-0.0156        ?
   function-test                                      4.8984+-0.0484            4.8279+-0.0644          might be 1.0146x faster
   get-by-id-chain-from-try-block                     7.9750+-0.1025     ?      8.0604+-0.0497        ? might be 1.0107x slower
   get-by-id-proto-or-self                           26.0254+-0.2469           25.9218+-0.2275        
   get-by-id-self-or-proto                           23.5892+-0.5697     ?     24.1112+-0.5569        ? might be 1.0221x slower
   get-by-val-out-of-bounds                           7.2017+-0.0405     ?      7.2586+-0.0829        ?
   get_callee_monomorphic                             5.0048+-0.0284            4.9645+-0.0720        
   get_callee_polymorphic                             4.8374+-0.0195     ?      4.8529+-0.0252        ?
   global-var-const-infer-fire-from-opt               1.0596+-0.0322            1.0420+-0.0403          might be 1.0170x faster
   global-var-const-infer                             0.7934+-0.0047     ?      0.8015+-0.0148        ? might be 1.0102x slower
   HashMap-put-get-iterate-keys                      42.5726+-0.1723     ?     42.7582+-0.3977        ?
   HashMap-put-get-iterate                           54.1825+-0.2382           53.9908+-0.2194        
   HashMap-string-put-get-iterate                    51.0364+-0.2387     !     51.7064+-0.1416        ! definitely 1.0131x slower
   imul-double-only                                  17.8478+-0.2100           17.8090+-0.2026        
   imul-int-only                                     14.8675+-0.0586     ?     14.9082+-0.1298        ?
   imul-mixed                                        22.6525+-1.1861           21.8513+-0.0533          might be 1.0367x faster
   in-four-cases                                     26.0893+-0.3343           25.9324+-0.0818        
   in-one-case-false                                 12.1976+-0.1419           12.1059+-0.0934        
   in-one-case-true                                  12.1375+-0.1174     ?     12.1520+-0.1107        ?
   in-two-cases                                      12.9217+-0.0946     ?     12.9538+-0.1379        ?
   indexed-properties-in-objects                      4.1739+-0.0058     ?      4.1804+-0.0186        ?
   infer-closure-const-then-mov-no-inline            15.3281+-0.1128           15.2802+-0.1277        
   infer-closure-const-then-mov                      29.0251+-0.1101           28.9565+-0.1305        
   infer-closure-const-then-put-to-scope-no-inline   
                                                     17.7331+-0.1104     ?     17.8360+-0.1115        ?
   infer-closure-const-then-put-to-scope             36.1560+-0.3797           36.0647+-0.1402        
   infer-closure-const-then-reenter-no-inline   
                                                     84.3638+-0.1028     ?     84.4197+-0.1293        ?
   infer-closure-const-then-reenter                  35.8842+-0.1179           35.7842+-0.2610        
   infer-one-time-closure-ten-vars                   29.0607+-0.0876           29.0329+-0.1251        
   infer-one-time-closure-two-vars                   28.6865+-0.1169     ?     28.8179+-0.1182        ?
   infer-one-time-closure                            28.7644+-0.1554           28.6901+-0.0973        
   infer-one-time-deep-closure                       58.4586+-0.2975           58.1458+-0.4836        
   inline-arguments-access                            1.6988+-0.0201            1.6913+-0.0062        
   inline-arguments-aliased-access                    1.7984+-0.0038     ?      1.8175+-0.0292        ? might be 1.0106x slower
   inline-arguments-local-escape                     22.3476+-0.1816     !     23.1594+-0.2313        ! definitely 1.0363x slower
   inline-get-scoped-var                              7.4958+-0.0301     ?      7.5051+-0.0773        ?
   inlined-put-by-id-transition                      15.5200+-0.2314           15.3140+-0.3532          might be 1.0135x faster
   int-or-other-abs-then-get-by-val                   9.4592+-0.0713            9.4270+-0.1012        
   int-or-other-abs-zero-then-get-by-val             37.5753+-0.4658           37.4160+-0.4805        
   int-or-other-add-then-get-by-val                  10.6186+-0.1170           10.5239+-0.1398        
   int-or-other-add                                  10.9441+-0.1061     ?     10.9992+-0.1408        ?
   int-or-other-div-then-get-by-val                   6.3811+-0.0907     ?      6.3881+-0.0594        ?
   int-or-other-max-then-get-by-val                   8.8033+-0.1652            8.7307+-0.1996        
   int-or-other-min-then-get-by-val                   7.0657+-0.1118     ?      7.0900+-0.0554        ?
   int-or-other-mod-then-get-by-val                   6.1677+-0.0584     ?      6.1950+-0.0260        ?
   int-or-other-mul-then-get-by-val                   6.6599+-0.0204     ?      6.6836+-0.0370        ?
   int-or-other-neg-then-get-by-val                   7.9441+-0.0725     ?      7.9768+-0.0536        ?
   int-or-other-neg-zero-then-get-by-val             37.0841+-0.4728           36.9707+-0.1211        
   int-or-other-sub-then-get-by-val                  10.5878+-0.1879           10.5281+-0.1236        
   int-or-other-sub                                   8.8847+-0.0806     ?      8.8912+-0.0798        ?
   int-overflow-local                                 6.4179+-0.0699            6.4086+-0.0780        
   Int16Array-alloc-long-lived                       67.8005+-0.4408     ?     68.0532+-0.3504        ?
   Int16Array-bubble-sort-with-byteLength            48.9869+-0.1013           48.9125+-0.0931        
   Int16Array-bubble-sort                            47.9287+-0.1285           47.9140+-0.1042        
   Int16Array-load-int-mul                            1.8318+-0.0192            1.8238+-0.0160        
   Int16Array-to-Int32Array-set                      91.6772+-1.1026           90.8073+-1.1896        
   Int32Array-alloc-huge-long-lived                 704.7167+-3.4719     ?    706.8021+-4.6910        ?
   Int32Array-alloc-huge                            812.5910+-7.0389          808.8862+-8.5080        
   Int32Array-alloc-large-long-lived                977.4719+-8.9179          974.8927+-8.8645        
   Int32Array-alloc-large                            44.8821+-0.7305     ?     45.0096+-0.8970        ?
   Int32Array-alloc-long-lived                       81.9039+-0.4930     ^     80.6772+-0.4421        ^ definitely 1.0152x faster
   Int32Array-alloc                                   4.5365+-0.0137            4.5330+-0.0141        
   Int32Array-Int8Array-view-alloc                   15.0308+-0.1240     ?     15.0865+-0.0673        ?
   int52-spill                                       12.9146+-0.1476     ^     12.4607+-0.1318        ^ definitely 1.0364x faster
   Int8Array-alloc-long-lived                        67.2255+-0.6582           66.9081+-0.2442        
   Int8Array-load-with-byteLength                     5.0716+-0.0181            5.0697+-0.0148        
   Int8Array-load                                     5.0287+-0.0729     ?      5.0328+-0.0684        ?
   integer-divide                                    15.0604+-0.0687           15.0161+-0.0645        
   integer-modulo                                     2.0249+-0.0093     ?      2.0279+-0.0090        ?
   large-int-captured                                 9.7935+-0.1134     ?      9.9273+-0.4263        ? might be 1.0137x slower
   large-int-neg                                     26.4185+-0.1947     ^     25.4640+-0.0965        ^ definitely 1.0375x faster
   large-int                                         23.1027+-0.1549     ?     23.1228+-0.1314        ?
   logical-not                                       11.2944+-0.1161           11.1845+-0.1361        
   lots-of-fields                                    12.3774+-0.0463     ?     12.3963+-0.1072        ?
   make-indexed-storage                               4.3506+-0.0280            4.3479+-0.0180        
   make-rope-cse                                      6.0846+-0.0831     ?      6.1780+-0.1686        ? might be 1.0153x slower
   marsaglia-larger-ints                            111.8204+-0.1424     ?    111.8354+-0.1845        ?
   marsaglia-osr-entry                               47.0383+-0.1557     ?     47.0844+-0.1041        ?
   marsaglia                                        462.5316+-0.4905     ?    466.3638+-6.2116        ?
   method-on-number                                  30.2394+-0.6397           29.9569+-0.6533        
   negative-zero-divide                               0.4297+-0.0016            0.4266+-0.0016        
   negative-zero-modulo                               0.4233+-0.0174            0.4107+-0.0013          might be 1.0307x faster
   negative-zero-negate                               0.3972+-0.0027            0.3961+-0.0020        
   nested-function-parsing-random                   378.9720+-0.2362     !    383.0845+-2.8884        ! definitely 1.0109x slower
   nested-function-parsing                           47.2952+-0.0747     ?     47.4606+-0.1228        ?
   new-array-buffer-dead                              3.7938+-0.0195            3.7741+-0.0138        
   new-array-buffer-push                             10.5815+-0.1373     ?     10.6480+-0.1280        ?
   new-array-dead                                    28.5589+-0.1123     ?     28.5859+-0.0666        ?
   new-array-push                                     7.0704+-0.0991            6.9181+-0.0652          might be 1.0220x faster
   number-test                                        4.3518+-0.0083     ?      4.3749+-0.0261        ?
   object-closure-call                               13.3799+-0.1060           13.3589+-0.1282        
   object-test                                        4.6741+-0.0562     ?      4.7112+-0.0318        ?
   poly-stricteq                                     76.3901+-0.1470     ?     76.7875+-1.4595        ?
   polymorphic-structure                             20.6050+-0.2798           20.5654+-0.2373        
   polyvariant-monomorphic-get-by-id                 11.9903+-0.0818     ?     12.0390+-0.0798        ?
   proto-custom-getter                              171.6590+-10.5159         160.2037+-5.6062          might be 1.0715x faster
   proto-getter-access                              488.8543+-5.0545          486.5065+-5.7242        
   put-by-id                                         19.6309+-0.2719           19.4162+-0.4531          might be 1.0111x faster
   put-by-val-large-index-blank-indexing-type   
                                                     20.6385+-0.1173           20.5120+-0.1351        
   put-by-val-machine-int                             3.3790+-0.0144            3.3733+-0.0155        
   rare-osr-exit-on-local                            20.3731+-0.2672           20.3135+-0.1434        
   register-pressure-from-osr                        31.3959+-0.1195           31.3047+-0.1266        
   simple-activation-demo                            35.2131+-0.0815     ?     35.2404+-0.1953        ?
   simple-custom-getter                             499.9823+-0.1376     ?    502.8483+-6.4137        ?
   simple-getter-access                             785.6796+-11.1201         780.8234+-10.7176       
   slow-array-profile-convergence                     4.0207+-0.0143            4.0025+-0.0091        
   slow-convergence                                   4.5401+-0.0241            4.5347+-0.0358        
   sparse-conditional                                 1.4182+-0.0206            1.4056+-0.0101        
   splice-to-remove                                  77.5571+-0.0850     ^     77.1464+-0.1307        ^ definitely 1.0053x faster
   stepanov_container                             10174.9525+-23.9838    ?  10193.8967+-31.6146       ?
   string-concat-object                               3.1043+-0.0257            3.0884+-0.0144        
   string-concat-pair-object                          3.0250+-0.0407            3.0198+-0.0295        
   string-concat-pair-simple                         17.3161+-0.2044           17.1913+-0.2716        
   string-concat-simple                              17.1775+-0.2815     ?     17.3934+-0.1522        ? might be 1.0126x slower
   string-cons-repeat                                10.6600+-0.0166           10.6304+-0.0307        
   string-cons-tower                                 11.0881+-0.0522     ?     11.1168+-0.0333        ?
   string-equality                                   43.0957+-0.6796           42.8582+-0.3906        
   string-get-by-val-big-char                        12.6683+-0.0791     !     12.8922+-0.1098        ! definitely 1.0177x slower
   string-get-by-val-out-of-bounds-insane             5.8244+-0.1501     ?      5.8853+-0.1241        ? might be 1.0105x slower
   string-get-by-val-out-of-bounds                    5.3385+-0.0417     ?      5.3627+-0.0359        ?
   string-get-by-val                                  4.8791+-0.0523     ?      4.8974+-0.0255        ?
   string-hash                                        2.7676+-0.0017     ?      2.7685+-0.0034        ?
   string-long-ident-equality                        37.3431+-0.4742     ?     37.4205+-0.5557        ?
   string-repeat-arith                               48.4223+-0.3274     ?     48.5672+-0.3253        ?
   string-sub                                       101.5079+-0.5455     ?    101.5321+-0.5503        ?
   string-test                                        4.3558+-0.0403            4.3337+-0.0345        
   string-var-equality                               70.4319+-0.7565           70.3158+-0.5820        
   structure-hoist-over-transitions                   3.4409+-0.0109     ?      3.4517+-0.0125        ?
   switch-char-constant                               3.4822+-0.0076     ?      3.4865+-0.0051        ?
   switch-char                                        8.1621+-0.0764            8.1361+-0.0774        
   switch-constant                                    9.3953+-0.1342            9.3920+-0.1472        
   switch-string-basic-big-var                       20.4859+-0.1025     ?     20.5443+-0.1077        ?
   switch-string-basic-big                           21.4525+-0.3412     ?     21.8008+-0.7625        ? might be 1.0162x slower
   switch-string-basic-var                           20.3714+-0.0840     ?     20.4165+-0.1195        ?
   switch-string-basic                               21.9225+-0.5452           21.8629+-0.7706        
   switch-string-big-length-tower-var                28.7346+-0.1571     ?     28.9811+-0.3019        ?
   switch-string-length-tower-var                    21.9731+-0.2908     ?     22.0099+-0.2148        ?
   switch-string-length-tower                        16.5312+-0.1256           16.4957+-0.0619        
   switch-string-short                               16.5067+-0.0988     ?     16.5518+-0.1853        ?
   switch                                            13.5787+-0.1316           13.5270+-0.1202        
   tear-off-arguments-simple                          2.3921+-0.0102     ?      2.3957+-0.0203        ?
   tear-off-arguments                                 3.6885+-0.0073            3.6822+-0.0142        
   temporal-structure                                17.1693+-0.1300     ?     17.2083+-0.0624        ?
   to-int32-boolean                                  21.4740+-0.1426     ?     21.6157+-0.1456        ?
   undefined-test                                     4.5393+-0.0342     ?      4.5602+-0.0078        ?
   weird-inlining-const-prop                          2.4013+-0.0134            2.3966+-0.0125        

   <arithmetic>                                     133.5770+-0.2288     ?    133.6999+-0.2098        ? might be 1.0009x slower
   <geometric> *                                     14.6895+-0.0163           14.6819+-0.0094          might be 1.0005x faster
   <harmonic>                                         5.2554+-0.0177            5.2399+-0.0094          might be 1.0030x faster

                                                        TipOfTree                 SimpleMath                                    
All benchmarks:
   <arithmetic>                                     201.5284+-0.2470          201.4174+-0.2462          might be 1.0006x faster
   <geometric>                                       20.4493+-0.0180           20.4336+-0.0090          might be 1.0008x faster
   <harmonic>                                         4.8161+-0.0133            4.8045+-0.0090          might be 1.0024x faster

                                                        TipOfTree                 SimpleMath                                    
Geomean of preferred means:
   <scaled-result>                                   49.3394+-0.0426           49.2936+-0.0655          might be 1.0009x faster
Comment 11 Filip Pizlo 2014-01-06 19:17:08 PST
(In reply to comment #8)
> (From update of attachment 220466 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=220466&action=review
> 
> r=me
> 
> > Source/JavaScriptCore/ChangeLog:3
> > +        Make the different flavors of integer arithmetic more explicit, and don't rely on (possibly stale) results of the backwards propagator to decide integer arithmetic semantis
> 
> "semantics"
> 
> > Source/JavaScriptCore/dfg/DFGArithMode.h:38
> > +    Unchecked, // Don't check anything and just do an integer operation.
> 
> I think this might be clearer as "Int" or "Hardware". "Unchecked" doesn't really specify what *is* required, only what's *not* required.
> 
> > Source/JavaScriptCore/dfg/DFGArithMode.h:41
> > +    DoOverflow // Even though the inputs are integers, up-convert them to doubles and return a double.
> 
> I think this might be clearer as "Double" or "OverflowToDouble". At first, I read "DoOverflow" as "allow the CPU to overflow in int space".
> 
> > Source/JavaScriptCore/dfg/DFGArithMode.h:45
> > +inline bool doesOverflow(Arith::Mode mode)
> 
> I think this might be clearer as "isDouble" or "shouldOverflowToDouble".

I will land without changing the names, but I will change the documentation.
Comment 12 Filip Pizlo 2014-01-06 19:18:11 PST
(In reply to comment #11)
> (In reply to comment #8)
> > (From update of attachment 220466 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=220466&action=review
> > 
> > r=me
> > 
> > > Source/JavaScriptCore/ChangeLog:3
> > > +        Make the different flavors of integer arithmetic more explicit, and don't rely on (possibly stale) results of the backwards propagator to decide integer arithmetic semantis
> > 
> > "semantics"
> > 
> > > Source/JavaScriptCore/dfg/DFGArithMode.h:38
> > > +    Unchecked, // Don't check anything and just do an integer operation.
> > 
> > I think this might be clearer as "Int" or "Hardware". "Unchecked" doesn't really specify what *is* required, only what's *not* required.
> > 
> > > Source/JavaScriptCore/dfg/DFGArithMode.h:41
> > > +    DoOverflow // Even though the inputs are integers, up-convert them to doubles and return a double.
> > 
> > I think this might be clearer as "Double" or "OverflowToDouble". At first, I read "DoOverflow" as "allow the CPU to overflow in int space".
> > 
> > > Source/JavaScriptCore/dfg/DFGArithMode.h:45
> > > +inline bool doesOverflow(Arith::Mode mode)
> > 
> > I think this might be clearer as "isDouble" or "shouldOverflowToDouble".
> 
> I will land without changing the names, but I will change the documentation.

New documentation:

enum Mode {
    NotSet, // Arithmetic mode is either not relevant because we're using doubles anyway or we are at a phase in compilation where we don't know what we're doing, yet. Should never see this after FixupPhase except for nodes that take doubles as inputs already.
    Unchecked, // Don't check anything and just do the direct hardware operation.
    CheckOverflow, // Check for overflow but don't bother with negative zero.
    CheckOverflowAndNegativeZero, // Check for both overflow and negative zero.
    DoOverflow // Up-convert to the smallest type that soundly represents all possible results after input type speculation.
};
Comment 13 Filip Pizlo 2014-01-06 20:49:13 PST
Landed in http://trac.webkit.org/changeset/161399