Attachment 211263 Details for Bug 121132 – the patch

[patch] the patch

blah.patch (text/plain), 13.18 KB, created by Filip Pizlo on 2013-09-10 16:38:39 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Filip Pizlo

Created: 2013-09-10 16:38:39 PDT

Size: 13.18 KB

patch

obsolete

>Index: Source/JavaScriptCore/ChangeLog
>===================================================================
>--- Source/JavaScriptCore/ChangeLog	(revision 155491)
>+++ Source/JavaScriptCore/ChangeLog	(working copy)
>@@ -1,3 +1,36 @@
>+2013-09-10  Filip Pizlo  <fpizlo@apple.com>
>+
>+        Propagate the Int48 stuff into the prediction propagator.
>+        https://bugs.webkit.org/show_bug.cgi?id=121132
>+
>+        Reviewed by NOBODY (OOPS!).
>+        
>+        This still has no effect on codegen since Int48 still looks like a Double right now.
>+
>+        * bytecode/ExitKind.cpp:
>+        (JSC::exitKindToString):
>+        * bytecode/ExitKind.h:
>+        * bytecode/SpeculatedType.h:
>+        (JSC::isMachineIntSpeculation):
>+        (JSC::isMachineIntSpeculationExpectingDefined):
>+        (JSC::isMachineIntSpeculationForArithmetic):
>+        * dfg/DFGGraph.h:
>+        (JSC::DFG::Graph::addShouldSpeculateMachineInt):
>+        (JSC::DFG::Graph::mulShouldSpeculateInt32):
>+        (JSC::DFG::Graph::mulShouldSpeculateMachineInt):
>+        (JSC::DFG::Graph::negateShouldSpeculateMachineInt):
>+        (JSC::DFG::Graph::hasExitSite):
>+        * dfg/DFGNode.h:
>+        (JSC::DFG::Node::shouldSpeculateMachineInt):
>+        (JSC::DFG::Node::shouldSpeculateMachineIntForArithmetic):
>+        (JSC::DFG::Node::shouldSpeculateMachineIntExpectingDefined):
>+        (JSC::DFG::Node::canSpeculateInt48):
>+        * dfg/DFGNodeFlags.h:
>+        (JSC::DFG::nodeMayNegZero):
>+        (JSC::DFG::nodeCanSpeculateInt48):
>+        * dfg/DFGPredictionPropagationPhase.cpp:
>+        (JSC::DFG::PredictionPropagationPhase::propagate):
>+
> 2013-09-10  Mark Hahnenberg  <mhahnenberg@apple.com>
> 
>         MapData has some issues
>Index: Source/JavaScriptCore/bytecode/ExitKind.cpp
>===================================================================
>--- Source/JavaScriptCore/bytecode/ExitKind.cpp	(revision 155488)
>+++ Source/JavaScriptCore/bytecode/ExitKind.cpp	(working copy)
>@@ -52,6 +52,8 @@ const char* exitKindToString(ExitKind ki
>         return "Overflow";
>     case NegativeZero:
>         return "NegativeZero";
>+    case Int48Overflow:
>+        return "Int48Overflow";
>     case StoreToHole:
>         return "StoreToHole";
>     case LoadFromHole:
>Index: Source/JavaScriptCore/bytecode/ExitKind.h
>===================================================================
>--- Source/JavaScriptCore/bytecode/ExitKind.h	(revision 155488)
>+++ Source/JavaScriptCore/bytecode/ExitKind.h	(working copy)
>@@ -38,6 +38,7 @@ enum ExitKind {
>     BadIndexingType, // We exited because an indexing type was wrong.
>     Overflow, // We exited because of overflow.
>     NegativeZero, // We exited because we encountered negative zero.
>+    Int48Overflow, // We exited because of an Int48 overflow.
>     StoreToHole, // We had a store to a hole.
>     LoadFromHole, // We had a load from a hole.
>     OutOfBounds, // We had an out-of-bounds access to an array.
>Index: Source/JavaScriptCore/bytecode/SpeculatedType.h
>===================================================================
>--- Source/JavaScriptCore/bytecode/SpeculatedType.h	(revision 155488)
>+++ Source/JavaScriptCore/bytecode/SpeculatedType.h	(working copy)
>@@ -63,6 +63,7 @@ static const SpeculatedType SpecCellOthe
> static const SpeculatedType SpecCell              = 0x0007ffff; // It's definitely a JSCell.
> static const SpeculatedType SpecInt32             = 0x00800000; // It's definitely an Int32.
> static const SpeculatedType SpecInt48             = 0x01000000; // It's definitely an Int48, it's inside a double, but it doesn't need to be.
>+static const SpeculatedType SpecMachineInt        = 0x01800000; // It's something that we can do machine int arithmetic on.
> static const SpeculatedType SpecInt48AsDouble     = 0x02000000; // It's definitely an Int48 and it's inside a double.
> static const SpeculatedType SpecInteger           = 0x03800000; // It's definitely some kind of integer.
> static const SpeculatedType SpecNonIntAsDouble    = 0x04000000; // It's definitely not an Int48 but it's a real number and it's a double.
>@@ -252,6 +253,21 @@ inline bool isInt48Speculation(Speculate
>     return value == SpecInt48;
> }
> 
>+inline bool isMachineIntSpeculation(SpeculatedType value)
>+{
>+    return !!value && (value & SpecMachineInt) == value;
>+}
>+
>+inline bool isMachineIntSpeculationExpectingDefined(SpeculatedType value)
>+{
>+    return isMachineIntSpeculation(value & ~SpecOther);
>+}
>+
>+inline bool isMachineIntSpeculationForArithmetic(SpeculatedType value)
>+{
>+    return !(value & (SpecDouble & ~SpecMachineInt));
>+}
>+
> inline bool isInt48AsDoubleSpeculation(SpeculatedType value)
> {
>     return value == SpecInt48AsDouble;
>Index: Source/JavaScriptCore/dfg/DFGGraph.h
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGGraph.h	(revision 155488)
>+++ Source/JavaScriptCore/dfg/DFGGraph.h	(working copy)
>@@ -228,6 +228,20 @@ public:
>         return addSpeculationMode(add) != DontSpeculateInt32;
>     }
>     
>+    bool addShouldSpeculateMachineInt(Node* add)
>+    {
>+        Node* left = add->child1().node();
>+        Node* right = add->child2().node();
>+
>+        bool speculation;
>+        if (add->op() == ValueAdd)
>+            speculation = Node::shouldSpeculateMachineIntExpectingDefined(left, right);
>+        else
>+            speculation = Node::shouldSpeculateMachineIntForArithmetic(left, right);
>+
>+        return speculation && !hasExitSite(add, Int48Overflow);
>+    }
>+    
>     bool mulShouldSpeculateInt32(Node* mul)
>     {
>         ASSERT(mul->op() == ArithMul);
>@@ -235,7 +249,20 @@ public:
>         Node* left = mul->child1().node();
>         Node* right = mul->child2().node();
>         
>-        return Node::shouldSpeculateInt32ForArithmetic(left, right) && mul->canSpeculateInt32();
>+        return Node::shouldSpeculateInt32ForArithmetic(left, right)
>+            && mul->canSpeculateInt32();
>+    }
>+    
>+    bool mulShouldSpeculateMachineInt(Node* mul)
>+    {
>+        ASSERT(mul->op() == ArithMul);
>+        
>+        Node* left = mul->child1().node();
>+        Node* right = mul->child2().node();
>+
>+        return Node::shouldSpeculateMachineIntForArithmetic(left, right)
>+            && mul->canSpeculateInt48()
>+            && !hasExitSite(mul, Int48Overflow);
>     }
>     
>     bool negateShouldSpeculateInt32(Node* negate)
>@@ -244,6 +271,14 @@ public:
>         return negate->child1()->shouldSpeculateInt32ForArithmetic() && negate->canSpeculateInt32();
>     }
>     
>+    bool negateShouldSpeculateMachineInt(Node* negate)
>+    {
>+        ASSERT(negate->op() == ArithNegate);
>+        return negate->child1()->shouldSpeculateMachineIntForArithmetic()
>+            && !hasExitSite(negate, Int48Overflow)
>+            && negate->canSpeculateInt48();
>+    }
>+    
>     // Helper methods to check nodes for constants.
>     bool isConstant(Node* node)
>     {
>@@ -380,6 +415,11 @@ public:
>         return baselineCodeBlockFor(codeOrigin)->hasExitSite(FrequentExitSite(codeOrigin.bytecodeIndex, exitKind));
>     }
>     
>+    bool hasExitSite(Node* node, ExitKind exitKind)
>+    {
>+        return hasExitSite(node->codeOrigin, exitKind);
>+    }
>+    
>     int argumentsRegisterFor(const CodeOrigin& codeOrigin)
>     {
>         if (!codeOrigin.inlineCallFrame)
>Index: Source/JavaScriptCore/dfg/DFGNode.h
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGNode.h	(revision 155488)
>+++ Source/JavaScriptCore/dfg/DFGNode.h	(working copy)
>@@ -1214,6 +1214,21 @@ struct Node {
>         return isInt32SpeculationExpectingDefined(prediction());
>     }
>     
>+    bool shouldSpeculateMachineInt()
>+    {
>+        return isMachineIntSpeculation(prediction());
>+    }
>+    
>+    bool shouldSpeculateMachineIntForArithmetic()
>+    {
>+        return isMachineIntSpeculationForArithmetic(prediction());
>+    }
>+    
>+    bool shouldSpeculateMachineIntExpectingDefined()
>+    {
>+        return isMachineIntSpeculationExpectingDefined(prediction());
>+    }
>+    
>     bool shouldSpeculateDouble()
>     {
>         return isDoubleSpeculation(prediction());
>@@ -1364,6 +1379,21 @@ struct Node {
>         return op1->shouldSpeculateInt32ExpectingDefined() && op2->shouldSpeculateInt32ExpectingDefined();
>     }
>     
>+    static bool shouldSpeculateMachineInt(Node* op1, Node* op2)
>+    {
>+        return op1->shouldSpeculateMachineInt() && op2->shouldSpeculateMachineInt();
>+    }
>+    
>+    static bool shouldSpeculateMachineIntForArithmetic(Node* op1, Node* op2)
>+    {
>+        return op1->shouldSpeculateMachineIntForArithmetic() && op2->shouldSpeculateMachineIntForArithmetic();
>+    }
>+    
>+    static bool shouldSpeculateMachineIntExpectingDefined(Node* op1, Node* op2)
>+    {
>+        return op1->shouldSpeculateMachineIntExpectingDefined() && op2->shouldSpeculateMachineIntExpectingDefined();
>+    }
>+    
>     static bool shouldSpeculateDoubleForArithmetic(Node* op1, Node* op2)
>     {
>         return op1->shouldSpeculateDoubleForArithmetic() && op2->shouldSpeculateDoubleForArithmetic();
>@@ -1394,6 +1424,11 @@ struct Node {
>         return nodeCanSpeculateInt32(arithNodeFlags());
>     }
>     
>+    bool canSpeculateInt48()
>+    {
>+        return nodeCanSpeculateInt48(arithNodeFlags());
>+    }
>+    
>     void dumpChildren(PrintStream& out)
>     {
>         if (!child1())
>Index: Source/JavaScriptCore/dfg/DFGNodeFlags.h
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGNodeFlags.h	(revision 155488)
>+++ Source/JavaScriptCore/dfg/DFGNodeFlags.h	(working copy)
>@@ -91,6 +91,11 @@ static inline bool nodeMayOverflow(NodeF
>     return !!(flags & NodeMayOverflow);
> }
> 
>+static inline bool nodeMayNegZero(NodeFlags flags)
>+{
>+    return !!(flags & NodeMayNegZero);
>+}
>+
> static inline bool nodeCanSpeculateInt32(NodeFlags flags)
> {
>     if (flags & NodeMayOverflow)
>@@ -102,6 +107,14 @@ static inline bool nodeCanSpeculateInt32
>     return true;
> }
> 
>+static inline bool nodeCanSpeculateInt48(NodeFlags flags)
>+{
>+    if (flags & NodeMayNegZero)
>+        return nodeCanIgnoreNegativeZero(flags);
>+    
>+    return true;
>+}
>+
> void dumpNodeFlags(PrintStream&, NodeFlags);
> MAKE_PRINT_ADAPTOR(NodeFlagsDump, NodeFlags, dumpNodeFlags);
> 
>Index: Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp	(revision 155488)
>+++ Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp	(working copy)
>@@ -141,7 +141,10 @@ private:
>         switch (op) {
>         case JSConstant:
>         case WeakJSConstant: {
>-            changed |= setPrediction(speculationFromValue(m_graph.valueOfJSConstant(node)));
>+            SpeculatedType type = speculationFromValue(m_graph.valueOfJSConstant(node));
>+            if (type == SpecInt48AsDouble)
>+                type = SpecInt48;
>+            changed |= setPrediction(type);
>             break;
>         }
>             
>@@ -212,6 +215,8 @@ private:
>                 if (isNumberSpeculationExpectingDefined(left) && isNumberSpeculationExpectingDefined(right)) {
>                     if (m_graph.addSpeculationMode(node) != DontSpeculateInt32)
>                         changed |= mergePrediction(SpecInt32);
>+                    else if (m_graph.addShouldSpeculateMachineInt(node))
>+                        changed |= mergePrediction(SpecInt48);
>                     else
>                         changed |= mergePrediction(speculatedDoubleTypeForPredictions(left, right));
>                 } else if (!(left & SpecNumber) || !(right & SpecNumber)) {
>@@ -230,6 +235,8 @@ private:
>             if (left && right) {
>                 if (m_graph.addSpeculationMode(node) != DontSpeculateInt32)
>                     changed |= mergePrediction(SpecInt32);
>+                else if (m_graph.addShouldSpeculateMachineInt(node))
>+                    changed |= mergePrediction(SpecInt48);
>                 else
>                     changed |= mergePrediction(speculatedDoubleTypeForPredictions(left, right));
>             }
>@@ -243,6 +250,8 @@ private:
>             if (left && right) {
>                 if (m_graph.addSpeculationMode(node) != DontSpeculateInt32)
>                     changed |= mergePrediction(SpecInt32);
>+                else if (m_graph.addShouldSpeculateMachineInt(node))
>+                    changed |= mergePrediction(SpecInt48);
>                 else
>                     changed |= mergePrediction(speculatedDoubleTypeForPredictions(left, right));
>             }
>@@ -253,6 +262,8 @@ private:
>             if (node->child1()->prediction()) {
>                 if (m_graph.negateShouldSpeculateInt32(node))
>                     changed |= mergePrediction(SpecInt32);
>+                else if (m_graph.negateShouldSpeculateMachineInt(node))
>+                    changed |= mergePrediction(SpecInt48);
>                 else
>                     changed |= mergePrediction(speculatedDoubleTypeForPrediction(node->child1()->prediction()));
>             }
>@@ -280,6 +291,8 @@ private:
>             if (left && right) {
>                 if (m_graph.mulShouldSpeculateInt32(node))
>                     changed |= mergePrediction(SpecInt32);
>+                else if (m_graph.mulShouldSpeculateMachineInt(node))
>+                    changed |= mergePrediction(SpecInt48);
>                 else
>                     changed |= mergePrediction(speculatedDoubleTypeForPredictions(left, right));
>             }

Flags:

mhahnenberg: review+
gtk-ews: commit-queue-

Actions: View | Formatted Diff | Diff

Attachments on bug 121132: 211263