Bug 216669 – DFG should ensure there are PhantomLocals for the taken block of op_jneq_ptr

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Keith Miller]

DFG should ensure there are PhantomLocals for the not-taken block of op_jneq_ptr

[Keith Miller]

Created attachment 409077 [details]
Patch

[Keith Miller]

Created attachment 409079 [details]
Patch

[Keith Miller]

rdar://68968982

[Saam Barati]

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?

[Keith Miller]

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.

[Saam Barati]

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

[Keith Miller]

Created attachment 409153 [details]
Patch for landing

[EWS]

Committed r267255: <https://trac.webkit.org/changeset/267255>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 409153 [details].

[Radar WebKit Bug Importer]

<rdar://problem/69168812>