There were issues with how we used to do this. So I rolled it out in r242989 thinking it might not be a perf regression to remove it. It turns out that removing this is a 1% speedometer2 regression. I'll add back the optimization but this time making it sound.
Created attachment 365069 [details] patch
Attachment 365069 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:7333: This { should be at the end of the previous line [whitespace/braces] [4] Total errors found: 1 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.
<rdar://problem/48991872>
Comment on attachment 365069 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=365069&action=review r=me. > Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:7400 > + node->removeWithoutChecks(); Maybe put a comment here that this is needed to make sure that we don't emit phantoms for the results of opcodes that we are going to remove.
Comment on attachment 365069 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=365069&action=review >> Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:7400 >> + node->removeWithoutChecks(); > > Maybe put a comment here that this is needed to make sure that we don't emit phantoms for the results of opcodes that we are going to remove. I'm not super sure a comment here is super helpful. We're also removing nodes because we're bumping nodeIndex in the loop below. So we end up inserting phantoms after some of these things we're converting to Check(). So it serves as two purposes: don't emit Phantom on things defined after ForceOSRExit. And don't have those nodes be anything besides a Check()
Comment on attachment 365069 [details] patch Clearing flags on attachment: 365069 Committed r243176: <https://trac.webkit.org/changeset/243176>
All reviewed patches have been landed. Closing bug.