Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (106878 => 106879)
--- trunk/Source/_javascript_Core/ChangeLog 2012-02-07 00:55:15 UTC (rev 106878)
+++ trunk/Source/_javascript_Core/ChangeLog 2012-02-07 01:07:32 UTC (rev 106879)
@@ -1,3 +1,43 @@
+2012-02-06 Filip Pizlo <[email protected]>
+
+ ValueToNumber and ValueToDouble nodes don't do anything and should be removed
+ https://bugs.webkit.org/show_bug.cgi?id=77855
+ <rdar://problem/10811325>
+
+ Reviewed by Gavin Barraclough.
+
+ Removed ValueToNumber and ValueToDouble, because the only thing they were doing
+ was wasting registers.
+
+ This looks like a 1% win on V8 (with a 5% win on crypto) and a 2-3% win on Kraken,
+ mostly due to a >10% win on gaussian-blur. No win anywhere else.
+
+ * dfg/DFGAbstractState.cpp:
+ (JSC::DFG::AbstractState::execute):
+ * dfg/DFGByteCodeParser.cpp:
+ (JSC::DFG::ByteCodeParser::getToInt32):
+ (ByteCodeParser):
+ (JSC::DFG::ByteCodeParser::handleMinMax):
+ (JSC::DFG::ByteCodeParser::handleIntrinsic):
+ (JSC::DFG::ByteCodeParser::parseBlock):
+ * dfg/DFGNode.h:
+ (DFG):
+ (JSC::DFG::Node::hasArithNodeFlags):
+ * dfg/DFGPropagator.cpp:
+ (JSC::DFG::Propagator::propagateArithNodeFlags):
+ (JSC::DFG::Propagator::propagateNodePredictions):
+ (JSC::DFG::Propagator::vote):
+ (JSC::DFG::Propagator::doRoundOfDoubleVoting):
+ (Propagator):
+ (JSC::DFG::Propagator::fixupNode):
+ (JSC::DFG::Propagator::canonicalize):
+ * dfg/DFGSpeculativeJIT.cpp:
+ (JSC::DFG::SpeculativeJIT::computeValueRecoveryFor):
+ * dfg/DFGSpeculativeJIT32_64.cpp:
+ (JSC::DFG::SpeculativeJIT::compile):
+ * dfg/DFGSpeculativeJIT64.cpp:
+ (JSC::DFG::SpeculativeJIT::compile):
+
2012-02-06 Patrick Gansterer <[email protected]>
Unreviewed WinCE build fix after r106197.
Modified: trunk/Source/_javascript_Core/dfg/DFGAbstractState.cpp (106878 => 106879)
--- trunk/Source/_javascript_Core/dfg/DFGAbstractState.cpp 2012-02-07 00:55:15 UTC (rev 106878)
+++ trunk/Source/_javascript_Core/dfg/DFGAbstractState.cpp 2012-02-07 01:07:32 UTC (rev 106879)
@@ -262,22 +262,6 @@
forNode(nodeIndex).set(PredictInt32);
break;
- case ValueToNumber:
- if (m_graph[node.child1()].shouldNotSpeculateInteger()) {
- forNode(node.child1()).filter(PredictNumber);
- forNode(nodeIndex).set(PredictDouble);
- break;
- }
-
- forNode(node.child1()).filter(PredictInt32);
- forNode(nodeIndex).set(PredictInt32);
- break;
-
- case ValueToDouble:
- forNode(node.child1()).filter(PredictNumber);
- forNode(nodeIndex).set(PredictDouble);
- break;
-
case ValueAdd:
case ArithAdd: {
if (m_graph.addShouldSpeculateInteger(node, m_codeBlock)) {
Modified: trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp (106878 => 106879)
--- trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp 2012-02-07 00:55:15 UTC (rev 106878)
+++ trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp 2012-02-07 01:07:32 UTC (rev 106879)
@@ -313,10 +313,6 @@
{
return toInt32(get(operand));
}
- NodeIndex getToNumber(int operand)
- {
- return toNumber(get(operand));
- }
// Perform an ES5 ToInt32 operation - returns a node of type NodeResultInt32.
NodeIndex toInt32(NodeIndex index)
@@ -341,23 +337,6 @@
return addToGraph(ValueToInt32, index);
}
- // Perform an ES5 ToNumber operation - returns a node of type NodeResultDouble.
- NodeIndex toNumber(NodeIndex index)
- {
- Node& node = m_graph[index];
-
- if (node.hasNumberResult())
- return index;
-
- if (node.op == JSConstant) {
- JSValue v = valueOfJSConstant(index);
- if (v.isNumber())
- return getJSConstant(node.constantNumber());
- }
-
- return addToGraph(ValueToNumber, OpInfo(NodeUseBottom), index);
- }
-
NodeIndex getJSConstantForValue(JSValue constantValue)
{
unsigned constantIndex = m_codeBlock->addOrFindConstant(constantValue);
@@ -1198,12 +1177,12 @@
}
if (argumentCountIncludingThis == 2) { // Math.min(x)
- set(resultOperand, getToNumber(registerOffset + argumentToOperand(1)));
+ set(resultOperand, get(registerOffset + argumentToOperand(1)));
return true;
}
if (argumentCountIncludingThis == 3) { // Math.min(x, y)
- set(resultOperand, addToGraph(op, OpInfo(NodeUseBottom), getToNumber(registerOffset + argumentToOperand(1)), getToNumber(registerOffset + argumentToOperand(2))));
+ set(resultOperand, addToGraph(op, OpInfo(NodeUseBottom), get(registerOffset + argumentToOperand(1)), get(registerOffset + argumentToOperand(2))));
return true;
}
@@ -1231,7 +1210,7 @@
if (!MacroAssembler::supportsFloatingPointAbs())
return false;
- NodeIndex nodeIndex = addToGraph(ArithAbs, OpInfo(NodeUseBottom), getToNumber(registerOffset + argumentToOperand(1)));
+ NodeIndex nodeIndex = addToGraph(ArithAbs, OpInfo(NodeUseBottom), get(registerOffset + argumentToOperand(1)));
if (m_inlineStackTop->m_exitProfile.hasExitSite(m_currentIndex, Overflow))
m_graph[nodeIndex].mergeArithNodeFlags(NodeMayOverflow);
set(resultOperand, nodeIndex);
@@ -1256,7 +1235,7 @@
if (!MacroAssembler::supportsFloatingPointSqrt())
return false;
- set(resultOperand, addToGraph(ArithSqrt, getToNumber(registerOffset + argumentToOperand(1))));
+ set(resultOperand, addToGraph(ArithSqrt, get(registerOffset + argumentToOperand(1))));
return true;
}
@@ -1512,7 +1491,7 @@
case op_pre_inc: {
unsigned srcDst = currentInstruction[1].u.operand;
- NodeIndex op = getToNumber(srcDst);
+ NodeIndex op = get(srcDst);
set(srcDst, makeSafe(addToGraph(ArithAdd, OpInfo(NodeUseBottom), op, one())));
NEXT_OPCODE(op_pre_inc);
}
@@ -1521,7 +1500,7 @@
unsigned result = currentInstruction[1].u.operand;
unsigned srcDst = currentInstruction[2].u.operand;
ASSERT(result != srcDst); // Required for assumptions we make during OSR.
- NodeIndex op = getToNumber(srcDst);
+ NodeIndex op = get(srcDst);
set(result, op);
set(srcDst, makeSafe(addToGraph(ArithAdd, OpInfo(NodeUseBottom), op, one())));
NEXT_OPCODE(op_post_inc);
@@ -1529,7 +1508,7 @@
case op_pre_dec: {
unsigned srcDst = currentInstruction[1].u.operand;
- NodeIndex op = getToNumber(srcDst);
+ NodeIndex op = get(srcDst);
set(srcDst, makeSafe(addToGraph(ArithSub, OpInfo(NodeUseBottom), op, one())));
NEXT_OPCODE(op_pre_dec);
}
@@ -1537,7 +1516,7 @@
case op_post_dec: {
unsigned result = currentInstruction[1].u.operand;
unsigned srcDst = currentInstruction[2].u.operand;
- NodeIndex op = getToNumber(srcDst);
+ NodeIndex op = get(srcDst);
set(result, op);
set(srcDst, makeSafe(addToGraph(ArithSub, OpInfo(NodeUseBottom), op, one())));
NEXT_OPCODE(op_post_dec);
@@ -1549,37 +1528,37 @@
NodeIndex op1 = get(currentInstruction[2].u.operand);
NodeIndex op2 = get(currentInstruction[3].u.operand);
if (m_graph[op1].hasNumberResult() && m_graph[op2].hasNumberResult())
- set(currentInstruction[1].u.operand, makeSafe(addToGraph(ArithAdd, OpInfo(NodeUseBottom), toNumber(op1), toNumber(op2))));
+ set(currentInstruction[1].u.operand, makeSafe(addToGraph(ArithAdd, OpInfo(NodeUseBottom), op1, op2)));
else
set(currentInstruction[1].u.operand, makeSafe(addToGraph(ValueAdd, OpInfo(NodeUseBottom), op1, op2)));
NEXT_OPCODE(op_add);
}
case op_sub: {
- NodeIndex op1 = getToNumber(currentInstruction[2].u.operand);
- NodeIndex op2 = getToNumber(currentInstruction[3].u.operand);
+ NodeIndex op1 = get(currentInstruction[2].u.operand);
+ NodeIndex op2 = get(currentInstruction[3].u.operand);
set(currentInstruction[1].u.operand, makeSafe(addToGraph(ArithSub, OpInfo(NodeUseBottom), op1, op2)));
NEXT_OPCODE(op_sub);
}
case op_mul: {
// Multiply requires that the inputs are not truncated, unfortunately.
- NodeIndex op1 = getToNumber(currentInstruction[2].u.operand);
- NodeIndex op2 = getToNumber(currentInstruction[3].u.operand);
+ NodeIndex op1 = get(currentInstruction[2].u.operand);
+ NodeIndex op2 = get(currentInstruction[3].u.operand);
set(currentInstruction[1].u.operand, makeSafe(addToGraph(ArithMul, OpInfo(NodeUseBottom), op1, op2)));
NEXT_OPCODE(op_mul);
}
case op_mod: {
- NodeIndex op1 = getToNumber(currentInstruction[2].u.operand);
- NodeIndex op2 = getToNumber(currentInstruction[3].u.operand);
+ NodeIndex op1 = get(currentInstruction[2].u.operand);
+ NodeIndex op2 = get(currentInstruction[3].u.operand);
set(currentInstruction[1].u.operand, makeSafe(addToGraph(ArithMod, OpInfo(NodeUseBottom), op1, op2)));
NEXT_OPCODE(op_mod);
}
case op_div: {
- NodeIndex op1 = getToNumber(currentInstruction[2].u.operand);
- NodeIndex op2 = getToNumber(currentInstruction[3].u.operand);
+ NodeIndex op1 = get(currentInstruction[2].u.operand);
+ NodeIndex op2 = get(currentInstruction[3].u.operand);
set(currentInstruction[1].u.operand, makeDivSafe(addToGraph(ArithDiv, OpInfo(NodeUseBottom), op1, op2)));
NEXT_OPCODE(op_div);
}
Modified: trunk/Source/_javascript_Core/dfg/DFGNode.h (106878 => 106879)
--- trunk/Source/_javascript_Core/dfg/DFGNode.h 2012-02-07 00:55:15 UTC (rev 106878)
+++ trunk/Source/_javascript_Core/dfg/DFGNode.h 2012-02-07 01:07:32 UTC (rev 106879)
@@ -211,12 +211,7 @@
macro(ArithMin, NodeResultNumber) \
macro(ArithMax, NodeResultNumber) \
macro(ArithSqrt, NodeResultNumber) \
- /* Arithmetic operators call ToNumber on their operands. */\
- macro(ValueToNumber, NodeResultNumber | NodeMustGenerate) \
\
- /* A variant of ValueToNumber, which a hint that the parents will always use this as a double. */\
- macro(ValueToDouble, NodeResultNumber | NodeMustGenerate) \
- \
/* Add of values may either be arithmetic, or result in string concatenation. */\
macro(ValueAdd, NodeResultJS | NodeMustGenerate | NodeMightClobber) \
\
@@ -522,8 +517,6 @@
bool hasArithNodeFlags()
{
switch (op) {
- case ValueToNumber:
- case ValueToDouble:
case UInt32ToNumber:
case ArithAdd:
case ArithSub:
Modified: trunk/Source/_javascript_Core/dfg/DFGPropagator.cpp (106878 => 106879)
--- trunk/Source/_javascript_Core/dfg/DFGPropagator.cpp 2012-02-07 00:55:15 UTC (rev 106878)
+++ trunk/Source/_javascript_Core/dfg/DFGPropagator.cpp 2012-02-07 01:07:32 UTC (rev 106879)
@@ -138,8 +138,6 @@
break;
}
- case ValueToNumber:
- case ValueToDouble:
case UInt32ToNumber: {
changed |= m_graph[node.child1()].mergeArithNodeFlags(flags);
break;
@@ -363,19 +361,6 @@
break;
}
- case ValueToNumber: {
- PredictedType prediction = m_graph[node.child1()].prediction();
-
- if (prediction) {
- if (!(prediction & PredictDouble) && nodeCanSpeculateInteger(node.arithNodeFlags()))
- changed |= mergePrediction(PredictInt32);
- else
- changed |= mergePrediction(PredictNumber);
- }
-
- break;
- }
-
case ValueAdd: {
PredictedType left = m_graph[node.child1()].prediction();
PredictedType right = m_graph[node.child2()].prediction();
@@ -597,7 +582,6 @@
break;
}
- case ValueToDouble:
case GetArrayLength:
case GetByteArrayLength:
case GetInt8ArrayLength:
@@ -679,8 +663,6 @@
void vote(NodeUse nodeUse, VariableAccessData::Ballot ballot)
{
switch (m_graph[nodeUse].op) {
- case ValueToNumber:
- case ValueToDouble:
case ValueToInt32:
case UInt32ToNumber:
nodeUse = m_graph[nodeUse].child1();
@@ -775,11 +757,6 @@
vote(node.child1(), VariableAccessData::VoteDouble);
break;
- case ValueToNumber:
- case ValueToDouble:
- // Don't vote.
- break;
-
case SetLocal: {
PredictedType prediction = m_graph[node.child1()].prediction();
if (isDoublePrediction(prediction))
@@ -835,16 +812,6 @@
} while (m_changed);
}
- void toDouble(NodeUse nodeUse)
- {
- if (m_graph[nodeUse].op == ValueToNumber) {
-#if DFG_ENABLE(DEBUG_PROPAGATION_VERBOSE)
- printf(" @%u -> ValueToDouble", nodeUse.index());
-#endif
- m_graph[nodeUse].op = ValueToDouble;
- }
- }
-
void fixupNode(Node& node)
{
if (!node.shouldGenerate())
@@ -857,60 +824,6 @@
#endif
switch (op) {
- case ValueAdd:
- case ArithAdd:
- case ArithSub: {
- PredictedType left = m_graph[node.child1()].prediction();
- PredictedType right = m_graph[node.child2()].prediction();
-
- if (left && right
- && isNumberPrediction(left) && isNumberPrediction(right)
- && !m_graph.addShouldSpeculateInteger(node, m_codeBlock)) {
- toDouble(node.child1());
- toDouble(node.child2());
- }
- break;
- }
-
- case ArithMul:
- case ArithMin:
- case ArithMax:
- case ArithMod:
- case ArithDiv: {
- if (!nodeCanSpeculateInteger(node.arithNodeFlags())) {
- toDouble(node.child1());
- toDouble(node.child2());
- break;
- }
-
- PredictedType left = m_graph[node.child1()].prediction();
- PredictedType right = m_graph[node.child2()].prediction();
-
- if (left && right
- && ((left & PredictDouble) || (right & PredictDouble))) {
- toDouble(node.child1());
- toDouble(node.child2());
- }
- break;
- }
-
- case ArithAbs: {
- if (!nodeCanSpeculateInteger(node.arithNodeFlags())) {
- toDouble(node.child1());
- break;
- }
-
- PredictedType prediction = m_graph[node.child1()].prediction();
- if (prediction & PredictDouble)
- toDouble(node.child1());
- break;
- }
-
- case ArithSqrt: {
- toDouble(node.child1());
- break;
- }
-
case GetById: {
if (!isInt32Prediction(m_graph[m_compileIndex].prediction()))
break;
@@ -1004,9 +917,6 @@
if (nodeIndex == NoNode)
return NoNode;
- if (m_graph[nodeIndex].op == ValueToNumber)
- nodeIndex = m_graph[nodeIndex].child1().index();
-
if (m_graph[nodeIndex].op == ValueToInt32)
nodeIndex = m_graph[nodeIndex].child1().index();
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp (106878 => 106879)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2012-02-07 00:55:15 UTC (rev 106878)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2012-02-07 01:07:32 UTC (rev 106879)
@@ -1268,16 +1268,9 @@
// Try to see if there is an alternate node that would contain the value we want.
// There are four possibilities:
//
- // ValueToNumber: If the only live version of the value is a ValueToNumber node
- // then it means that all remaining uses of the value would have performed a
- // ValueToNumber conversion anyway. Thus, we can substitute ValueToNumber.
+ // ValueToInt32: If the only remaining live version of the value is
+ // ValueToInt32, then we can use it.
//
- // ValueToInt32: Likewise, if the only remaining live version of the value is
- // ValueToInt32, then we can use it. But if there is both a ValueToInt32
- // and a ValueToNumber, then we better go with ValueToNumber because it
- // means that some remaining uses would have converted to number while
- // others would have converted to Int32.
- //
// UInt32ToNumber: If the only live version of the value is a UInt32ToNumber
// then the only remaining uses are ones that want a properly formed number
// rather than a UInt32 intermediate.
@@ -1297,7 +1290,6 @@
}
if (!found) {
- NodeIndex valueToNumberIndex = NoNode;
NodeIndex valueToInt32Index = NoNode;
NodeIndex uint32ToNumberIndex = NoNode;
@@ -1311,10 +1303,6 @@
if (node.child1Unchecked() != valueSource.nodeIndex())
continue;
switch (node.op) {
- case ValueToNumber:
- case ValueToDouble:
- valueToNumberIndex = info.nodeIndex();
- break;
case ValueToInt32:
valueToInt32Index = info.nodeIndex();
break;
@@ -1327,9 +1315,7 @@
}
NodeIndex nodeIndexToUse;
- if (valueToNumberIndex != NoNode)
- nodeIndexToUse = valueToNumberIndex;
- else if (valueToInt32Index != NoNode)
+ if (valueToInt32Index != NoNode)
nodeIndexToUse = valueToInt32Index;
else if (uint32ToNumberIndex != NoNode)
nodeIndexToUse = uint32ToNumberIndex;
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp (106878 => 106879)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp 2012-02-07 00:55:15 UTC (rev 106878)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp 2012-02-07 01:07:32 UTC (rev 106879)
@@ -1861,30 +1861,6 @@
break;
}
- case ValueToNumber: {
- if (at(node.child1()).shouldNotSpeculateInteger()) {
- SpeculateDoubleOperand op1(this, node.child1());
- FPRTemporary result(this, op1);
- m_jit.moveDouble(op1.fpr(), result.fpr());
- doubleResult(result.fpr(), m_compileIndex);
- break;
- }
-
- SpeculateIntegerOperand op1(this, node.child1());
- GPRTemporary result(this, op1);
- m_jit.move(op1.gpr(), result.gpr());
- integerResult(result.gpr(), m_compileIndex, op1.format());
- break;
- }
-
- case ValueToDouble: {
- SpeculateDoubleOperand op1(this, node.child1());
- FPRTemporary result(this, op1);
- m_jit.moveDouble(op1.fpr(), result.fpr());
- doubleResult(result.fpr(), m_compileIndex);
- break;
- }
-
case ValueAdd:
case ArithAdd:
compileAdd(node);
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp (106878 => 106879)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp 2012-02-07 00:55:15 UTC (rev 106878)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp 2012-02-07 01:07:32 UTC (rev 106879)
@@ -1940,30 +1940,6 @@
break;
}
- case ValueToNumber: {
- if (at(node.child1()).shouldNotSpeculateInteger()) {
- SpeculateDoubleOperand op1(this, node.child1());
- FPRTemporary result(this, op1);
- m_jit.moveDouble(op1.fpr(), result.fpr());
- doubleResult(result.fpr(), m_compileIndex);
- break;
- }
-
- SpeculateIntegerOperand op1(this, node.child1());
- GPRTemporary result(this, op1);
- m_jit.move(op1.gpr(), result.gpr());
- integerResult(result.gpr(), m_compileIndex, op1.format());
- break;
- }
-
- case ValueToDouble: {
- SpeculateDoubleOperand op1(this, node.child1());
- FPRTemporary result(this, op1);
- m_jit.moveDouble(op1.fpr(), result.fpr());
- doubleResult(result.fpr(), m_compileIndex);
- break;
- }
-
case ValueAdd:
case ArithAdd:
compileAdd(node);
