This will allow us to reason about not just whether a node is used, but whether it's used for a particular purpose.
Created attachment 125460 [details] it begins. Work in progress. Three files down, 51 to go.
Created attachment 125466 [details] getting closer
Created attachment 125517 [details] 64-bit works Still need to do 32-64 and make sure I didn't screw up performance.
Comment on attachment 125517 [details] 64-bit works Attachment 125517 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/11424834
Comment on attachment 125517 [details] 64-bit works Attachment 125517 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/11421879
Comment on attachment 125517 [details] 64-bit works Attachment 125517 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/11427085
<rdar://problem/10810732>
Created attachment 125537 [details] the patch Made both 32_64 and 64 work.
Comment on attachment 125537 [details] the patch Attachment 125537 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/11427257
Created attachment 125538 [details] the patch Fix a really bad operator precedence bug found by Qt EWS. Instead of writing (a << b) - c, I had written a << b - c, which is totally different.
Comment on attachment 125538 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=125538&action=review Other than the comments included (below?) this looks fairly sensible to me. > Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:173 > - nodeIndex = addToGraph(GetLocal, OpInfo(variableAccessData), nodePtr->child1()); > + nodeIndex = addToGraph(GetLocal, OpInfo(variableAccessData), nodePtr->child1().index()); It seems like it would be nicer to make addToGraph take a Node/NodeUse rather than a NodeIndex, which would remove some of the churn in this diff. > Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:2273 > + phiNode->children.child1() = NodeUse(valueInPredecessor); I'd prefer setChild1(..) to returning a mutable reference.
(In reply to comment #11) > (From update of attachment 125538 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=125538&action=review > > Other than the comments included (below?) this looks fairly sensible to me. > > > Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:173 > > - nodeIndex = addToGraph(GetLocal, OpInfo(variableAccessData), nodePtr->child1()); > > + nodeIndex = addToGraph(GetLocal, OpInfo(variableAccessData), nodePtr->child1().index()); > > It seems like it would be nicer to make addToGraph take a Node/NodeUse rather than a NodeIndex, which would remove some of the churn in this diff. Do you really think that's a good idea? There are 4 addToGraph() methods, and only 2 places where this patch needs to convert a NodeUse to NodeIndex for a call to addToGraph(). In general, ByteCodeParser almost never needs to create a node that references a child of another node - which is the only case where it would make sense for addToGraph() to take NodeUse. > > > Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:2273 > > + phiNode->children.child1() = NodeUse(valueInPredecessor); > > I'd prefer setChild1(..) to returning a mutable reference. I can change those things, but it is necessary for NodeReferenceBlob to return a mutable reference. Otherwise we'd have no way of later doing a pass that ascribes types to edges.
Created attachment 125545 [details] the patch Changed all present uses of childX() = foo with setChildX(foo). I did not remove the childX() methods that returned mutable references, because that would defeat the entire purpose of this patch.
Landed in http://trac.webkit.org/changeset/106775