Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (144130 => 144131)
--- trunk/Source/_javascript_Core/ChangeLog 2013-02-27 01:20:27 UTC (rev 144130)
+++ trunk/Source/_javascript_Core/ChangeLog 2013-02-27 01:45:28 UTC (rev 144131)
@@ -1,3 +1,52 @@
+2013-02-25 Filip Pizlo <fpi...@apple.com>
+
+ The DFG backend's and OSR's decision to unbox a variable should be based on whether it's used in a typed context
+ https://bugs.webkit.org/show_bug.cgi?id=110433
+
+ Reviewed by Oliver Hunt and Mark Hahnenberg.
+
+ This introduces the equivalent of a liveness analysis, except for type checking.
+ A variable is said to be "profitable for unboxing" (i.e. live at a type check)
+ if there exists a type check on a GetLocal of that variable, and the type check
+ is consistent with the variable's prediction. Variables that are not profitable
+ for unboxing aren't unboxed. Previously they would have been.
+
+ This is a slight speed-up on some things but mostly neutral.
+
+ * dfg/DFGArgumentPosition.h:
+ (JSC::DFG::ArgumentPosition::ArgumentPosition):
+ (JSC::DFG::ArgumentPosition::mergeShouldNeverUnbox):
+ (JSC::DFG::ArgumentPosition::mergeArgumentPredictionAwareness):
+ (JSC::DFG::ArgumentPosition::mergeArgumentUnboxingAwareness):
+ (ArgumentPosition):
+ (JSC::DFG::ArgumentPosition::isProfitableToUnbox):
+ (JSC::DFG::ArgumentPosition::shouldUseDoubleFormat):
+ * dfg/DFGCommon.h:
+ (JSC::DFG::checkAndSet):
+ (DFG):
+ * dfg/DFGFixupPhase.cpp:
+ (JSC::DFG::FixupPhase::run):
+ (JSC::DFG::FixupPhase::fixupNode):
+ (JSC::DFG::FixupPhase::fixupSetLocalsInBlock):
+ (FixupPhase):
+ (JSC::DFG::FixupPhase::alwaysUnboxSimplePrimitives):
+ (JSC::DFG::FixupPhase::setUseKindAndUnboxIfProfitable):
+ * dfg/DFGPredictionPropagationPhase.cpp:
+ (JSC::DFG::PredictionPropagationPhase::doRoundOfDoubleVoting):
+ * dfg/DFGSpeculativeJIT.cpp:
+ (JSC::DFG::SpeculativeJIT::checkArgumentTypes):
+ * dfg/DFGVariableAccessData.h:
+ (JSC::DFG::VariableAccessData::VariableAccessData):
+ (JSC::DFG::VariableAccessData::mergeIsCaptured):
+ (JSC::DFG::VariableAccessData::mergeIsProfitableToUnbox):
+ (VariableAccessData):
+ (JSC::DFG::VariableAccessData::isProfitableToUnbox):
+ (JSC::DFG::VariableAccessData::shouldUnboxIfPossible):
+ (JSC::DFG::VariableAccessData::mergeStructureCheckHoistingFailed):
+ (JSC::DFG::VariableAccessData::mergeIsArgumentsAlias):
+ (JSC::DFG::VariableAccessData::shouldUseDoubleFormat):
+ (JSC::DFG::VariableAccessData::mergeFlags):
+
2013-02-26 Oliver Hunt <oli...@apple.com>
Fix windows build.
Modified: trunk/Source/_javascript_Core/dfg/DFGArgumentPosition.h (144130 => 144131)
--- trunk/Source/_javascript_Core/dfg/DFGArgumentPosition.h 2013-02-27 01:20:27 UTC (rev 144130)
+++ trunk/Source/_javascript_Core/dfg/DFGArgumentPosition.h 2013-02-27 01:45:28 UTC (rev 144131)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2012 Apple Inc. All rights reserved.
+ * Copyright (C) 2012, 2013 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -37,6 +37,7 @@
ArgumentPosition()
: m_prediction(SpecNone)
, m_doubleFormatState(EmptyDoubleFormatState)
+ , m_isProfitableToUnbox(false)
, m_shouldNeverUnbox(false)
{
}
@@ -48,14 +49,10 @@
bool mergeShouldNeverUnbox(bool shouldNeverUnbox)
{
- bool newShouldNeverUnbox = m_shouldNeverUnbox | shouldNeverUnbox;
- if (newShouldNeverUnbox == m_shouldNeverUnbox)
- return false;
- m_shouldNeverUnbox = newShouldNeverUnbox;
- return true;
+ return checkAndSet(m_shouldNeverUnbox, m_shouldNeverUnbox | shouldNeverUnbox);
}
- bool mergeArgumentAwareness()
+ bool mergeArgumentPredictionAwareness()
{
bool changed = false;
for (unsigned i = 0; i < m_variables.size(); ++i) {
@@ -74,16 +71,32 @@
return changed;
}
+ bool mergeArgumentUnboxingAwareness()
+ {
+ bool changed = false;
+ for (unsigned i = 0; i < m_variables.size(); ++i)
+ changed |= checkAndSet(m_isProfitableToUnbox, m_isProfitableToUnbox | m_variables[i]->isProfitableToUnbox());
+ if (!changed)
+ return false;
+ changed = false;
+ for (unsigned i = 0; i < m_variables.size(); ++i)
+ changed |= m_variables[i]->mergeIsProfitableToUnbox(m_isProfitableToUnbox);
+ return changed;
+ }
+
+ bool shouldUnboxIfPossible() const { return m_isProfitableToUnbox && !m_shouldNeverUnbox; }
+
SpeculatedType prediction() const { return m_prediction; }
DoubleFormatState doubleFormatState() const { return m_doubleFormatState; }
bool shouldUseDoubleFormat() const
{
- return doubleFormatState() == UsingDoubleFormat;
+ return doubleFormatState() == UsingDoubleFormat && shouldUnboxIfPossible();
}
private:
SpeculatedType m_prediction;
DoubleFormatState m_doubleFormatState;
+ bool m_isProfitableToUnbox;
bool m_shouldNeverUnbox;
Vector<VariableAccessData*, 2> m_variables;
Modified: trunk/Source/_javascript_Core/dfg/DFGCommon.h (144130 => 144131)
--- trunk/Source/_javascript_Core/dfg/DFGCommon.h 2013-02-27 01:20:27 UTC (rev 144130)
+++ trunk/Source/_javascript_Core/dfg/DFGCommon.h 2013-02-27 01:45:28 UTC (rev 144131)
@@ -221,6 +221,15 @@
enum SpeculationDirection { ForwardSpeculation, BackwardSpeculation };
+template<typename T, typename U>
+bool checkAndSet(T& left, U right)
+{
+ if (left == right)
+ return false;
+ left = right;
+ return true;
+}
+
} } // namespace JSC::DFG
namespace WTF {
Modified: trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp (144130 => 144131)
--- trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp 2013-02-27 01:20:27 UTC (rev 144130)
+++ trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp 2013-02-27 01:45:28 UTC (rev 144131)
@@ -31,6 +31,7 @@
#include "DFGGraph.h"
#include "DFGInsertionSet.h"
#include "DFGPhase.h"
+#include "DFGVariableAccessDataDump.h"
#include "Operations.h"
namespace JSC { namespace DFG {
@@ -48,8 +49,20 @@
ASSERT(m_graph.m_fixpointState == BeforeFixpoint);
ASSERT(m_graph.m_form == ThreadedCPS);
+ m_profitabilityChanged = false;
for (BlockIndex blockIndex = 0; blockIndex < m_graph.m_blocks.size(); ++blockIndex)
fixupBlock(m_graph.m_blocks[blockIndex].get());
+
+ while (m_profitabilityChanged) {
+ m_profitabilityChanged = false;
+
+ for (unsigned i = m_graph.m_argumentPositions.size(); i--;)
+ m_graph.m_argumentPositions[i].mergeArgumentUnboxingAwareness();
+
+ for (BlockIndex blockIndex = 0; blockIndex < m_graph.m_blocks.size(); ++blockIndex)
+ fixupSetLocalsInBlock(m_graph.m_blocks[blockIndex].get());
+ }
+
return true;
}
@@ -80,23 +93,7 @@
switch (op) {
case SetLocal: {
- VariableAccessData* variable = node->variableAccessData();
-
- if (!variable->shouldUnboxIfPossible())
- break;
-
- if (variable->shouldUseDoubleFormat()) {
- fixDoubleEdge<NumberUse>(node->child1(), ForwardSpeculation);
- break;
- }
-
- SpeculatedType predictedType = variable->argumentAwarePrediction();
- if (isInt32Speculation(predictedType))
- setUseKindAndUnboxIfProfitable<Int32Use>(node->child1());
- else if (isCellSpeculation(predictedType))
- setUseKindAndUnboxIfProfitable<CellUse>(node->child1());
- else if (isBooleanSpeculation(predictedType))
- setUseKindAndUnboxIfProfitable<BooleanUse>(node->child1());
+ // This gets handled by fixupSetLocalsInBlock().
break;
}
@@ -807,6 +804,40 @@
#endif
}
+ void fixupSetLocalsInBlock(BasicBlock* block)
+ {
+ if (!block)
+ return;
+ ASSERT(block->isReachable);
+ m_block = block;
+ for (m_indexInBlock = 0; m_indexInBlock < block->size(); ++m_indexInBlock) {
+ Node* node = m_currentNode = block->at(m_indexInBlock);
+ if (node->op() != SetLocal)
+ continue;
+ if (!node->shouldGenerate())
+ continue;
+
+ VariableAccessData* variable = node->variableAccessData();
+
+ if (!variable->shouldUnboxIfPossible())
+ continue;
+
+ if (variable->shouldUseDoubleFormat()) {
+ fixDoubleEdge<NumberUse>(node->child1(), ForwardSpeculation);
+ continue;
+ }
+
+ SpeculatedType predictedType = variable->argumentAwarePrediction();
+ if (isInt32Speculation(predictedType))
+ setUseKindAndUnboxIfProfitable<Int32Use>(node->child1());
+ else if (isCellSpeculation(predictedType))
+ setUseKindAndUnboxIfProfitable<CellUse>(node->child1());
+ else if (isBooleanSpeculation(predictedType))
+ setUseKindAndUnboxIfProfitable<BooleanUse>(node->child1());
+ }
+ m_insertionSet.execute(block);
+ }
+
Node* checkArray(ArrayMode arrayMode, CodeOrigin codeOrigin, Node* array, Node* index, bool (*storageCheck)(const ArrayMode&) = canCSEStorage, bool shouldGenerate = true)
{
ASSERT(arrayMode.isSpecific());
@@ -895,11 +926,52 @@
} }
}
+ bool alwaysUnboxSimplePrimitives()
+ {
+#if USE(JSVALUE64)
+ return false;
+#else
+ // Any boolean, int, or cell value is profitable to unbox on 32-bit because it
+ // reduces traffic.
+ return true;
+#endif
+ }
+
// Set the use kind of the edge. In the future (https://bugs.webkit.org/show_bug.cgi?id=110433),
// this can be used to notify the GetLocal that the variable is profitable to unbox.
template<UseKind useKind>
void setUseKindAndUnboxIfProfitable(Edge& edge)
{
+ if (edge->op() == GetLocal) {
+ VariableAccessData* variable = edge->variableAccessData();
+ switch (useKind) {
+ case Int32Use:
+ if (alwaysUnboxSimplePrimitives()
+ || isInt32Speculation(variable->prediction()))
+ m_profitabilityChanged |= variable->mergeIsProfitableToUnbox(true);
+ break;
+ case NumberUse:
+ case RealNumberUse:
+ if (variable->doubleFormatState() == UsingDoubleFormat)
+ m_profitabilityChanged |= variable->mergeIsProfitableToUnbox(true);
+ break;
+ case BooleanUse:
+ if (alwaysUnboxSimplePrimitives()
+ || isBooleanSpeculation(variable->prediction()))
+ m_profitabilityChanged |= variable->mergeIsProfitableToUnbox(true);
+ break;
+ case CellUse:
+ case ObjectUse:
+ case StringUse:
+ if (alwaysUnboxSimplePrimitives()
+ || isCellSpeculation(variable->prediction()))
+ m_profitabilityChanged |= variable->mergeIsProfitableToUnbox(true);
+ break;
+ default:
+ break;
+ }
+ }
+
edge.setUseKind(useKind);
}
@@ -1001,6 +1073,7 @@
unsigned m_indexInBlock;
Node* m_currentNode;
InsertionSet m_insertionSet;
+ bool m_profitabilityChanged;
};
bool performFixup(Graph& graph)
Modified: trunk/Source/_javascript_Core/dfg/DFGPredictionPropagationPhase.cpp (144130 => 144131)
--- trunk/Source/_javascript_Core/dfg/DFGPredictionPropagationPhase.cpp 2013-02-27 01:20:27 UTC (rev 144130)
+++ trunk/Source/_javascript_Core/dfg/DFGPredictionPropagationPhase.cpp 2013-02-27 01:45:28 UTC (rev 144131)
@@ -1025,7 +1025,7 @@
m_changed |= variableAccessData->tallyVotesForShouldUseDoubleFormat();
}
for (unsigned i = 0; i < m_graph.m_argumentPositions.size(); ++i)
- m_changed |= m_graph.m_argumentPositions[i].mergeArgumentAwareness();
+ m_changed |= m_graph.m_argumentPositions[i].mergeArgumentPredictionAwareness();
for (unsigned i = 0; i < m_graph.m_variableAccessData.size(); ++i) {
VariableAccessData* variableAccessData = &m_graph.m_variableAccessData[i];
if (!variableAccessData->isRoot())
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp (144130 => 144131)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2013-02-27 01:20:27 UTC (rev 144130)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2013-02-27 01:45:28 UTC (rev 144131)
@@ -1646,10 +1646,10 @@
valueSource = ValueSource(SourceIsDead);
else if (node->variableAccessData()->isArgumentsAlias())
valueSource = ValueSource(ArgumentsSource);
+ else if (!node->refCount())
+ valueSource = ValueSource(SourceIsDead);
else if (!node->variableAccessData()->shouldUnboxIfPossible())
valueSource = ValueSource(ValueInJSStack);
- else if (!node->refCount())
- valueSource = ValueSource(SourceIsDead);
else if (node->variableAccessData()->shouldUseDoubleFormat())
valueSource = ValueSource(DoubleInJSStack);
else
@@ -1712,7 +1712,9 @@
ArgumentPosition& argumentPosition =
m_jit.graph().m_argumentPositions[argumentPositionStart + i];
ValueSource valueSource;
- if (argumentPosition.shouldUseDoubleFormat())
+ if (!argumentPosition.shouldUnboxIfPossible())
+ valueSource = ValueSource(ValueInJSStack);
+ else if (argumentPosition.shouldUseDoubleFormat())
valueSource = ValueSource(DoubleInJSStack);
else if (isInt32Speculation(argumentPosition.prediction()))
valueSource = ValueSource(Int32InJSStack);
@@ -1839,6 +1841,9 @@
}
VariableAccessData* variableAccessData = node->variableAccessData();
+ if (!variableAccessData->isProfitableToUnbox())
+ continue;
+
VirtualRegister virtualRegister = variableAccessData->local();
SpeculatedType predictedType = variableAccessData->prediction();
Modified: trunk/Source/_javascript_Core/dfg/DFGVariableAccessData.h (144130 => 144131)
--- trunk/Source/_javascript_Core/dfg/DFGVariableAccessData.h 2013-02-27 01:20:27 UTC (rev 144130)
+++ trunk/Source/_javascript_Core/dfg/DFGVariableAccessData.h 2013-02-27 01:45:28 UTC (rev 144131)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2011, 2012 Apple Inc. All rights reserved.
+ * Copyright (C) 2011, 2012, 2013 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -50,6 +50,7 @@
, m_shouldNeverUnbox(false)
, m_isArgumentsAlias(false)
, m_structureCheckHoistingFailed(false)
+ , m_isProfitableToUnbox(false)
, m_doubleFormatState(EmptyDoubleFormatState)
{
clearVotes();
@@ -64,6 +65,7 @@
, m_shouldNeverUnbox(isCaptured)
, m_isArgumentsAlias(false)
, m_structureCheckHoistingFailed(false)
+ , m_isProfitableToUnbox(false)
, m_doubleFormatState(EmptyDoubleFormatState)
{
clearVotes();
@@ -82,12 +84,8 @@
bool mergeIsCaptured(bool isCaptured)
{
- m_shouldNeverUnbox |= isCaptured;
- bool newIsCaptured = m_isCaptured | isCaptured;
- if (newIsCaptured == m_isCaptured)
- return false;
- m_isCaptured = newIsCaptured;
- return true;
+ return checkAndSet(m_shouldNeverUnbox, m_shouldNeverUnbox | isCaptured)
+ | checkAndSet(m_isCaptured, m_isCaptured | isCaptured);
}
bool isCaptured()
@@ -95,6 +93,16 @@
return m_isCaptured;
}
+ bool mergeIsProfitableToUnbox(bool isProfitableToUnbox)
+ {
+ return checkAndSet(m_isProfitableToUnbox, m_isProfitableToUnbox | isProfitableToUnbox);
+ }
+
+ bool isProfitableToUnbox()
+ {
+ return m_isProfitableToUnbox;
+ }
+
bool mergeShouldNeverUnbox(bool shouldNeverUnbox)
{
bool newShouldNeverUnbox = m_shouldNeverUnbox | shouldNeverUnbox;
@@ -118,16 +126,12 @@
// returns false, since this incorporates heuristics of profitability.
bool shouldUnboxIfPossible()
{
- return !shouldNeverUnbox();
+ return !shouldNeverUnbox() && isProfitableToUnbox();
}
-
+
bool mergeStructureCheckHoistingFailed(bool failed)
{
- bool newFailed = m_structureCheckHoistingFailed | failed;
- if (newFailed == m_structureCheckHoistingFailed)
- return false;
- m_structureCheckHoistingFailed = newFailed;
- return true;
+ return checkAndSet(m_structureCheckHoistingFailed, m_structureCheckHoistingFailed | failed);
}
bool structureCheckHoistingFailed()
@@ -137,11 +141,7 @@
bool mergeIsArgumentsAlias(bool isArgumentsAlias)
{
- bool newIsArgumentsAlias = m_isArgumentsAlias | isArgumentsAlias;
- if (newIsArgumentsAlias == m_isArgumentsAlias)
- return false;
- m_isArgumentsAlias = newIsArgumentsAlias;
- return true;
+ return checkAndSet(m_isArgumentsAlias, m_isArgumentsAlias | isArgumentsAlias);
}
bool isArgumentsAlias()
@@ -239,10 +239,10 @@
bool shouldUseDoubleFormat()
{
ASSERT(isRoot());
- bool result = m_doubleFormatState == UsingDoubleFormat;
- ASSERT(!(result && shouldNeverUnbox()));
- ASSERT(!(result && isCaptured()));
- return result;
+ bool doubleState = m_doubleFormatState == UsingDoubleFormat;
+ ASSERT(!(doubleState && shouldNeverUnbox()));
+ ASSERT(!(doubleState && isCaptured()));
+ return doubleState && isProfitableToUnbox();
}
bool tallyVotesForShouldUseDoubleFormat()
@@ -287,11 +287,7 @@
bool mergeFlags(NodeFlags newFlags)
{
- newFlags |= m_flags;
- if (newFlags == m_flags)
- return false;
- m_flags = newFlags;
- return true;
+ return checkAndSet(m_flags, m_flags | newFlags);
}
private:
@@ -304,11 +300,12 @@
SpeculatedType m_prediction;
SpeculatedType m_argumentAwarePrediction;
NodeFlags m_flags;
-
+
bool m_isCaptured;
bool m_shouldNeverUnbox;
bool m_isArgumentsAlias;
bool m_structureCheckHoistingFailed;
+ bool m_isProfitableToUnbox;
float m_votes[2]; // Used primarily for double voting but may be reused for other purposes.
DoubleFormatState m_doubleFormatState;
