[JSC] Temporal Dead Zone checks on "this" are eliminated when doing OSR Entry to FTL
Created attachment 272168 [details] Patch
I went this way because that was easy :) I am not familiar with the responsibilities of VariableAccessData. Would it make more sense to have a flush format FlushedJSValueOrEmpty and a flag on VariableAccessData when empty is an option?
Comment on attachment 272168 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=272168&action=review > Source/JavaScriptCore/ChangeLog:9 > + that this may have been set to null by a previously executed block. "null" or the empty value? Two different things and I think you mean empty value. > Source/JavaScriptCore/ChangeLog:14 > + In this patch, I added a simple workaround: A new node CanBeEmptyHint This is really ugly. Can we just make ExtractOSREntryLocal have a flag or something that indicates that it might return the empty value?
Comment on attachment 272168 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=272168&action=review > Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp:166 > + changed |= variableAccessData->predict(SpecBytecodeTop); This is extremely bad. You're turning off all type inference for 'this'!
(In reply to comment #3) > Comment on attachment 272168 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=272168&action=review > > > Source/JavaScriptCore/ChangeLog:9 > > + that this may have been set to null by a previously executed block. > > "null" or the empty value? Two different things and I think you mean empty > value. > > > Source/JavaScriptCore/ChangeLog:14 > > + In this patch, I added a simple workaround: A new node CanBeEmptyHint > > This is really ugly. Can we just make ExtractOSREntryLocal have a flag or > something that indicates that it might return the empty value? Right - you can't do this because there is no ExtractOSREntryLocal. I think that this problem has nothing to do with prediction propagation. Why isn't this an AbstractInterpreter change that makes sure that AI knows that 'this' could be the empty value?
Created attachment 272250 [details] Patch
I was convinced this would be a perf regression but it is completely neutral: Conf#1 Conf#2 Octane: encrypt 0.26922+-0.00229 0.26788+-0.00274 decrypt 4.98148+-0.02076 ? 4.98607+-0.01449 ? deltablue x2 0.22267+-0.00673 0.22164+-0.00638 earley 0.51225+-0.00240 0.50874+-0.00208 boyer 8.34783+-0.03074 8.34247+-0.03670 navier-stokes x2 6.47051+-0.00903 ? 6.47304+-0.00743 ? raytrace x2 1.51409+-0.01446 1.51369+-0.01413 richards x2 0.14226+-0.00122 0.14169+-0.00130 splay x2 0.53940+-0.00313 0.53757+-0.00336 regexp x2 39.26591+-0.32509 ? 39.48002+-0.62183 ? pdfjs x2 61.29357+-0.26341 61.11385+-0.19545 mandreel x2 70.67145+-0.65415 70.09903+-0.42861 gbemu x2 46.61627+-1.11143 46.12028+-0.78242 might be 1.0108x faster closure 0.93911+-0.00197 0.93727+-0.00265 jquery 11.91794+-0.07194 ? 12.01694+-0.16999 ? box2d x2 16.65716+-0.12012 16.64404+-0.08303 zlib x2 582.30538+-2.23464 ? 582.94125+-1.60820 ? typescript x2 1115.89500+-3.30588 ? 1119.10688+-6.57086 ? <geometric> 8.65685+-0.03625 8.64086+-0.02104 might be 1.0019x faster Conf#1 Conf#2 Kraken: ai-astar 151.121+-3.629 ? 151.314+-1.159 ? audio-beat-detection 71.331+-0.263 ? 71.519+-0.323 ? audio-dft 127.270+-1.026 ? 127.357+-0.840 ? audio-fft 57.120+-0.125 57.041+-0.117 audio-oscillator 85.345+-0.151 ? 85.349+-0.158 ? imaging-darkroom 95.645+-0.178 95.468+-0.133 imaging-desaturate 92.703+-0.271 92.581+-0.333 imaging-gaussian-blur 132.539+-2.278 130.049+-3.360 might be 1.0191x faster json-parse-financial 66.548+-0.428 ? 66.781+-0.349 ? json-stringify-tinderbox 40.954+-0.088 ? 41.128+-0.320 ? stanford-crypto-aes 63.024+-0.348 62.991+-0.614 stanford-crypto-ccm 60.982+-1.559 ? 61.174+-1.411 ? stanford-crypto-pbkdf2 153.449+-1.405 152.747+-1.040 stanford-crypto-sha256-iterative 59.976+-0.413 ? 60.595+-0.440 ? might be 1.0103x slower <arithmetic> 89.858+-0.345 89.721+-0.259 might be 1.0015x faster Conf#1 Conf#2 AsmBench: bigfib.cpp 661.8311+-9.5454 657.4292+-6.3489 cray.c 577.4569+-2.2899 ? 580.9863+-2.2050 ? dry.c 675.2116+-17.1696 674.9946+-7.3268 FloatMM.c 907.1338+-8.6865 897.4068+-8.5989 might be 1.0108x faster gcc-loops.cpp 6307.8684+-6.3469 ? 6316.4279+-9.5400 ? n-body.c 1583.8569+-3.1433 1582.5694+-2.4083 Quicksort.c 589.0678+-1.4176 ? 589.9603+-1.7609 ? stepanov_container.cpp 4538.2725+-32.5647 ? 4548.8271+-27.7810 ? Towers.c 386.0708+-0.3375 ? 386.7750+-0.8667 ? <geometric> 1102.7601+-2.8050 1102.1545+-1.5262 might be 1.0005x faster Conf#1 Conf#2 Geomean of preferred means: <scaled-result> 95.0157+-0.2075 94.8919+-0.0949 might be 1.0013x faster
Comment on attachment 272250 [details] Patch Clearing flags on attachment: 272250 Committed r197155: <http://trac.webkit.org/changeset/197155>
All reviewed patches have been landed. Closing bug.