This will follow the theme of putting other optimization information on UnlinkedCodeBlock. This allows the unlinked code cache to remember OSR exit data. Obviously this can make some programs worse: if the same exact code is run in a different way. However, it's probably a solid heuristic in practice, since the same code tends to run in similar ways, and do similar things. Doing this appears to be a 1% speedup on Speedometer.
Created attachment 331152 [details] WIP
Attachment 331152 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/bytecode/DFGExitProfile.h:178: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/bytecode/DFGExitProfile.h:179: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/bytecode/DFGExitProfile.h:181: Should have a space between // and comment [whitespace/comments] [4] Total errors found: 3 in 15 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 331236 [details] patch
Comment on attachment 331236 [details] patch r=me
Comment on attachment 331236 [details] patch Clearing flags on attachment: 331236 Committed r226928: <https://trac.webkit.org/changeset/226928>
All reviewed patches have been landed. Closing bug.
<rdar://problem/36486548>
This broke the CLOOP build. Fixing now.
(In reply to Saam Barati from comment #8) > This broke the CLOOP build. Fixing now. Fixed CLOOP build in: https://trac.webkit.org/changeset/226942/webkit
Comment on attachment 331236 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=331236&action=review > Source/JavaScriptCore/dfg/DFGLICMPhase.cpp:284 > - && m_graph.baselineCodeBlockFor(originalOrigin.semantic)->hasExitSite(FrequentExitSite(HoistingFailed))) { > + && m_graph.hasExitSite(originalOrigin.semantic, HoistingFailed)) { This introduced a licm-dragons regression because you should have called hasGlobalExitSite.
Comment on attachment 331236 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=331236&action=review B >> Source/JavaScriptCore/dfg/DFGLICMPhase.cpp:284 >> + && m_graph.hasExitSite(originalOrigin.semantic, HoistingFailed)) { > > This introduced a licm-dragons regression because you should have called hasGlobalExitSite. How is this different than what was here before?
(In reply to Saam Barati from comment #11) > Comment on attachment 331236 [details] > patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=331236&action=review > > B > > >> Source/JavaScriptCore/dfg/DFGLICMPhase.cpp:284 > >> + && m_graph.hasExitSite(originalOrigin.semantic, HoistingFailed)) { > > > > This introduced a licm-dragons regression because you should have called hasGlobalExitSite. > > How is this different than what was here before? Global exit sites don’t store the bytecode index.
(In reply to Filip Pizlo from comment #12) > (In reply to Saam Barati from comment #11) > > Comment on attachment 331236 [details] > > patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=331236&action=review > > > > B > > > > >> Source/JavaScriptCore/dfg/DFGLICMPhase.cpp:284 > > >> + && m_graph.hasExitSite(originalOrigin.semantic, HoistingFailed)) { > > > > > > This introduced a licm-dragons regression because you should have called hasGlobalExitSite. > > > > How is this different than what was here before? > > Global exit sites don’t store the bytecode index. I understand that. But the code wasn’t using global exit site before my change either...
(In reply to Saam Barati from comment #13) > (In reply to Filip Pizlo from comment #12) > > (In reply to Saam Barati from comment #11) > > > Comment on attachment 331236 [details] > > > patch > > > > > > View in context: > > > https://bugs.webkit.org/attachment.cgi?id=331236&action=review > > > > > > B > > > > > > >> Source/JavaScriptCore/dfg/DFGLICMPhase.cpp:284 > > > >> + && m_graph.hasExitSite(originalOrigin.semantic, HoistingFailed)) { > > > > > > > > This introduced a licm-dragons regression because you should have called hasGlobalExitSite. > > > > > > How is this different than what was here before? > > > > Global exit sites don’t store the bytecode index. > > I understand that. But the code wasn’t using global exit site before my > change either... Oh wait. Yeah it was. I see