| Summary: | DFG should ensure there are PhantomLocals for the taken block of op_jneq_ptr | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Keith Miller <keith_miller> | ||||||||
| Component: | New Bugs | Assignee: | Keith Miller <keith_miller> | ||||||||
| Status: | RESOLVED FIXED | ||||||||||
| Severity: | Normal | CC: | ews-watchlist, mark.lam, msaboff, saam, tzagallo, webkit-bug-importer | ||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||
| Version: | WebKit Nightly Build | ||||||||||
| Hardware: | Unspecified | ||||||||||
| OS: | Unspecified | ||||||||||
| Attachments: |
|
||||||||||
|
Description
Keith Miller
2020-09-17 16:09:22 PDT
Created attachment 409077 [details]
Patch
Created attachment 409079 [details]
Patch
Comment on attachment 409079 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=409079&action=review > Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:7175 > + flushForTerminalImpl(CodeOrigin(BytecodeIndex(m_currentIndex.offset() + relativeOffset), inlineCallFrame()), addFlushDirect, addPhantomLocalDirect); why not the current code origin? Comment on attachment 409079 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=409079&action=review >> Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:7175 >> + flushForTerminalImpl(CodeOrigin(BytecodeIndex(m_currentIndex.offset() + relativeOffset), inlineCallFrame()), addFlushDirect, addPhantomLocalDirect); > > why not the current code origin? Because the set of live locals at the target is a subset of the locals live here. There's no point in Phantoming things that we don't need to. Comment on attachment 409079 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=409079&action=review >>> Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:7175 >>> + auto addPhantomLocalDirect = [&] (InlineCallFrame*, Operand operand) { phantomLocalDirect(operand); }; >>> + flushForTerminalImpl(CodeOrigin(BytecodeIndex(m_currentIndex.offset() + relativeOffset), inlineCallFrame()), addFlushDirect, addPhantomLocalDirect); >> >> why not the current code origin? > > Because the set of live locals at the target is a subset of the locals live here. There's no point in Phantoming things that we don't need to. let's add the comment on why we flushForTerminal here, since we don't know if the target would've ended up doing it Created attachment 409153 [details]
Patch for landing
Committed r267255: <https://trac.webkit.org/changeset/267255> All reviewed patches have been landed. Closing bug and clearing flags on attachment 409153 [details]. |