[webkit-reviews] review granted: [Bug 124181] FTL should have an explicit notion of bytecode liveness : [Attachment 217073] the patch

[bugzilla-daemon at webkit.org]

Mark Hahnenberg <mhahnenberg at apple.com> has granted Filip Pizlo
<fpizlo at apple.com>'s request for review:
Bug 124181: FTL should have an explicit notion of bytecode liveness
https://bugs.webkit.org/show_bug.cgi?id=124181

Attachment 217073: the patch
https://bugs.webkit.org/attachment.cgi?id=217073&action=review

------- Additional Comments from Mark Hahnenberg <mhahnenberg at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=217073&action=review


r=me

> Source/JavaScriptCore/dfg/DFGGraph.cpp:676
> +		   if (reg.offset() >= JSStack::CallFrameHeaderSize)
> +		       return true;

Maybe we should ASSERT this instead, since it should be impossible to take this
branch.

> Source/JavaScriptCore/dfg/DFGMovHintCountingPhase.h:45
> +// Computes VariableAccessData::m_movHintCount. Variables that are assigned
only once can get
> +// treated specially. For example, OSR exit in the FTL can allow the format
used for such
> +// nodes BasicBlock::ssa->availabiltiyAtHead/Tail. This is a forward flow
type inference
> +// over MovHints. At the time when MovHints are introduced, we were certain
that at any
> +// point in the program where a bytecode variable had been live, all of the
MovHints that
> +// flowed into that point for that variable had the same FlushFormat. This
analysis will tell
> +// you what that format would have been.

Comment seems whacky.

> Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp:3960
> +	   else if (direction == ForwardSpeculation) {

else?

> Source/JavaScriptCore/ftl/FTLOSRExit.cpp:76
> -    
> +

Revert!