Modified: trunk/Source/_javascript_Core/ChangeLog (173671 => 173672)
--- trunk/Source/_javascript_Core/ChangeLog 2014-09-16 22:14:42 UTC (rev 173671)
+++ trunk/Source/_javascript_Core/ChangeLog 2014-09-16 22:18:23 UTC (rev 173672)
@@ -1,3 +1,43 @@
+2014-09-16 Filip Pizlo <[email protected]>
+
+ Local OSR availability calculation should be reusable
+ https://bugs.webkit.org/show_bug.cgi?id=136860
+
+ Reviewed by Oliver Hunt.
+
+ Previously, the FTL lowering repeated some of the logic of the OSR availability analysis
+ phase. Humorously, it actually did this logic a bit differently; for example the phase
+ would claim that a SetLocal makes both the flush and the node available while the FTL
+ only claimed that the flush was available. This different was benign, but still: yuck!
+
+ Also, previously if you wanted to use availability information then you'd have to repeat
+ some of the logic that both the phase itself and the FTL lowering already had.
+ Presumably, you could get epic style points for finding other benign ways in which to
+ make your copy of the logic different from the other two!
+
+ This reduces the amount of style points one could conceivably get in the future when
+ hacking JSC, by creating a single reusable thingy for computing local OSR availability.
+
+ * dfg/DFGOSRAvailabilityAnalysisPhase.cpp:
+ (JSC::DFG::OSRAvailabilityAnalysisPhase::run):
+ (JSC::DFG::LocalOSRAvailabilityCalculator::LocalOSRAvailabilityCalculator):
+ (JSC::DFG::LocalOSRAvailabilityCalculator::~LocalOSRAvailabilityCalculator):
+ (JSC::DFG::LocalOSRAvailabilityCalculator::beginBlock):
+ (JSC::DFG::LocalOSRAvailabilityCalculator::executeNode):
+ * dfg/DFGOSRAvailabilityAnalysisPhase.h:
+ * ftl/FTLLowerDFGToLLVM.cpp:
+ (JSC::FTL::LowerDFGToLLVM::LowerDFGToLLVM):
+ (JSC::FTL::LowerDFGToLLVM::compileBlock):
+ (JSC::FTL::LowerDFGToLLVM::compileNode):
+ (JSC::FTL::LowerDFGToLLVM::compileSetLocal):
+ (JSC::FTL::LowerDFGToLLVM::compileInvalidationPoint):
+ (JSC::FTL::LowerDFGToLLVM::appendOSRExit):
+ (JSC::FTL::LowerDFGToLLVM::buildExitArguments):
+ (JSC::FTL::LowerDFGToLLVM::availability):
+ (JSC::FTL::LowerDFGToLLVM::compileMovHint): Deleted.
+ (JSC::FTL::LowerDFGToLLVM::compileZombieHint): Deleted.
+ (JSC::FTL::LowerDFGToLLVM::initializeOSRExitStateForBlock): Deleted.
+
2014-09-16 Csaba Osztrogonác <[email protected]>
JSC test gardening
Modified: trunk/Source/_javascript_Core/dfg/DFGOSRAvailabilityAnalysisPhase.cpp (173671 => 173672)
--- trunk/Source/_javascript_Core/dfg/DFGOSRAvailabilityAnalysisPhase.cpp 2014-09-16 22:14:42 UTC (rev 173671)
+++ trunk/Source/_javascript_Core/dfg/DFGOSRAvailabilityAnalysisPhase.cpp 2014-09-16 22:18:23 UTC (rev 173672)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2013 Apple Inc. All rights reserved.
+ * Copyright (C) 2013, 2014 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -71,7 +71,8 @@
}
// This could be made more efficient by processing blocks in reverse postorder.
- Operands<Availability> availability;
+
+ LocalOSRAvailabilityCalculator calculator;
bool changed;
do {
changed = false;
@@ -81,54 +82,23 @@
if (!block)
continue;
- availability = block->ssa->availabilityAtHead;
+ calculator.beginBlock(block);
- for (unsigned nodeIndex = 0; nodeIndex < block->size(); ++nodeIndex) {
- Node* node = block->at(nodeIndex);
-
- switch (node->op()) {
- case SetLocal: {
- VariableAccessData* variable = node->variableAccessData();
- availability.operand(variable->local()) =
- Availability(node->child1().node(), variable->flushedAt());
- break;
- }
-
- case GetArgument: {
- VariableAccessData* variable = node->variableAccessData();
- availability.operand(variable->local()) =
- Availability(node, variable->flushedAt());
- break;
- }
-
- case MovHint: {
- availability.operand(node->unlinkedLocal()) =
- Availability(node->child1().node());
- break;
- }
-
- case ZombieHint: {
- availability.operand(node->unlinkedLocal()) =
- Availability::unavailable();
- break;
- }
-
- default:
- break;
- }
- }
+ for (unsigned nodeIndex = 0; nodeIndex < block->size(); ++nodeIndex)
+ calculator.executeNode(block->at(nodeIndex));
- if (availability == block->ssa->availabilityAtTail)
+ if (calculator.m_availability == block->ssa->availabilityAtTail)
continue;
- block->ssa->availabilityAtTail = availability;
+ block->ssa->availabilityAtTail = calculator.m_availability;
changed = true;
for (unsigned successorIndex = block->numSuccessors(); successorIndex--;) {
BasicBlock* successor = block->successor(successorIndex);
- for (unsigned i = availability.size(); i--;) {
- successor->ssa->availabilityAtHead[i] = availability[i].merge(
- successor->ssa->availabilityAtHead[i]);
+ for (unsigned i = calculator.m_availability.size(); i--;) {
+ successor->ssa->availabilityAtHead[i] =
+ calculator.m_availability[i].merge(
+ successor->ssa->availabilityAtHead[i]);
}
}
}
@@ -144,6 +114,53 @@
return runPhase<OSRAvailabilityAnalysisPhase>(graph);
}
+LocalOSRAvailabilityCalculator::LocalOSRAvailabilityCalculator()
+{
+}
+
+LocalOSRAvailabilityCalculator::~LocalOSRAvailabilityCalculator()
+{
+}
+
+void LocalOSRAvailabilityCalculator::beginBlock(BasicBlock* block)
+{
+ m_availability = block->ssa->availabilityAtHead;
+}
+
+void LocalOSRAvailabilityCalculator::executeNode(Node* node)
+{
+ switch (node->op()) {
+ case SetLocal: {
+ VariableAccessData* variable = node->variableAccessData();
+ m_availability.operand(variable->local()) =
+ Availability(node->child1().node(), variable->flushedAt());
+ break;
+ }
+
+ case GetArgument: {
+ VariableAccessData* variable = node->variableAccessData();
+ m_availability.operand(variable->local()) =
+ Availability(node, variable->flushedAt());
+ break;
+ }
+
+ case MovHint: {
+ m_availability.operand(node->unlinkedLocal()) =
+ Availability(node->child1().node());
+ break;
+ }
+
+ case ZombieHint: {
+ m_availability.operand(node->unlinkedLocal()) =
+ Availability::unavailable();
+ break;
+ }
+
+ default:
+ break;
+ }
+}
+
} } // namespace JSC::DFG
#endif // ENABLE(DFG_JIT)
Modified: trunk/Source/_javascript_Core/dfg/DFGOSRAvailabilityAnalysisPhase.h (173671 => 173672)
--- trunk/Source/_javascript_Core/dfg/DFGOSRAvailabilityAnalysisPhase.h 2014-09-16 22:14:42 UTC (rev 173671)
+++ trunk/Source/_javascript_Core/dfg/DFGOSRAvailabilityAnalysisPhase.h 2014-09-16 22:18:23 UTC (rev 173672)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2013 Apple Inc. All rights reserved.
+ * Copyright (C) 2013, 2014 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -28,6 +28,7 @@
#if ENABLE(DFG_JIT)
+#include "DFGBasicBlock.h"
#include "DFGCommon.h"
namespace JSC { namespace DFG {
@@ -35,10 +36,24 @@
class Graph;
// Computes BasicBlock::ssa->availabiltiyAtHead/Tail. This is a forward flow type inference
-// over MovHints and SetLocals.
+// over MovHints and SetLocals. This analysis is run directly by the Plan for preparing for
+// lowering to LLVM IR, but it can also be used as a utility.
bool performOSRAvailabilityAnalysis(Graph&);
+// Local calculator for figuring out the availability at any node in a basic block. Requires
+// having run the availability analysis.
+class LocalOSRAvailabilityCalculator {
+public:
+ LocalOSRAvailabilityCalculator();
+ ~LocalOSRAvailabilityCalculator();
+
+ void beginBlock(BasicBlock*);
+ void executeNode(Node*);
+
+ Operands<Availability> m_availability;
+};
+
} } // namespace JSC::DFG
#endif // ENABLE(DFG_JIT)
Modified: trunk/Source/_javascript_Core/ftl/FTLLowerDFGToLLVM.cpp (173671 => 173672)
--- trunk/Source/_javascript_Core/ftl/FTLLowerDFGToLLVM.cpp 2014-09-16 22:14:42 UTC (rev 173671)
+++ trunk/Source/_javascript_Core/ftl/FTLLowerDFGToLLVM.cpp 2014-09-16 22:18:23 UTC (rev 173672)
@@ -31,6 +31,7 @@
#include "CodeBlockWithJITType.h"
#include "DFGAbstractInterpreterInlines.h"
#include "DFGInPlaceAbstractState.h"
+#include "DFGOSRAvailabilityAnalysisPhase.h"
#include "FTLAbstractHeapRepository.h"
#include "FTLAvailableRecovery.h"
#include "FTLForOSREntryJITCode.h"
@@ -93,7 +94,6 @@
, m_ftlState(state)
, m_heaps(state.context)
, m_out(state.context)
- , m_availability(OperandsLike, state.graph.block(0)->variablesAtHead)
, m_state(state.graph)
, m_interpreter(state.graph, m_state)
, m_stackmapIDs(0)
@@ -295,7 +295,7 @@
return;
}
- initializeOSRExitStateForBlock();
+ m_availabilityCalculator.beginBlock(m_highBlock);
m_state.reset();
m_state.beginBasicBlock(m_highBlock);
@@ -388,18 +388,12 @@
case SetLocal:
compileSetLocal();
break;
- case MovHint:
- compileMovHint();
- break;
case GetMyArgumentsLength:
compileGetMyArgumentsLength();
break;
case GetMyArgumentByVal:
compileGetMyArgumentByVal();
break;
- case ZombieHint:
- compileZombieHint();
- break;
case Phantom:
case HardPhantom:
case Check:
@@ -739,12 +733,16 @@
case FunctionReentryWatchpoint:
case TypedArrayWatchpoint:
case AllocationProfileWatchpoint:
+ case MovHint:
+ case ZombieHint:
break;
default:
DFG_CRASH(m_graph, m_node, "Unrecognized node in FTL backend");
break;
}
+ m_availabilityCalculator.executeNode(m_node);
+
if (shouldExecuteEffects)
m_interpreter.executeEffects(nodeIndex);
@@ -1064,24 +1062,8 @@
DFG_CRASH(m_graph, m_node, "Bad flush format");
break;
}
-
- m_availability.operand(variable->local()) = Availability(variable->flushedAt());
}
- void compileMovHint()
- {
- ASSERT(m_node->containsMovHint());
- ASSERT(m_node->op() != ZombieHint);
-
- VirtualRegister operand = m_node->unlinkedLocal();
- m_availability.operand(operand) = Availability(m_node->child1().node());
- }
-
- void compileZombieHint()
- {
- m_availability.operand(m_node->unlinkedLocal()) = Availability::unavailable();
- }
-
void compilePhantom()
{
DFG_NODE_DO_TO_CHILDREN(m_graph, m_node, speculate);
@@ -3973,12 +3955,12 @@
void compileInvalidationPoint()
{
if (verboseCompilationEnabled())
- dataLog(" Invalidation point with availability: ", m_availability, "\n");
+ dataLog(" Invalidation point with availability: ", availability(), "\n");
m_ftlState.jitCode->osrExit.append(OSRExit(
UncountableInvalidation, InvalidValueFormat, MethodOfGettingAValueProfile(),
m_codeOriginForExitTarget, m_codeOriginForExitProfile,
- m_availability.numberOfArguments(), m_availability.numberOfLocals()));
+ availability().numberOfArguments(), availability().numberOfLocals()));
m_ftlState.finalizer->osrExit.append(OSRExitCompilationInfo());
OSRExit& exit = m_ftlState.jitCode->osrExit.last();
@@ -6360,16 +6342,11 @@
return m_blocks.get(block);
}
- void initializeOSRExitStateForBlock()
- {
- m_availability = m_highBlock->ssa->availabilityAtHead;
- }
-
void appendOSRExit(
ExitKind kind, FormattedValue lowValue, Node* highValue, LValue failCondition)
{
if (verboseCompilationEnabled()) {
- dataLog(" OSR exit #", m_ftlState.jitCode->osrExit.size(), " with availability: ", m_availability, "\n");
+ dataLog(" OSR exit #", m_ftlState.jitCode->osrExit.size(), " with availability: ", availability(), "\n");
if (!m_availableRecoveries.isEmpty())
dataLog(" Available recoveries: ", listDump(m_availableRecoveries), "\n");
}
@@ -6379,7 +6356,7 @@
m_ftlState.jitCode->osrExit.append(OSRExit(
kind, lowValue.format(), m_graph.methodOfGettingAValueProfileFor(highValue),
m_codeOriginForExitTarget, m_codeOriginForExitProfile,
- m_availability.numberOfArguments(), m_availability.numberOfLocals()));
+ availability().numberOfArguments(), availability().numberOfLocals()));
m_ftlState.finalizer->osrExit.append(OSRExitCompilationInfo());
OSRExit& exit = m_ftlState.jitCode->osrExit.last();
@@ -6427,7 +6404,7 @@
continue;
}
- Availability availability = m_availability[i];
+ Availability availability = this->availability()[i];
FlushedAt flush = availability.flushedAt();
switch (flush.format()) {
case DeadFlush:
@@ -6766,6 +6743,8 @@
m_out.unreachable();
}
+ Operands<Availability>& availability() { return m_availabilityCalculator.m_availability; }
+
VM& vm() { return m_graph.m_vm; }
CodeBlock* codeBlock() { return m_graph.m_codeBlock; }
@@ -6795,7 +6774,7 @@
HashMap<Node*, LValue> m_phis;
- Operands<Availability> m_availability;
+ LocalOSRAvailabilityCalculator m_availabilityCalculator;
Vector<AvailableRecovery, 3> m_availableRecoveries;
