[Webkit-unassigned] [Bug 79899] DFG BasicBlocks should not require that their nodes have contiguous indices in the graph

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=79899


Filip Pizlo <fpizlo at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #129614|review?                     |review-
               Flag|                            |




--- Comment #6 from Filip Pizlo <fpizlo at apple.com>  2012-02-29 19:42:24 PST ---
(From update of attachment 129614)
View in context: https://bugs.webkit.org/attachment.cgi?id=129614&action=review

This is really awesome.  R- but only because I want to make sure we do CSE right.

> Source/JavaScriptCore/dfg/DFGCSEPhase.cpp:720
>      
>      void performBlockCSE(BasicBlock& block)
>      {
> -        m_start = block.begin;
> -        NodeIndex end = block.end;
> -        for (m_compileIndex = m_start; m_compileIndex < end; ++m_compileIndex)
> +        m_start = block[block.startExcludingPhis];
> +        for (unsigned i = block.startExcludingPhis; i < block.size(); ++i) {
> +            m_compileIndex = block[i];
>              performNodeCSE(m_graph[m_compileIndex]);
> +        }
>      }
>      
>      NodeIndex m_start;

I'm somewhat surprised that these are the only changes in CSE!  CSE has this whole performance hack where it uses the NodeIndex of the instruction that is a candidate for elimination, and the NodeIndices of its children, to determine the range that it will search.

Seems like that relies on NodeIndices of things in a basic block being monotonically increasing.

Would be better to instead make all of the search things instead break early when they encounter a NodeIndex at block[i] that matches the NodeIndex in one of the children of the node from which the search originated.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.