Bug 125519

Summary: 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
Product: WebKit Reporter: Filip Pizlo <fpizlo>
Component: JavaScriptCoreAssignee: Filip Pizlo <fpizlo>
Status: RESOLVED FIXED    
Severity: Normal CC: barraclough, bunhere, commit-queue, ggaren, gyuyoung.kim, mark.lam, mhahnenberg, msaboff, nrotem, oliver, rakuco, sam
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on: 125523    
Bug Blocks: 125433    
Attachments:
Description Flags
work in progress
none
more correcter
none
more
none
the patch ggaren: review+

Filip Pizlo
Reported 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.
Attachments
work in progress (38.00 KB, patch)
2014-01-03 11:22 PST, Filip Pizlo 		no flags
DetailsFormatted DiffDiff
more correcter (53.68 KB, patch)
2014-01-03 20:12 PST, Filip Pizlo 		no flags
DetailsFormatted DiffDiff
more (63.12 KB, patch)
2014-01-06 13:47 PST, Filip Pizlo 		no flags
DetailsFormatted DiffDiff
the patch (71.09 KB, patch)
2014-01-06 16:00 PST, Filip Pizlo 		ggaren: review+
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Filip Pizlo
Comment 1 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.
Filip Pizlo
Comment 2 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.
Filip Pizlo
Comment 3 2014-01-03 11:22:41 PST
Created attachment 220323 [details] work in progress
Filip Pizlo
Comment 4 2014-01-03 20:12:19 PST
Created attachment 220365 [details] more correcter
Filip Pizlo
Comment 5 2014-01-06 13:47:34 PST
Created attachment 220453 [details] more
Filip Pizlo
Comment 6 2014-01-06 16:00:33 PST
Created attachment 220466 [details] the patch
WebKit Commit Bot
Comment 7 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.
Geoffrey Garen
Comment 8 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".
Filip Pizlo
Comment 9 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.
Filip Pizlo
Comment 10 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
Filip Pizlo
Comment 11 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.
Filip Pizlo
Comment 12 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. };
Filip Pizlo
Comment 13 2014-01-06 20:49:13 PST
Landed in http://trac.webkit.org/changeset/161399
Note You need to log in before you can comment on or make changes to this bug.