Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (180159 => 180160)
--- trunk/Source/_javascript_Core/ChangeLog 2015-02-16 19:05:06 UTC (rev 180159)
+++ trunk/Source/_javascript_Core/ChangeLog 2015-02-16 19:27:37 UTC (rev 180160)
@@ -1,3 +1,80 @@
+2015-02-16 Filip Pizlo <fpi...@apple.com>
+
+ DFG SSA should use GetLocal for arguments, and the GetArgument node type should be removed
+ https://bugs.webkit.org/show_bug.cgi?id=141623
+
+ Reviewed by Oliver Hunt.
+
+ During development of https://bugs.webkit.org/show_bug.cgi?id=141332, I realized that I
+ needed to use GetArgument for loading something that has magically already appeared on the
+ stack, so currently trunk sort of allows this. But then I realized three things:
+
+ - A GetArgument with a non-JSValue flush format means speculating that the value on the
+ stack obeys that format, rather than just assuming that that it already has that format.
+ In bug 141332, I want it to assume rather than speculate. That also happens to be more
+ intuitive; I don't think I was wrong to expect that.
+
+ - The node I really want is GetLocal. I'm just getting the value of the local and I don't
+ want to do anything else.
+
+ - Maybe it would be easier if we just used GetLocal for all of the cases where we currently
+ use GetArgument.
+
+ This changes the FTL to do argument speculations in the prologue just like the DFG does.
+ This brings some consistency to our system, and allows us to get rid of the GetArgument
+ node. The speculations that the FTL must do are now made explicit in the m_argumentFormats
+ vector in DFG::Graph. This has natural DCE behavior: even if all uses of the argument are
+ dead we will still speculate. We already have safeguards to ensure we only speculate if
+ there are uses that benefit from speculation (which is a much more conservative criterion
+ than DCE).
+
+ * dfg/DFGAbstractInterpreterInlines.h:
+ (JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
+ * dfg/DFGClobberize.h:
+ (JSC::DFG::clobberize):
+ * dfg/DFGDCEPhase.cpp:
+ (JSC::DFG::DCEPhase::run):
+ * dfg/DFGDoesGC.cpp:
+ (JSC::DFG::doesGC):
+ * dfg/DFGFixupPhase.cpp:
+ (JSC::DFG::FixupPhase::fixupNode):
+ * dfg/DFGFlushFormat.h:
+ (JSC::DFG::typeFilterFor):
+ * dfg/DFGGraph.cpp:
+ (JSC::DFG::Graph::dump):
+ * dfg/DFGGraph.h:
+ (JSC::DFG::Graph::valueProfileFor):
+ (JSC::DFG::Graph::methodOfGettingAValueProfileFor):
+ * dfg/DFGInPlaceAbstractState.cpp:
+ (JSC::DFG::InPlaceAbstractState::initialize):
+ * dfg/DFGNode.cpp:
+ (JSC::DFG::Node::hasVariableAccessData):
+ * dfg/DFGNodeType.h:
+ * dfg/DFGOSRAvailabilityAnalysisPhase.cpp:
+ (JSC::DFG::OSRAvailabilityAnalysisPhase::run):
+ (JSC::DFG::LocalOSRAvailabilityCalculator::executeNode):
+ * dfg/DFGPredictionPropagationPhase.cpp:
+ (JSC::DFG::PredictionPropagationPhase::propagate):
+ * dfg/DFGPutLocalSinkingPhase.cpp:
+ * dfg/DFGSSAConversionPhase.cpp:
+ (JSC::DFG::SSAConversionPhase::run):
+ * dfg/DFGSafeToExecute.h:
+ (JSC::DFG::safeToExecute):
+ * dfg/DFGSpeculativeJIT32_64.cpp:
+ (JSC::DFG::SpeculativeJIT::compile):
+ * dfg/DFGSpeculativeJIT64.cpp:
+ (JSC::DFG::SpeculativeJIT::compile):
+ * ftl/FTLCapabilities.cpp:
+ (JSC::FTL::canCompile):
+ * ftl/FTLLowerDFGToLLVM.cpp:
+ (JSC::FTL::LowerDFGToLLVM::lower):
+ (JSC::FTL::LowerDFGToLLVM::compileNode):
+ (JSC::FTL::LowerDFGToLLVM::compileGetLocal):
+ (JSC::FTL::LowerDFGToLLVM::compileGetArgument): Deleted.
+ * tests/stress/dead-speculating-argument-use.js: Added.
+ (foo):
+ (o.valueOf):
+
2015-02-15 Filip Pizlo <fpi...@apple.com>
Rare case profiling should actually work
Modified: trunk/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h (180159 => 180160)
--- trunk/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h 2015-02-16 19:05:06 UTC (rev 180159)
+++ trunk/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h 2015-02-16 19:27:37 UTC (rev 180160)
@@ -141,18 +141,6 @@
break;
}
- case GetArgument: {
- ASSERT(m_graph.m_form == SSA);
- VariableAccessData* variable = node->variableAccessData();
- AbstractValue& value = m_state.variables().operand(variable->local().offset());
- ASSERT(value.isHeapTop());
- FiltrationResult result =
- value.filter(typeFilterFor(useKindFor(variable->flushFormat())));
- ASSERT_UNUSED(result, result == FiltrationOK);
- forNode(node) = value;
- break;
- }
-
case ExtractOSREntryLocal: {
if (!(node->unlinkedLocal().isArgument())
&& m_graph.m_lazyVars.get(node->unlinkedLocal().toLocal())) {
@@ -170,6 +158,8 @@
case GetLocal: {
VariableAccessData* variableAccessData = node->variableAccessData();
AbstractValue value = m_state.variables().operand(variableAccessData->local().offset());
+ // The value in the local should already be checked.
+ DFG_ASSERT(m_graph, node, value.isType(typeFilterFor(variableAccessData->flushFormat())));
if (value.value())
m_state.setFoundConstants(true);
forNode(node) = value;
Modified: trunk/Source/_javascript_Core/dfg/DFGClobberize.h (180159 => 180160)
--- trunk/Source/_javascript_Core/dfg/DFGClobberize.h 2015-02-16 19:05:06 UTC (rev 180159)
+++ trunk/Source/_javascript_Core/dfg/DFGClobberize.h 2015-02-16 19:27:37 UTC (rev 180160)
@@ -391,7 +391,6 @@
return;
case GetLocal:
- case GetArgument:
read(AbstractHeap(Variables, node->local()));
def(HeapLocation(VariableLoc, AbstractHeap(Variables, node->local())), node);
return;
Modified: trunk/Source/_javascript_Core/dfg/DFGDCEPhase.cpp (180159 => 180160)
--- trunk/Source/_javascript_Core/dfg/DFGDCEPhase.cpp 2015-02-16 19:05:06 UTC (rev 180159)
+++ trunk/Source/_javascript_Core/dfg/DFGDCEPhase.cpp 2015-02-16 19:27:37 UTC (rev 180160)
@@ -53,12 +53,21 @@
if (m_graph.m_form == SSA) {
for (BasicBlock* block : m_graph.blocksInPreOrder())
fixupBlock(block);
+
+ // This is like cleanVariables, but has a much simpler approach to GetLocal.
+ for (unsigned i = m_graph.m_arguments.size(); i--;) {
+ Node* node = m_graph.m_arguments[i];
+ if (!node)
+ continue;
+ if (node->op() != Phantom && node->op() != Check && node->shouldGenerate())
+ continue;
+ m_graph.m_arguments[i] = nullptr;
+ }
} else {
RELEASE_ASSERT(m_graph.m_form == ThreadedCPS);
for (BlockIndex blockIndex = 0; blockIndex < m_graph.numBlocks(); ++blockIndex)
fixupBlock(m_graph.block(blockIndex));
-
cleanVariables(m_graph.m_arguments);
}
Modified: trunk/Source/_javascript_Core/dfg/DFGDoesGC.cpp (180159 => 180160)
--- trunk/Source/_javascript_Core/dfg/DFGDoesGC.cpp 2015-02-16 19:05:06 UTC (rev 180159)
+++ trunk/Source/_javascript_Core/dfg/DFGDoesGC.cpp 2015-02-16 19:27:37 UTC (rev 180160)
@@ -53,7 +53,6 @@
case SetLocal:
case MovHint:
case ZombieHint:
- case GetArgument:
case Phantom:
case HardPhantom:
case Upsilon:
Modified: trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp (180159 => 180160)
--- trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp 2015-02-16 19:05:06 UTC (rev 180159)
+++ trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp 2015-02-16 19:27:37 UTC (rev 180160)
@@ -1033,7 +1033,6 @@
case GetArrayLength:
case Phi:
case Upsilon:
- case GetArgument:
case GetIndexedPropertyStorage:
case GetTypedArrayByteOffset:
case LastNodeType:
Modified: trunk/Source/_javascript_Core/dfg/DFGFlushFormat.h (180159 => 180160)
--- trunk/Source/_javascript_Core/dfg/DFGFlushFormat.h 2015-02-16 19:05:06 UTC (rev 180159)
+++ trunk/Source/_javascript_Core/dfg/DFGFlushFormat.h 2015-02-16 19:27:37 UTC (rev 180160)
@@ -93,6 +93,11 @@
return UntypedUse;
}
+inline SpeculatedType typeFilterFor(FlushFormat format)
+{
+ return typeFilterFor(useKindFor(format));
+}
+
inline DataFormat dataFormatFor(FlushFormat format)
{
switch (format) {
Modified: trunk/Source/_javascript_Core/dfg/DFGGraph.cpp (180159 => 180160)
--- trunk/Source/_javascript_Core/dfg/DFGGraph.cpp 2015-02-16 19:05:06 UTC (rev 180159)
+++ trunk/Source/_javascript_Core/dfg/DFGGraph.cpp 2015-02-16 19:27:37 UTC (rev 180160)
@@ -425,6 +425,10 @@
out.print("\n");
out.print("DFG for ", CodeBlockWithJITType(m_codeBlock, JITCode::DFGJIT), ":\n");
out.print(" Fixpoint state: ", m_fixpointState, "; Form: ", m_form, "; Unification state: ", m_unificationState, "; Ref count state: ", m_refCountState, "\n");
+ if (m_form == SSA)
+ out.print(" Argument formats: ", listDump(m_argumentFormats), "\n");
+ else
+ out.print(" Arguments: ", listDump(m_arguments), "\n");
out.print("\n");
Node* lastNode = 0;
Modified: trunk/Source/_javascript_Core/dfg/DFGGraph.h (180159 => 180160)
--- trunk/Source/_javascript_Core/dfg/DFGGraph.h 2015-02-16 19:05:06 UTC (rev 180159)
+++ trunk/Source/_javascript_Core/dfg/DFGGraph.h 2015-02-16 19:27:37 UTC (rev 180160)
@@ -475,21 +475,19 @@
ValueProfile* valueProfileFor(Node* node)
{
if (!node)
- return 0;
+ return nullptr;
CodeBlock* profiledBlock = baselineCodeBlockFor(node->origin.semantic);
- if (node->op() == GetArgument)
- return profiledBlock->valueProfileForArgument(node->local().toArgument());
-
if (node->hasLocal(*this)) {
- if (m_form == SSA)
- return 0;
if (!node->local().isArgument())
return 0;
int argument = node->local().toArgument();
- if (node->variableAccessData() != m_arguments[argument]->variableAccessData())
- return 0;
+ Node* argumentNode = m_arguments[argument];
+ if (!argumentNode)
+ return nullptr;
+ if (node->variableAccessData() != argumentNode->variableAccessData())
+ return nullptr;
return profiledBlock->valueProfileForArgument(argument);
}
@@ -504,16 +502,19 @@
if (!node)
return MethodOfGettingAValueProfile();
- CodeBlock* profiledBlock = baselineCodeBlockFor(node->origin.semantic);
+ if (ValueProfile* valueProfile = valueProfileFor(node))
+ return MethodOfGettingAValueProfile(valueProfile);
if (node->op() == GetLocal) {
+ CodeBlock* profiledBlock = baselineCodeBlockFor(node->origin.semantic);
+
return MethodOfGettingAValueProfile::fromLazyOperand(
profiledBlock,
LazyOperandValueProfileKey(
node->origin.semantic.bytecodeIndex, node->local()));
}
- return MethodOfGettingAValueProfile(valueProfileFor(node));
+ return MethodOfGettingAValueProfile();
}
bool usesArguments() const
@@ -811,7 +812,37 @@
Bag<FrozenValue> m_frozenValues;
Bag<StorageAccessData> m_storageAccessData;
+
+ // In CPS, this is all of the SetArgument nodes for the arguments in the machine code block
+ // that survived DCE. All of them except maybe "this" will survive DCE, because of the Flush
+ // nodes.
+ //
+ // In SSA, this is all of the GetLocal nodes for the arguments in the machine code block that
+ // may have some speculation in the prologue and survived DCE. Note that to get the speculation
+ // for an argument in SSA, you must use m_argumentFormats, since we still have to speculate
+ // even if the argument got killed. For example:
+ //
+ // function foo(x) {
+ // var tmp = x + 1;
+ // }
+ //
+ // Assume that x is always int during profiling. The ArithAdd for "x + 1" will be dead and will
+ // have a proven check for the edge to "x". So, we will not insert a Check node and we will
+ // kill the GetLocal for "x". But, we must do the int check in the progolue, because that's the
+ // thing we used to allow DCE of ArithAdd. Otherwise the add could be impure:
+ //
+ // var o = {
+ // valueOf: function() { do side effects; }
+ // };
+ // foo(o);
+ //
+ // If we DCE the ArithAdd and we remove the int check on x, then this won't do the side
+ // effects.
Vector<Node*, 8> m_arguments;
+
+ // In CPS, this is meaningless. In SSA, this is the argument speculation that we've locked in.
+ Vector<FlushFormat> m_argumentFormats;
+
SegmentedVector<VariableAccessData, 16> m_variableAccessData;
SegmentedVector<ArgumentPosition, 8> m_argumentPositions;
SegmentedVector<StructureSet, 16> m_structureSet;
Modified: trunk/Source/_javascript_Core/dfg/DFGInPlaceAbstractState.cpp (180159 => 180160)
--- trunk/Source/_javascript_Core/dfg/DFGInPlaceAbstractState.cpp 2015-02-16 19:05:06 UTC (rev 180159)
+++ trunk/Source/_javascript_Core/dfg/DFGInPlaceAbstractState.cpp 2015-02-16 19:27:37 UTC (rev 180160)
@@ -96,28 +96,37 @@
root->cfaStructureClobberStateAtTail = StructuresAreWatched;
for (size_t i = 0; i < root->valuesAtHead.numberOfArguments(); ++i) {
root->valuesAtTail.argument(i).clear();
- if (m_graph.m_form == SSA) {
- root->valuesAtHead.argument(i).makeHeapTop();
- continue;
+
+ FlushFormat format;
+ if (m_graph.m_form == SSA)
+ format = m_graph.m_argumentFormats[i];
+ else {
+ Node* node = m_graph.m_arguments[i];
+ if (!node)
+ format = FlushedJSValue;
+ else {
+ ASSERT(node->op() == SetArgument);
+ format = node->variableAccessData()->flushFormat();
+ }
}
- Node* node = root->variablesAtHead.argument(i);
- ASSERT(node->op() == SetArgument);
- if (!node->variableAccessData()->shouldUnboxIfPossible()) {
- root->valuesAtHead.argument(i).makeHeapTop();
- continue;
- }
-
- SpeculatedType prediction =
- node->variableAccessData()->argumentAwarePrediction();
- if (isInt32Speculation(prediction))
+ switch (format) {
+ case FlushedInt32:
root->valuesAtHead.argument(i).setType(SpecInt32);
- else if (isBooleanSpeculation(prediction))
+ break;
+ case FlushedBoolean:
root->valuesAtHead.argument(i).setType(SpecBoolean);
- else if (isCellSpeculation(prediction))
+ break;
+ case FlushedCell:
root->valuesAtHead.argument(i).setType(SpecCell);
- else
+ break;
+ case FlushedJSValue:
root->valuesAtHead.argument(i).makeHeapTop();
+ break;
+ default:
+ DFG_CRASH(m_graph, nullptr, "Bad flush format for argument");
+ break;
+ }
}
for (size_t i = 0; i < root->valuesAtHead.numberOfLocals(); ++i) {
Node* node = root->variablesAtHead.local(i);
Modified: trunk/Source/_javascript_Core/dfg/DFGNode.cpp (180159 => 180160)
--- trunk/Source/_javascript_Core/dfg/DFGNode.cpp 2015-02-16 19:05:06 UTC (rev 180159)
+++ trunk/Source/_javascript_Core/dfg/DFGNode.cpp 2015-02-16 19:27:37 UTC (rev 180160)
@@ -74,7 +74,6 @@
case Phi:
return graph.m_form != SSA;
case GetLocal:
- case GetArgument:
case SetLocal:
case SetArgument:
case Flush:
Modified: trunk/Source/_javascript_Core/dfg/DFGNodeType.h (180159 => 180160)
--- trunk/Source/_javascript_Core/dfg/DFGNodeType.h 2015-02-16 19:05:06 UTC (rev 180159)
+++ trunk/Source/_javascript_Core/dfg/DFGNodeType.h 2015-02-16 19:27:37 UTC (rev 180160)
@@ -61,7 +61,6 @@
macro(KillLocal, NodeMustGenerate) \
macro(MovHint, 0) \
macro(ZombieHint, 0) \
- macro(GetArgument, NodeResultJS | NodeMustGenerate) \
macro(Phantom, NodeMustGenerate) \
macro(HardPhantom, NodeMustGenerate) /* Like Phantom, but we never remove any of its children. */ \
macro(Check, NodeMustGenerate) /* Used if we want just a type check but not liveness. Non-checking uses will be removed. */\
Modified: trunk/Source/_javascript_Core/dfg/DFGOSRAvailabilityAnalysisPhase.cpp (180159 => 180160)
--- trunk/Source/_javascript_Core/dfg/DFGOSRAvailabilityAnalysisPhase.cpp 2015-02-16 19:05:06 UTC (rev 180159)
+++ trunk/Source/_javascript_Core/dfg/DFGOSRAvailabilityAnalysisPhase.cpp 2015-02-16 19:27:37 UTC (rev 180160)
@@ -58,10 +58,11 @@
BasicBlock* root = m_graph.block(0);
root->ssa->availabilityAtHead.m_locals.fill(Availability::unavailable());
- for (unsigned argument = root->ssa->availabilityAtHead.m_locals.numberOfArguments(); argument--;) {
- root->ssa->availabilityAtHead.m_locals.argument(argument) =
- Availability::unavailable().withFlush(
- FlushedAt(FlushedJSValue, virtualRegisterForArgument(argument)));
+ for (unsigned argument = m_graph.m_argumentFormats.size(); argument--;) {
+ FlushedAt flushedAt = FlushedAt(
+ m_graph.m_argumentFormats[argument],
+ virtualRegisterForArgument(argument));
+ root->ssa->availabilityAtHead.m_locals.argument(argument) = Availability(flushedAt);
}
// This could be made more efficient by processing blocks in reverse postorder.
@@ -138,7 +139,7 @@
break;
}
- case GetArgument: {
+ case GetLocal: {
VariableAccessData* variable = node->variableAccessData();
m_availability.m_locals.operand(variable->local()) =
Availability(node, variable->flushedAt());
Modified: trunk/Source/_javascript_Core/dfg/DFGPredictionPropagationPhase.cpp (180159 => 180160)
--- trunk/Source/_javascript_Core/dfg/DFGPredictionPropagationPhase.cpp 2015-02-16 19:05:06 UTC (rev 180159)
+++ trunk/Source/_javascript_Core/dfg/DFGPredictionPropagationPhase.cpp 2015-02-16 19:27:37 UTC (rev 180160)
@@ -552,7 +552,6 @@
break;
case Upsilon:
- case GetArgument:
// These don't get inserted until we go into SSA.
RELEASE_ASSERT_NOT_REACHED();
break;
Modified: trunk/Source/_javascript_Core/dfg/DFGPutLocalSinkingPhase.cpp (180159 => 180160)
--- trunk/Source/_javascript_Core/dfg/DFGPutLocalSinkingPhase.cpp 2015-02-16 19:05:06 UTC (rev 180159)
+++ trunk/Source/_javascript_Core/dfg/DFGPutLocalSinkingPhase.cpp 2015-02-16 19:27:37 UTC (rev 180160)
@@ -371,7 +371,7 @@
ssaCalculator.newDef(
operandToVariable.operand(node->local()), block, node->child1().node());
break;
- case GetArgument:
+ case GetLocal:
ssaCalculator.newDef(
operandToVariable.operand(node->local()), block, node);
break;
@@ -450,13 +450,6 @@
break;
}
- case GetArgument: {
- VariableAccessData* variable = node->variableAccessData();
- VirtualRegister operand = variable->local();
- mapping.operand(operand) = node;
- break;
- }
-
case KillLocal: {
deferred.operand(node->unlinkedLocal()) = VariableDeferral();
break;
@@ -489,6 +482,16 @@
preciseLocalClobberize(
m_graph, node, escapeHandler, escapeHandler,
[&] (VirtualRegister, Node*) { });
+
+ // If we're a GetLocal, then we also create a mapping.
+ // FIXME: We should be able to just eliminate such GetLocals, when we know
+ // what their incoming value will be.
+ // https://bugs.webkit.org/show_bug.cgi?id=141624
+ if (node->op() == GetLocal) {
+ VariableAccessData* variable = node->variableAccessData();
+ VirtualRegister operand = variable->local();
+ mapping.operand(operand) = node;
+ }
break;
} }
}
Modified: trunk/Source/_javascript_Core/dfg/DFGSSAConversionPhase.cpp (180159 => 180160)
--- trunk/Source/_javascript_Core/dfg/DFGSSAConversionPhase.cpp 2015-02-16 19:05:06 UTC (rev 180159)
+++ trunk/Source/_javascript_Core/dfg/DFGSSAConversionPhase.cpp 2015-02-16 19:27:37 UTC (rev 180160)
@@ -73,7 +73,7 @@
}
// Find all SetLocals and create Defs for them. We handle SetArgument by creating a
- // GetArgument.
+ // GetLocal, and recording the flush format.
for (BlockIndex blockIndex = m_graph.numBlocks(); blockIndex--;) {
BasicBlock* block = m_graph.block(blockIndex);
if (!block)
@@ -97,7 +97,9 @@
ASSERT(node->op() == SetArgument);
childNode = m_insertionSet.insertNode(
nodeIndex, node->variableAccessData()->prediction(),
- GetArgument, node->origin, OpInfo(node->variableAccessData()));
+ GetLocal, node->origin, OpInfo(node->variableAccessData()));
+ m_argumentGetters.add(childNode);
+ m_argumentMapping.add(node, childNode);
}
m_calculator.newDef(
@@ -198,14 +200,14 @@
// - PhantomLocal becomes Phantom, and its child is whatever is specified by
// valueForOperand.
//
- // - SetArgument is removed. Note that GetArgument nodes have already been inserted.
+ // - SetArgument is removed. Note that GetLocal nodes have already been inserted.
Operands<Node*> valueForOperand(OperandsLike, m_graph.block(0)->variablesAtHead);
for (BasicBlock* block : m_graph.blocksInPreOrder()) {
valueForOperand.clear();
// CPS will claim that the root block has all arguments live. But we have already done
// the first step of SSA conversion: argument locals are no longer live at head;
- // instead we have GetArgument nodes for extracting the values of arguments. So, we
+ // instead we have GetLocal nodes for extracting the values of arguments. So, we
// skip the at-head available value calculation for the root block.
if (block != m_graph.block(0)) {
for (size_t i = valueForOperand.size(); i--;) {
@@ -293,9 +295,15 @@
}
case GetLocal: {
+ VariableAccessData* variable = node->variableAccessData();
+ if (m_argumentGetters.contains(node)) {
+ if (verbose)
+ dataLog("Mapping: ", variable->local(), " -> ", node, "\n");
+ valueForOperand.operand(variable->local()) = node;
+ break;
+ }
node->children.reset();
- VariableAccessData* variable = node->variableAccessData();
if (variable->isCaptured())
break;
@@ -337,15 +345,6 @@
break;
}
- case GetArgument: {
- VariableAccessData* variable = node->variableAccessData();
- ASSERT(!variable->isCaptured());
- if (verbose)
- dataLog("Mapping: ", variable->local(), " -> ", node, "\n");
- valueForOperand.operand(variable->local()) = node;
- break;
- }
-
default:
break;
}
@@ -392,7 +391,21 @@
block->ssa = std::make_unique<BasicBlock::SSAData>(block);
}
- m_graph.m_arguments.clear();
+ m_graph.m_argumentFormats.resize(m_graph.m_arguments.size());
+ for (unsigned i = m_graph.m_arguments.size(); i--;) {
+ FlushFormat format = FlushedJSValue;
+
+ Node* node = m_argumentMapping.get(m_graph.m_arguments[i]);
+
+ // m_argumentMapping.get could return null for a captured local. That's fine. We only
+ // track the argument loads of those arguments for which we speculate type. We don't
+ // speculate type for captured arguments.
+ if (node)
+ format = node->variableAccessData()->flushFormat();
+
+ m_graph.m_argumentFormats[i] = format;
+ m_graph.m_arguments[i] = node; // Record the load that loads the arguments for the benefit of exit profiling.
+ }
m_graph.m_form = SSA;
@@ -408,6 +421,8 @@
SSACalculator m_calculator;
InsertionSet m_insertionSet;
HashMap<VariableAccessData*, SSACalculator::Variable*> m_ssaVariableForVariable;
+ HashMap<Node*, Node*> m_argumentMapping;
+ HashSet<Node*> m_argumentGetters;
Vector<VariableAccessData*> m_variableForSSAIndex;
};
Modified: trunk/Source/_javascript_Core/dfg/DFGSafeToExecute.h (180159 => 180160)
--- trunk/Source/_javascript_Core/dfg/DFGSafeToExecute.h 2015-02-16 19:05:06 UTC (rev 180159)
+++ trunk/Source/_javascript_Core/dfg/DFGSafeToExecute.h 2015-02-16 19:27:37 UTC (rev 180160)
@@ -122,7 +122,6 @@
case KillLocal:
case MovHint:
case ZombieHint:
- case GetArgument:
case Phantom:
case HardPhantom:
case Upsilon:
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp (180159 => 180160)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp 2015-02-16 19:05:06 UTC (rev 180159)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp 2015-02-16 19:27:37 UTC (rev 180160)
@@ -4898,7 +4898,6 @@
case LastNodeType:
case Phi:
case Upsilon:
- case GetArgument:
case ExtractOSREntryLocal:
case CheckTierUpInLoop:
case CheckTierUpAtReturn:
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp (180159 => 180160)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp 2015-02-16 19:05:06 UTC (rev 180159)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp 2015-02-16 19:27:37 UTC (rev 180160)
@@ -4993,7 +4993,6 @@
case LastNodeType:
case Phi:
case Upsilon:
- case GetArgument:
case ExtractOSREntryLocal:
case CheckInBounds:
case ArithIMul:
Modified: trunk/Source/_javascript_Core/ftl/FTLCapabilities.cpp (180159 => 180160)
--- trunk/Source/_javascript_Core/ftl/FTLCapabilities.cpp 2015-02-16 19:05:06 UTC (rev 180159)
+++ trunk/Source/_javascript_Core/ftl/FTLCapabilities.cpp 2015-02-16 19:27:37 UTC (rev 180160)
@@ -51,7 +51,6 @@
case KillLocal:
case MovHint:
case ZombieHint:
- case GetArgument:
case Phantom:
case HardPhantom:
case Flush:
Modified: trunk/Source/_javascript_Core/ftl/FTLLowerDFGToLLVM.cpp (180159 => 180160)
--- trunk/Source/_javascript_Core/ftl/FTLLowerDFGToLLVM.cpp 2015-02-16 19:05:06 UTC (rev 180159)
+++ trunk/Source/_javascript_Core/ftl/FTLLowerDFGToLLVM.cpp 2015-02-16 19:27:37 UTC (rev 180160)
@@ -135,6 +135,8 @@
m_prologue = FTL_NEW_BLOCK(m_out, ("Prologue"));
LBasicBlock stackOverflow = FTL_NEW_BLOCK(m_out, ("Stack overflow"));
m_handleExceptions = FTL_NEW_BLOCK(m_out, ("Handle Exceptions"));
+
+ LBasicBlock checkArguments = FTL_NEW_BLOCK(m_out, ("Check arguments"));
for (BlockIndex blockIndex = 0; blockIndex < m_graph.numBlocks(); ++blockIndex) {
m_highBlock = m_graph.block(blockIndex);
@@ -193,7 +195,7 @@
m_out.storePtr(m_out.constIntPtr(codeBlock()), addressFor(JSStack::CodeBlock));
m_out.branch(
- didOverflowStack(), rarely(stackOverflow), usually(lowBlock(m_graph.block(0))));
+ didOverflowStack(), rarely(stackOverflow), usually(checkArguments));
m_out.appendTo(stackOverflow, m_handleExceptions);
m_out.call(m_out.operation(operationThrowStackOverflowError), m_callFrame, m_out.constIntPtr(codeBlock()));
@@ -203,13 +205,58 @@
m_out.constInt32(MacroAssembler::maxJumpReplacementSize()));
m_out.unreachable();
- m_out.appendTo(m_handleExceptions, lowBlock(m_graph.block(0)));
+ m_out.appendTo(m_handleExceptions, checkArguments);
m_ftlState.handleExceptionStackmapID = m_stackmapIDs++;
m_out.call(
m_out.stackmapIntrinsic(), m_out.constInt64(m_ftlState.handleExceptionStackmapID),
m_out.constInt32(MacroAssembler::maxJumpReplacementSize()));
m_out.unreachable();
-
+
+ m_out.appendTo(checkArguments, lowBlock(m_graph.block(0)));
+ availabilityMap().clear();
+ availabilityMap().m_locals = Operands<Availability>(codeBlock()->numParameters(), 0);
+ for (unsigned i = codeBlock()->numParameters(); i--;) {
+ availabilityMap().m_locals.argument(i) =
+ Availability(FlushedAt(FlushedJSValue, virtualRegisterForArgument(i)));
+ }
+ m_codeOriginForExitTarget = CodeOrigin(0);
+ m_codeOriginForExitProfile = CodeOrigin(0);
+ m_node = nullptr;
+ for (unsigned i = codeBlock()->numParameters(); i--;) {
+ Node* node = m_graph.m_arguments[i];
+ VirtualRegister operand = virtualRegisterForArgument(i);
+
+ LValue jsValue = m_out.load64(addressFor(operand));
+
+ if (node) {
+ DFG_ASSERT(m_graph, node, operand == node->variableAccessData()->machineLocal());
+
+ // This is a hack, but it's an effective one. It allows us to do CSE on the
+ // primordial load of arguments. This assumes that the GetLocal that got put in
+ // place of the original SetArgument doesn't have any effects before it. This
+ // should hold true.
+ m_loadedArgumentValues.add(node, jsValue);
+ }
+
+ switch (m_graph.m_argumentFormats[i]) {
+ case FlushedInt32:
+ speculate(BadType, jsValueValue(jsValue), node, isNotInt32(jsValue));
+ break;
+ case FlushedBoolean:
+ speculate(BadType, jsValueValue(jsValue), node, isNotBoolean(jsValue));
+ break;
+ case FlushedCell:
+ speculate(BadType, jsValueValue(jsValue), node, isNotCell(jsValue));
+ break;
+ case FlushedJSValue:
+ break;
+ default:
+ DFG_CRASH(m_graph, node, "Bad flush format for argument");
+ break;
+ }
+ }
+ m_out.jump(lowBlock(m_graph.block(0)));
+
for (BasicBlock* block : preOrder)
compileBlock(block);
@@ -381,9 +428,6 @@
case BooleanToNumber:
compileBooleanToNumber();
break;
- case GetArgument:
- compileGetArgument();
- break;
case ExtractOSREntryLocal:
compileExtractOSREntryLocal();
break;
@@ -981,36 +1025,6 @@
}
}
- void compileGetArgument()
- {
- VariableAccessData* variable = m_node->variableAccessData();
- VirtualRegister operand = variable->machineLocal();
- DFG_ASSERT(m_graph, m_node, operand.isArgument());
-
- LValue jsValue = m_out.load64(addressFor(operand));
-
- switch (useKindFor(variable->flushFormat())) {
- case Int32Use:
- speculate(BadType, jsValueValue(jsValue), m_node, isNotInt32(jsValue));
- setInt32(unboxInt32(jsValue));
- break;
- case CellUse:
- speculate(BadType, jsValueValue(jsValue), m_node, isNotCell(jsValue));
- setJSValue(jsValue);
- break;
- case BooleanUse:
- speculate(BadType, jsValueValue(jsValue), m_node, isNotBoolean(jsValue));
- setBoolean(unboxBoolean(jsValue));
- break;
- case UntypedUse:
- setJSValue(jsValue);
- break;
- default:
- DFG_CRASH(m_graph, m_node, "Bad use kind");
- break;
- }
- }
-
void compileExtractOSREntryLocal()
{
EncodedJSValue* buffer = static_cast<EncodedJSValue*>(
@@ -1020,13 +1034,16 @@
void compileGetLocal()
{
- // GetLocals arise only for captured variables.
+ // GetLocals arise only for captured variables and arguments. For arguments, we might have
+ // already loaded it.
+ if (LValue value = m_loadedArgumentValues.get(m_node)) {
+ setJSValue(value);
+ return;
+ }
VariableAccessData* variable = m_node->variableAccessData();
AbstractValue& value = m_state.variables().operand(variable->local());
- DFG_ASSERT(m_graph, m_node, variable->isCaptured());
-
if (isInt32Speculation(value.m_type))
setInt32(m_out.load32(payloadFor(variable->machineLocal())));
else
@@ -7014,6 +7031,11 @@
HashMap<Node*, LoweredNodeValue> m_storageValues;
HashMap<Node*, LoweredNodeValue> m_doubleValues;
+ // This is a bit of a hack. It prevents LLVM from having to do CSE on loading of arguments.
+ // It's nice to have these optimizations on our end because we can guarantee them a bit better.
+ // Probably also saves LLVM compile time.
+ HashMap<Node*, LValue> m_loadedArgumentValues;
+
HashMap<Node*, LValue> m_phis;
LocalOSRAvailabilityCalculator m_availabilityCalculator;
Added: trunk/Source/_javascript_Core/tests/stress/dead-speculating-argument-use.js (0 => 180160)
--- trunk/Source/_javascript_Core/tests/stress/dead-speculating-argument-use.js (rev 0)
+++ trunk/Source/_javascript_Core/tests/stress/dead-speculating-argument-use.js 2015-02-16 19:27:37 UTC (rev 180160)
@@ -0,0 +1,17 @@
+function foo(x) {
+ var tmp = x + 1;
+}
+
+noInline(foo);
+
+for (var i = 0; i < 10000; ++i)
+ foo(i);
+
+var didCall = false;
+var o = {
+ valueOf: function() { didCall = true; }
+};
+
+foo(o);
+if (!didCall)
+ throw "Error: didn't call valueOf";
