[Webkit-unassigned] [Bug 86905] DFG CFA should record if a node can OSR exit

[bugzilla-daemon at webkit.org]

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





--- Comment #5 from Oliver Hunt <oliver at apple.com>  2012-05-20 06:08:37 PST ---
(From update of attachment 142895)
View in context: https://bugs.webkit.org/attachment.cgi?id=142895&action=review

I'm too tired to actually finish this review (and i'm concerned about my ability to follow some of it right now), so i'll finish up/re-review later today.  I only got about half way through and it mostly looks fine, aside from your attempts to save columns ;p

I think it might be worth waiting until you've got the dfgopt branch merged completely to trunk, as I think we've dropped ByteArray from the tree, so there's a bit of code that will just disappear from this patch.

> Source/JavaScriptCore/dfg/DFGAbstractState.cpp:441
> +            speculateInt32Binary(
> +                node, !nodeCanTruncateInteger(node.arithNodeFlags()));

one line we don't have an 80 column limit :P

> Source/JavaScriptCore/dfg/DFGAbstractState.cpp:473
> +            speculateInt32Binary(
> +                node, !nodeCanTruncateInteger(node.arithNodeFlags()));

and again

> Source/JavaScriptCore/dfg/DFGAbstractState.cpp:514
> +            speculateInt32Binary(
> +                node,
> +                !nodeCanTruncateInteger(node.arithNodeFlags())
> +                || !nodeCanIgnoreNegativeZero(node.arithNodeFlags()));

see how bad this is at one line.  I'd prefer a single line, but this may be excessive

> Source/JavaScriptCore/dfg/DFGAbstractState.cpp:555
> +        if (Node::shouldSpeculateInteger(
> +                m_graph[node.child1()], m_graph[node.child2()])
> +            && node.canSpeculateInteger()) {

one line

> Source/JavaScriptCore/dfg/DFGAbstractState.cpp:575
> +        if (m_graph[node.child1()].shouldSpeculateInteger()
> +            && node.canSpeculateInteger()) {
> +            speculateInt32Unary(node, true);

one line.  Are you doing this out of some desire to punish me?  What did i ever do to you? :-(

> Source/JavaScriptCore/dfg/DFGAbstractState.cpp:797
> +        if (Node::shouldSpeculateInteger(
> +                m_graph[node.child1()], m_graph[node.child2()])) {

one line

> Source/JavaScriptCore/dfg/DFGAbstractState.cpp:802
> +        if (Node::shouldSpeculateNumber(
> +                m_graph[node.child1()], m_graph[node.child2()])) {

one line

> Source/JavaScriptCore/dfg/DFGAbstractState.cpp:807
> +        if (Node::shouldSpeculateFinalObject(
> +                m_graph[node.child1()], m_graph[node.child2()])) {

one line

> Source/JavaScriptCore/dfg/DFGAbstractState.cpp:810
> +                !isFinalObjectPrediction(forNode(node.child1()).m_type)
> +                || !isFinalObjectPrediction(forNode(node.child2()).m_type));

i'd prefer one line here i think

> Source/JavaScriptCore/dfg/DFGAbstractState.cpp:819
> +        if (Node::shouldSpeculateArray(
> +                m_graph[node.child1()], m_graph[node.child2()])) {
> +            node.setCanExit(
> +                !isArrayPrediction(forNode(node.child1()).m_type)
> +                || !isArrayPrediction(forNode(node.child2()).m_type));

less lines, moar columns!!!

> Source/JavaScriptCore/dfg/DFGAbstractState.cpp:1065
> +        node.setCanExit(
> +            !isCellPrediction(forNode(node.child1()).m_type)
> +            || !isCellPrediction(forNode(node.child2()).m_type));

and again

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