Truncating multiplication on integers should not OSR exit every time
https://bugs.webkit.org/show_bug.cgi?id=85752
Reviewed by Gavin Barraclough.
Merge r116264 from dfgopt.
* 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):
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@117993 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/JavaScriptCore/dfg/DFGAbstractState.cpp b/Source/JavaScriptCore/dfg/DFGAbstractState.cpp
index 1e2ed5a..cf2f71b 100644
--- a/Source/JavaScriptCore/dfg/DFGAbstractState.cpp
+++ b/Source/JavaScriptCore/dfg/DFGAbstractState.cpp
@@ -468,7 +468,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:
@@ -479,9 +498,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;
diff --git a/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp b/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
index d1b2767..84f29e90 100644
--- a/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
+++ b/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
@@ -250,7 +250,6 @@
case ArithMin:
case ArithMax:
- case ArithMul:
case ArithMod: {
if (Node::shouldSpeculateInteger(m_graph[node.child1()], m_graph[node.child2()])
&& node.canSpeculateInteger())
@@ -260,6 +259,14 @@
break;
}
+ case ArithMul: {
+ if (m_graph.mulShouldSpeculateInteger(node))
+ break;
+ fixDoubleEdge(0);
+ fixDoubleEdge(1);
+ break;
+ }
+
case ArithDiv: {
if (Node::shouldSpeculateInteger(m_graph[node.child1()], m_graph[node.child2()])
&& node.canSpeculateInteger()) {
diff --git a/Source/JavaScriptCore/dfg/DFGGraph.h b/Source/JavaScriptCore/dfg/DFGGraph.h
index c40ce49..c041f84 100644
--- a/Source/JavaScriptCore/dfg/DFGGraph.h
+++ b/Source/JavaScriptCore/dfg/DFGGraph.h
@@ -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);
diff --git a/Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp b/Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp
index f0132f3..b98b36c 100644
--- a/Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp
+++ b/Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp
@@ -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();
@@ -753,7 +774,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:
diff --git a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
index 25a072f..b14e2d4 100644
--- a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
+++ b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
@@ -2589,7 +2589,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);
@@ -2597,15 +2597,17 @@
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.
-
- speculationCheck(Overflow, JSValueRegs(), NoNode, m_jit.branchMul32(MacroAssembler::Overflow, reg1, reg2, result.gpr()));
+ // 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()));
+ }
// Check for negative zero, if the users of this node care about such things.
if (!nodeCanIgnoreNegativeZero(node.arithNodeFlags())) {
