Diff
Modified: branches/dfgopt/Source/_javascript_Core/ChangeLog (116263 => 116264)
--- branches/dfgopt/Source/_javascript_Core/ChangeLog 2012-05-07 03:13:12 UTC (rev 116263)
+++ branches/dfgopt/Source/_javascript_Core/ChangeLog 2012-05-07 03:43:03 UTC (rev 116264)
@@ -1,3 +1,24 @@
+2012-05-06 Filip Pizlo <[email protected]>
+
+ Truncating multiplication on integers should not OSR exit every time
+ https://bugs.webkit.org/show_bug.cgi?id=85752
+
+ Reviewed by Gavin Barraclough.
+
+ * dfg/DFGAbstractState.cpp:
+ (JSC::DFG::AbstractState::execute):
+ * dfg/DFGFixupPhase.cpp:
+ (JSC::DFG::FixupPhase::fixupNode):
+ * dfg/DFGGraph.h:
+ (JSC::DFG::Graph::mulShouldSpeculateInteger):
+ (Graph):
+ (JSC::DFG::Graph::mulImmediateShouldSpeculateInteger):
+ * dfg/DFGPredictionPropagationPhase.cpp:
+ (JSC::DFG::PredictionPropagationPhase::propagate):
+ (JSC::DFG::PredictionPropagationPhase::doRoundOfDoubleVoting):
+ * dfg/DFGSpeculativeJIT.cpp:
+ (JSC::DFG::SpeculativeJIT::compileArithMul):
+
2012-05-01 Filip Pizlo <[email protected]>
DFG should be able to compute dominators
Modified: branches/dfgopt/Source/_javascript_Core/dfg/DFGAbstractState.cpp (116263 => 116264)
--- branches/dfgopt/Source/_javascript_Core/dfg/DFGAbstractState.cpp 2012-05-07 03:13:12 UTC (rev 116263)
+++ branches/dfgopt/Source/_javascript_Core/dfg/DFGAbstractState.cpp 2012-05-07 03:43:03 UTC (rev 116264)
@@ -451,7 +451,26 @@
break;
}
- case ArithMul:
+ case ArithMul: {
+ JSValue left = forNode(node.child1()).value();
+ JSValue right = forNode(node.child2()).value();
+ if (left && right && left.isNumber() && right.isNumber()) {
+ forNode(nodeIndex).set(JSValue(left.asNumber() * right.asNumber()));
+ m_foundConstants = true;
+ break;
+ }
+ if (m_graph.mulShouldSpeculateInteger(node)) {
+ forNode(node.child1()).filter(PredictInt32);
+ forNode(node.child2()).filter(PredictInt32);
+ forNode(nodeIndex).set(PredictInt32);
+ break;
+ }
+ forNode(node.child1()).filter(PredictNumber);
+ forNode(node.child2()).filter(PredictNumber);
+ forNode(nodeIndex).set(PredictDouble);
+ break;
+ }
+
case ArithDiv:
case ArithMin:
case ArithMax:
@@ -462,9 +481,6 @@
double a = left.asNumber();
double b = right.asNumber();
switch (node.op()) {
- case ArithMul:
- forNode(nodeIndex).set(JSValue(a * b));
- break;
case ArithDiv:
forNode(nodeIndex).set(JSValue(a / b));
break;
Modified: branches/dfgopt/Source/_javascript_Core/dfg/DFGFixupPhase.cpp (116263 => 116264)
--- branches/dfgopt/Source/_javascript_Core/dfg/DFGFixupPhase.cpp 2012-05-07 03:13:12 UTC (rev 116263)
+++ branches/dfgopt/Source/_javascript_Core/dfg/DFGFixupPhase.cpp 2012-05-07 03:43:03 UTC (rev 116264)
@@ -251,9 +251,16 @@
break;
}
+ case ArithMul: {
+ if (m_graph.mulShouldSpeculateInteger(node))
+ break;
+ fixDoubleEdge(0);
+ fixDoubleEdge(1);
+ break;
+ }
+
case ArithMin:
case ArithMax:
- case ArithMul:
case ArithDiv:
case ArithMod: {
if (Node::shouldSpeculateInteger(m_graph[node.child1()], m_graph[node.child2()])
Modified: branches/dfgopt/Source/_javascript_Core/dfg/DFGGraph.h (116263 => 116264)
--- branches/dfgopt/Source/_javascript_Core/dfg/DFGGraph.h 2012-05-07 03:13:12 UTC (rev 116263)
+++ branches/dfgopt/Source/_javascript_Core/dfg/DFGGraph.h 2012-05-07 03:43:03 UTC (rev 116264)
@@ -198,6 +198,21 @@
return Node::shouldSpeculateInteger(left, right) && add.canSpeculateInteger();
}
+ bool mulShouldSpeculateInteger(Node& mul)
+ {
+ ASSERT(mul.op() == ArithMul);
+
+ Node& left = at(mul.child1());
+ Node& right = at(mul.child2());
+
+ if (left.hasConstant())
+ return mulImmediateShouldSpeculateInteger(mul, right, left);
+ if (right.hasConstant())
+ return mulImmediateShouldSpeculateInteger(mul, left, right);
+
+ return Node::shouldSpeculateInteger(left, right) && mul.canSpeculateInteger() && !nodeMayOverflow(mul.arithNodeFlags());
+ }
+
bool negateShouldSpeculateInteger(Node& negate)
{
ASSERT(negate.op() == ArithNegate);
@@ -482,6 +497,30 @@
return nodeCanTruncateInteger(add.arithNodeFlags());
}
+ bool mulImmediateShouldSpeculateInteger(Node& mul, Node& variable, Node& immediate)
+ {
+ ASSERT(immediate.hasConstant());
+
+ JSValue immediateValue = immediate.valueOfJSConstant(m_codeBlock);
+ if (!immediateValue.isInt32())
+ return false;
+
+ if (!variable.shouldSpeculateInteger())
+ return false;
+
+ int32_t intImmediate = immediateValue.asInt32();
+ // Doubles have a 53 bit mantissa so we expect a multiplication of 2^31 (the highest
+ // magnitude possible int32 value) and any value less than 2^22 to not result in any
+ // rounding in a double multiplication - hence it will be equivalent to an integer
+ // multiplication, if we are doing int32 truncation afterwards (which is what
+ // canSpeculateInteger() implies).
+ const int32_t twoToThe22 = 1 << 22;
+ if (intImmediate <= -twoToThe22 || intImmediate >= twoToThe22)
+ return mul.canSpeculateInteger() && !nodeMayOverflow(mul.arithNodeFlags());
+
+ return mul.canSpeculateInteger();
+ }
+
// When a node's refCount goes from 0 to 1, it must (logically) recursively ref all of its children, and vice versa.
void refChildren(NodeIndex);
void derefChildren(NodeIndex);
Modified: branches/dfgopt/Source/_javascript_Core/dfg/DFGPredictionPropagationPhase.cpp (116263 => 116264)
--- branches/dfgopt/Source/_javascript_Core/dfg/DFGPredictionPropagationPhase.cpp 2012-05-07 03:13:12 UTC (rev 116263)
+++ branches/dfgopt/Source/_javascript_Core/dfg/DFGPredictionPropagationPhase.cpp 2012-05-07 03:43:03 UTC (rev 116264)
@@ -331,8 +331,29 @@
changed |= m_graph[node.child2()].mergeFlags(flags);
break;
}
+
+ case ArithMul: {
+ PredictedType left = m_graph[node.child1()].prediction();
+ PredictedType right = m_graph[node.child2()].prediction();
- case ArithMul:
+ if (left && right) {
+ if (m_graph.mulShouldSpeculateInteger(node))
+ changed |= mergePrediction(PredictInt32);
+ else
+ changed |= mergePrediction(PredictDouble);
+ }
+
+ // As soon as a multiply happens, we can easily end up in the part
+ // of the double domain where the point at which you do truncation
+ // can change the outcome. So, ArithMul always checks for overflow
+ // no matter what, and always forces its inputs to check as well.
+
+ flags |= NodeUsedAsNumber | NodeNeedsNegZero;
+ changed |= m_graph[node.child1()].mergeFlags(flags);
+ changed |= m_graph[node.child2()].mergeFlags(flags);
+ break;
+ }
+
case ArithDiv: {
PredictedType left = m_graph[node.child1()].prediction();
PredictedType right = m_graph[node.child2()].prediction();
@@ -752,7 +773,23 @@
break;
}
- case ArithMul:
+ case ArithMul: {
+ PredictedType left = m_graph[node.child1()].prediction();
+ PredictedType right = m_graph[node.child2()].prediction();
+
+ VariableAccessData::Ballot ballot;
+
+ if (isNumberPrediction(left) && isNumberPrediction(right)
+ && !m_graph.mulShouldSpeculateInteger(node))
+ ballot = VariableAccessData::VoteDouble;
+ else
+ ballot = VariableAccessData::VoteValue;
+
+ vote(node.child1(), ballot);
+ vote(node.child2(), ballot);
+ break;
+ }
+
case ArithMin:
case ArithMax:
case ArithMod:
Modified: branches/dfgopt/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp (116263 => 116264)
--- branches/dfgopt/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2012-05-07 03:13:12 UTC (rev 116263)
+++ branches/dfgopt/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2012-05-07 03:43:03 UTC (rev 116264)
@@ -2645,7 +2645,7 @@
void SpeculativeJIT::compileArithMul(Node& node)
{
- if (Node::shouldSpeculateInteger(at(node.child1()), at(node.child2())) && node.canSpeculateInteger()) {
+ if (m_jit.graph().mulShouldSpeculateInteger(node)) {
SpeculateIntegerOperand op1(this, node.child1());
SpeculateIntegerOperand op2(this, node.child2());
GPRTemporary result(this);
@@ -2653,16 +2653,18 @@
GPRReg reg1 = op1.gpr();
GPRReg reg2 = op2.gpr();
- // What is unfortunate is that we cannot take advantage of nodeCanTruncateInteger()
- // here. A multiply on integers performed in the double domain and then truncated to
- // an integer will give a different result than a multiply performed in the integer
- // domain and then truncated, if the integer domain result would have resulted in
- // something bigger than what a 32-bit integer can hold. _javascript_ mandates that
- // the semantics are always as if the multiply had been performed in the double
- // domain.
+ // We can perform truncated multiplications if we get to this point, because if the
+ // fixup phase could not prove that it would be safe, it would have turned us into
+ // a double multiplication.
+ if (nodeCanTruncateInteger(node.arithNodeFlags())) {
+ m_jit.move(reg1, result.gpr());
+ m_jit.mul32(reg2, result.gpr());
+ } else {
+ speculationCheck(
+ Overflow, JSValueRegs(), NoNode,
+ m_jit.branchMul32(MacroAssembler::Overflow, reg1, reg2, result.gpr()));
+ }
- speculationCheck(Overflow, JSValueRegs(), NoNode, m_jit.branchMul32(MacroAssembler::Overflow, reg1, reg2, result.gpr()));
-
// Check for negative zero, if the users of this node care about such things.
if (!nodeCanIgnoreNegativeZero(node.arithNodeFlags())) {
MacroAssembler::Jump resultNonZero = m_jit.branchTest32(MacroAssembler::NonZero, result.gpr());
