[JSC] DFG should support relational comparisons of Number and Other
Created attachment 276564 [details] Patch
>5% on Sunspider/3d-raytrace. Easy wins here and there: Conf#1 Conf#2 SunSpider: 3d-cube 4.9859+-0.1048 ? 5.0979+-0.0845 ? might be 1.0225x slower 3d-morph 5.1717+-0.0509 ? 5.1923+-0.0660 ? 3d-raytrace 5.5222+-0.0303 ^ 5.2502+-0.0965 ^ definitely 1.0518x faster access-binary-trees 2.1145+-0.0198 ? 2.1724+-0.0559 ? might be 1.0274x slower access-fannkuch 5.9337+-0.0715 5.8528+-0.1176 might be 1.0138x faster access-nbody 2.6371+-0.1096 2.5488+-0.0300 might be 1.0347x faster access-nsieve 3.0457+-0.0229 ? 3.0681+-0.0387 ? bitops-3bit-bits-in-byte 1.1351+-0.0109 ? 1.1405+-0.0125 ? bitops-bits-in-byte 2.7861+-0.0213 ? 2.8001+-0.0502 ? bitops-bitwise-and 2.0675+-0.0332 2.0480+-0.0144 bitops-nsieve-bits 3.1379+-0.0645 3.1296+-0.0467 controlflow-recursive 2.3544+-0.0174 2.3503+-0.0337 crypto-aes 4.0292+-0.0413 ? 4.0324+-0.0696 ? crypto-md5 2.5080+-0.0444 2.5002+-0.0531 crypto-sha1 2.3578+-0.0277 2.3432+-0.0190 date-format-tofte 6.4053+-0.0903 ? 6.4075+-0.0770 ? date-format-xparb 4.8294+-0.1402 ? 4.8493+-0.1692 ? math-cordic 2.9286+-0.0389 2.8776+-0.0131 might be 1.0177x faster math-partial-sums 4.9225+-0.1037 4.8803+-0.0690 math-spectral-norm 1.9831+-0.0149 ? 1.9973+-0.0194 ? regexp-dna 6.3421+-0.0768 6.2721+-0.0501 might be 1.0112x faster string-base64 4.7411+-0.0730 ? 4.8863+-0.1663 ? might be 1.0306x slower string-fasta 5.9668+-0.1654 5.8749+-0.0860 might be 1.0157x faster string-tagcloud 8.1567+-0.0770 ? 8.2552+-0.2033 ? might be 1.0121x slower string-unpack-code 19.4009+-0.4762 ? 19.4415+-0.4419 ? string-validate-input 4.4489+-0.1219 4.3643+-0.0198 might be 1.0194x faster <arithmetic> 4.6120+-0.0239 4.6013+-0.0260 might be 1.0023x faster Conf#1 Conf#2 Octane: encrypt 0.16215+-0.00089 0.16188+-0.00077 decrypt 2.84181+-0.00700 2.83793+-0.00366 deltablue x2 0.13987+-0.00160 0.13956+-0.00119 earley 0.28584+-0.00080 ? 0.28594+-0.00089 ? boyer 5.01632+-0.06001 ? 5.02915+-0.05367 ? navier-stokes x2 4.99973+-0.00721 ? 5.00253+-0.00861 ? raytrace x2 0.79912+-0.00250 ? 0.80120+-0.00283 ? richards x2 0.08335+-0.00063 ? 0.08385+-0.00046 ? splay x2 0.34159+-0.00129 ? 0.34190+-0.00111 ? regexp x2 15.76614+-0.11218 ? 15.77291+-0.10053 ? pdfjs x2 39.76868+-0.27101 ? 39.80804+-0.28669 ? mandreel x2 42.61976+-0.08029 42.54716+-0.12615 gbemu x2 24.19040+-0.07575 24.14995+-0.08258 closure 0.53466+-0.00131 ^ 0.53167+-0.00110 ^ definitely 1.0056x faster jquery 6.83269+-0.02810 6.82235+-0.02935 box2d x2 9.17647+-0.03200 ? 9.18539+-0.02331 ? zlib x2 360.04454+-3.37642 358.57454+-4.06083 typescript x2 617.64299+-2.06962 ? 617.86455+-1.89938 ? <geometric> 5.02176+-0.00666 5.02161+-0.00704 might be 1.0000x faster Conf#1 Conf#2 Kraken: ai-astar 88.104+-0.407 ? 88.946+-1.186 ? audio-beat-detection 42.056+-0.063 ^ 41.061+-0.096 ^ definitely 1.0242x faster audio-dft 99.650+-0.822 99.536+-0.929 audio-fft 32.752+-0.019 ? 32.971+-0.364 ? audio-oscillator 47.862+-0.081 47.851+-0.075 imaging-darkroom 60.532+-0.482 ^ 59.988+-0.054 ^ definitely 1.0091x faster imaging-desaturate 45.517+-0.690 45.255+-0.166 imaging-gaussian-blur 64.913+-1.672 63.702+-2.006 might be 1.0190x faster json-parse-financial 37.855+-0.139 ? 37.921+-0.302 ? json-stringify-tinderbox 24.565+-0.790 24.554+-0.743 stanford-crypto-aes 39.708+-0.447 39.595+-0.794 stanford-crypto-ccm 32.765+-0.619 ? 33.094+-0.625 ? might be 1.0100x slower stanford-crypto-pbkdf2 96.472+-0.274 ? 96.727+-0.343 ? stanford-crypto-sha256-iterative 36.772+-0.126 36.634+-0.085 <arithmetic> 53.537+-0.190 53.417+-0.186 might be 1.0023x faster Conf#1 Conf#2 AsmBench: bigfib.cpp 441.5196+-0.7669 ? 443.0796+-1.7072 ? cray.c 363.6939+-0.5656 ? 364.3559+-2.4752 ? dry.c 461.6533+-27.8064 ? 462.1714+-28.9926 ? FloatMM.c 723.4377+-3.6195 ? 726.6473+-3.5986 ? gcc-loops.cpp 3709.0511+-4.2150 ? 3718.9008+-11.3078 ? n-body.c 808.7373+-1.9564 807.3993+-1.6012 Quicksort.c 398.2005+-0.7623 397.2776+-0.6105 stepanov_container.cpp 3310.4908+-12.0120 ? 3316.4831+-13.6885 ? Towers.c 273.4291+-0.9558 ? 273.5383+-0.8914 ? <geometric> 728.2313+-4.3860 ? 729.1486+-5.3243 ? might be 1.0013x slower Conf#1 Conf#2 Geomean of preferred means: <scaled-result> 30.8251+-0.0657 30.7990+-0.0761 might be 1.0008x faster
Comment on attachment 276564 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=276564&action=review > Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h:411 > + if (child.isNumber()) { > + setConstant(node, jsDoubleNumber(child.asNumber())); > + break; > + } > + if (child.isUndefined()) { > + setConstant(node, jsDoubleNumber(PNaN)); > + break; > + } > + if (child.isNull() || child.isFalse()) { > + setConstant(node, jsDoubleNumber(0)); > + break; > + } > + if (child.isTrue()) { > + setConstant(node, jsDoubleNumber(1)); > + break; > + } Seems like we would want this to share code with JSValue::toNumber function. This code should be something like this: if (child && !child.isCell()) { setConstant(node, jsDoubleNumber(child.toNumberNonCell())) break; } I suppose it’s OK to write it out like this instead, but it seems nice to concentrate this knowledge of how to turn values into numbers in one place.
(In reply to comment #3) > Seems like we would want this to share code with JSValue::toNumber function. > This code should be something like this: > > if (child && !child.isCell()) { > setConstant(node, jsDoubleNumber(child.toNumberNonCell())) > break; > } > > I suppose it’s OK to write it out like this instead, but it seems nice to > concentrate this knowledge of how to turn values into numbers in one place. I think this may be a bit dangerous for maintainability. In the abstract interpreter, we must ensure this transformation is thread-safe and without side effect. Someone could modify JSValue without knowing how it is used.
Comment on attachment 276564 [details] Patch Clearing flags on attachment: 276564 Committed r199639: <http://trac.webkit.org/changeset/199639>
All reviewed patches have been landed. Closing bug.
(In reply to comment #4) > (In reply to comment #3) > > Seems like we would want this to share code with JSValue::toNumber function. > > This code should be something like this: > > > > if (child && !child.isCell()) { > > setConstant(node, jsDoubleNumber(child.toNumberNonCell())) > > break; > > } > > > > I suppose it’s OK to write it out like this instead, but it seems nice to > > concentrate this knowledge of how to turn values into numbers in one place. > > I think this may be a bit dangerous for maintainability. > In the abstract interpreter, we must ensure this transformation is > thread-safe and without side effect. Someone could modify JSValue without > knowing how it is used. I understand what you are saying, but do not agree with you. All the non-cell transformations on JSValue have this property today. That’s what the entire class is about. The lack of a VM& or ExecState& argument is part of what makes this clear.