[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





--- Comment #7 from Yuqiang Xian <yuqiang.xian at intel.com>  2012-02-29 19:47:19 PST ---
(In reply to comment #6)
> (From update of attachment 129614 [details])
> 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.

Yes. You're absolutely right. I didn't touch CSE for now - actually it still relies on the assumption that the node indicies are continuous, which is true in current stage as we exclude the inserted Phi nodes. But yes we should fix it.

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