Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (111648 => 111649)
--- trunk/Source/_javascript_Core/ChangeLog 2012-03-22 03:40:53 UTC (rev 111648)
+++ trunk/Source/_javascript_Core/ChangeLog 2012-03-22 03:47:55 UTC (rev 111649)
@@ -1,3 +1,36 @@
+2012-03-21 Filip Pizlo <[email protected]>
+
+ DFG speculation on booleans should be rationalized
+ https://bugs.webkit.org/show_bug.cgi?id=81840
+
+ Reviewed by Gavin Barraclough.
+
+ This removes isKnownBoolean() and replaces it with AbstractState-based
+ optimization, and cleans up the control flow in code gen methods for
+ Branch and LogicalNot. Also fixes a goof in Node::shouldSpeculateNumber,
+ and removes isKnownNotBoolean() since that method appeared to be a
+ helper used solely by 32_64's speculateBooleanOperation().
+
+ This is performance-neutral.
+
+ * dfg/DFGAbstractState.cpp:
+ (JSC::DFG::AbstractState::execute):
+ * dfg/DFGNode.h:
+ (JSC::DFG::Node::shouldSpeculateNumber):
+ * dfg/DFGSpeculativeJIT.cpp:
+ (DFG):
+ * dfg/DFGSpeculativeJIT.h:
+ (SpeculativeJIT):
+ * dfg/DFGSpeculativeJIT32_64.cpp:
+ (JSC::DFG::SpeculativeJIT::fillSpeculateBoolean):
+ (JSC::DFG::SpeculativeJIT::compileLogicalNot):
+ (JSC::DFG::SpeculativeJIT::emitBranch):
+ (JSC::DFG::SpeculativeJIT::compile):
+ * dfg/DFGSpeculativeJIT64.cpp:
+ (JSC::DFG::SpeculativeJIT::compileLogicalNot):
+ (JSC::DFG::SpeculativeJIT::emitBranch):
+ (JSC::DFG::SpeculativeJIT::compile):
+
2012-03-21 Mark Rowe <[email protected]>
Fix the build.
Modified: trunk/Source/_javascript_Core/dfg/DFGAbstractState.cpp (111648 => 111649)
--- trunk/Source/_javascript_Core/dfg/DFGAbstractState.cpp 2012-03-22 03:40:53 UTC (rev 111648)
+++ trunk/Source/_javascript_Core/dfg/DFGAbstractState.cpp 2012-03-22 03:47:55 UTC (rev 111649)
@@ -395,7 +395,7 @@
case LogicalNot: {
Node& child = m_graph[node.child1()];
- if (isBooleanPrediction(child.prediction()) || !child.prediction())
+ if (isBooleanPrediction(child.prediction()))
forNode(node.child1()).filter(PredictBoolean);
else if (child.shouldSpeculateFinalObjectOrOther())
forNode(node.child1()).filter(PredictFinalObject | PredictOther);
@@ -644,7 +644,7 @@
// propagation, and to take it one step further, where a variable's value
// is specialized on each direction of a branch. For now, we don't do this.
Node& child = m_graph[node.child1()];
- if (isBooleanPrediction(child.prediction()) || !child.prediction())
+ if (child.shouldSpeculateBoolean())
forNode(node.child1()).filter(PredictBoolean);
else if (child.shouldSpeculateFinalObjectOrOther())
forNode(node.child1()).filter(PredictFinalObject | PredictOther);
Modified: trunk/Source/_javascript_Core/dfg/DFGNode.h (111648 => 111649)
--- trunk/Source/_javascript_Core/dfg/DFGNode.h 2012-03-22 03:40:53 UTC (rev 111648)
+++ trunk/Source/_javascript_Core/dfg/DFGNode.h 2012-03-22 03:47:55 UTC (rev 111649)
@@ -694,7 +694,7 @@
bool shouldSpeculateNumber()
{
- return isNumberPrediction(prediction()) || prediction() == PredictNone;
+ return isNumberPrediction(prediction());
}
bool shouldNotSpeculateInteger()
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp (111648 => 111649)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2012-03-22 03:40:53 UTC (rev 111648)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2012-03-22 03:47:55 UTC (rev 111649)
@@ -184,31 +184,6 @@
|| (node.hasConstant() && !isNumberConstant(nodeIndex));
}
-bool SpeculativeJIT::isKnownBoolean(NodeIndex nodeIndex)
-{
- Node& node = m_jit.graph()[nodeIndex];
- if (node.hasBooleanResult())
- return true;
-
- if (isBooleanConstant(nodeIndex))
- return true;
-
- VirtualRegister virtualRegister = node.virtualRegister();
- GenerationInfo& info = m_generationInfo[virtualRegister];
-
- return info.isJSBoolean();
-}
-
-bool SpeculativeJIT::isKnownNotBoolean(NodeIndex nodeIndex)
-{
- Node& node = m_jit.graph()[nodeIndex];
- VirtualRegister virtualRegister = node.virtualRegister();
- GenerationInfo& info = m_generationInfo[virtualRegister];
- if (node.hasConstant() && !valueOfJSConstant(nodeIndex).isBoolean())
- return true;
- return !(info.isJSBoolean() || info.isUnknownJS());
-}
-
void SpeculativeJIT::writeBarrier(MacroAssembler& jit, GPRReg owner, GPRReg scratch1, GPRReg scratch2, WriteBarrierUseKind useKind)
{
UNUSED_PARAM(jit);
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.h (111648 => 111649)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.h 2012-03-22 03:40:53 UTC (rev 111648)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.h 2012-03-22 03:47:55 UTC (rev 111649)
@@ -730,9 +730,6 @@
bool isKnownNotInteger(NodeIndex);
bool isKnownNotNumber(NodeIndex);
- bool isKnownBoolean(NodeIndex);
- bool isKnownNotBoolean(NodeIndex);
-
bool isKnownNotCell(NodeIndex);
// Checks/accessors for constant values.
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp (111648 => 111649)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp 2012-03-22 03:40:53 UTC (rev 111648)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp 2012-03-22 03:47:55 UTC (rev 111649)
@@ -1285,15 +1285,15 @@
#if DFG_ENABLE(DEBUG_VERBOSE)
dataLog("SpecBool@%d ", nodeIndex);
#endif
- if (isKnownNotBoolean(nodeIndex)) {
+ Node& node = m_jit.graph()[nodeIndex];
+ VirtualRegister virtualRegister = node.virtualRegister();
+ GenerationInfo& info = m_generationInfo[virtualRegister];
+ if ((node.hasConstant() && !valueOfJSConstant(nodeIndex).isBoolean())
+ || !(info.isJSBoolean() || info.isUnknownJS())) {
terminateSpeculativeExecution(Uncountable, JSValueRegs(), NoNode);
return allocate();
}
- Node& node = at(nodeIndex);
- VirtualRegister virtualRegister = node.virtualRegister();
- GenerationInfo& info = m_generationInfo[virtualRegister];
-
switch (info.registerFormat()) {
case DataFormatNone: {
@@ -1478,7 +1478,7 @@
void SpeculativeJIT::compileLogicalNot(Node& node)
{
- if (isKnownBoolean(node.child1().index()) || isBooleanPrediction(m_jit.getPrediction(node.child1().index()))) {
+ if (at(node.child1()).shouldSpeculateBoolean()) {
SpeculateBooleanOperand value(this, node.child1());
GPRTemporary result(this, value);
m_jit.xor32(TrustedImm32(1), value.gpr(), result.gpr());
@@ -1567,7 +1567,7 @@
BlockIndex taken = node.takenBlockIndex();
BlockIndex notTaken = node.notTakenBlockIndex();
- if (isKnownBoolean(node.child1().index())) {
+ if (at(node.child1()).shouldSpeculateBoolean()) {
SpeculateBooleanOperand value(this, node.child1());
MacroAssembler::ResultCondition condition = MacroAssembler::NonZero;
@@ -2729,8 +2729,6 @@
// FIXME: Add string speculation here.
- bool wasPrimitive = isKnownNumeric(node.child1().index()) || isKnownBoolean(node.child1().index());
-
JSValueOperand op1(this, node.child1());
GPRTemporary resultTag(this, op1);
GPRTemporary resultPayload(this, op1, false);
@@ -2742,7 +2740,7 @@
op1.use();
- if (wasPrimitive) {
+ if (!(m_state.forNode(node.child1()).m_type & ~(PredictNumber | PredictBoolean))) {
m_jit.move(op1TagGPR, resultTagGPR);
m_jit.move(op1PayloadGPR, resultPayloadGPR);
} else {
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp (111648 => 111649)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp 2012-03-22 03:40:53 UTC (rev 111648)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp 2012-03-22 03:47:55 UTC (rev 111649)
@@ -1560,16 +1560,6 @@
void SpeculativeJIT::compileLogicalNot(Node& node)
{
- if (isKnownBoolean(node.child1().index())) {
- SpeculateBooleanOperand value(this, node.child1());
- GPRTemporary result(this, value);
-
- m_jit.move(value.gpr(), result.gpr());
- m_jit.xorPtr(TrustedImm32(true), result.gpr());
-
- jsValueResult(result.gpr(), m_compileIndex, DataFormatJSBoolean);
- return;
- }
if (at(node.child1()).shouldSpeculateFinalObjectOrOther()) {
compileObjectOrOtherLogicalNot(node.child1(), &JSFinalObject::s_info, !isFinalObjectOrOtherPrediction(m_state.forNode(node.child1()).m_type));
return;
@@ -1599,7 +1589,18 @@
}
PredictedType prediction = m_jit.getPrediction(node.child1());
- if (isBooleanPrediction(prediction) || !prediction) {
+ if (isBooleanPrediction(prediction)) {
+ if (isBooleanPrediction(m_state.forNode(node.child1()).m_type)) {
+ SpeculateBooleanOperand value(this, node.child1());
+ GPRTemporary result(this, value);
+
+ m_jit.move(value.gpr(), result.gpr());
+ m_jit.xorPtr(TrustedImm32(true), result.gpr());
+
+ jsValueResult(result.gpr(), m_compileIndex, DataFormatJSBoolean);
+ return;
+ }
+
JSValueOperand value(this, node.child1());
GPRTemporary result(this); // FIXME: We could reuse, but on speculation fail would need recovery to restore tag (akin to add).
@@ -1667,21 +1668,7 @@
BlockIndex taken = node.takenBlockIndex();
BlockIndex notTaken = node.notTakenBlockIndex();
- if (isKnownBoolean(node.child1().index())) {
- MacroAssembler::ResultCondition condition = MacroAssembler::NonZero;
-
- if (taken == (m_block + 1)) {
- condition = MacroAssembler::Zero;
- BlockIndex tmp = taken;
- taken = notTaken;
- notTaken = tmp;
- }
-
- branchTest32(condition, valueGPR, TrustedImm32(true), taken);
- jump(notTaken);
-
- noResult(m_compileIndex);
- } else if (at(node.child1()).shouldSpeculateFinalObjectOrOther()) {
+ if (at(node.child1()).shouldSpeculateFinalObjectOrOther()) {
emitObjectOrOtherBranch(node.child1(), taken, notTaken, &JSFinalObject::s_info, !isFinalObjectOrOtherPrediction(m_state.forNode(node.child1()).m_type));
} else if (at(node.child1()).shouldSpeculateArrayOrOther()) {
emitObjectOrOtherBranch(node.child1(), taken, notTaken, &JSArray::s_info, !isArrayOrOtherPrediction(m_state.forNode(node.child1()).m_type));
@@ -1708,18 +1695,32 @@
noResult(m_compileIndex);
} else {
- GPRTemporary result(this);
- GPRReg resultGPR = result.gpr();
-
bool predictBoolean = isBooleanPrediction(m_jit.getPrediction(node.child1()));
if (predictBoolean) {
- branchPtr(MacroAssembler::Equal, valueGPR, MacroAssembler::TrustedImmPtr(JSValue::encode(jsBoolean(false))), notTaken);
- branchPtr(MacroAssembler::Equal, valueGPR, MacroAssembler::TrustedImmPtr(JSValue::encode(jsBoolean(true))), taken);
-
- speculationCheck(BadType, JSValueRegs(valueGPR), node.child1(), m_jit.jump());
+ if (isBooleanPrediction(m_state.forNode(node.child1()).m_type)) {
+ MacroAssembler::ResultCondition condition = MacroAssembler::NonZero;
+
+ if (taken == (m_block + 1)) {
+ condition = MacroAssembler::Zero;
+ BlockIndex tmp = taken;
+ taken = notTaken;
+ notTaken = tmp;
+ }
+
+ branchTest32(condition, valueGPR, TrustedImm32(true), taken);
+ jump(notTaken);
+ } else {
+ branchPtr(MacroAssembler::Equal, valueGPR, MacroAssembler::TrustedImmPtr(JSValue::encode(jsBoolean(false))), notTaken);
+ branchPtr(MacroAssembler::Equal, valueGPR, MacroAssembler::TrustedImmPtr(JSValue::encode(jsBoolean(true))), taken);
+
+ speculationCheck(BadType, JSValueRegs(valueGPR), node.child1(), m_jit.jump());
+ }
value.use();
} else {
+ GPRTemporary result(this);
+ GPRReg resultGPR = result.gpr();
+
branchPtr(MacroAssembler::Equal, valueGPR, MacroAssembler::TrustedImmPtr(JSValue::encode(jsNumber(0))), notTaken);
branchPtr(MacroAssembler::AboveOrEqual, valueGPR, GPRInfo::tagTypeNumberRegister, taken);
@@ -2754,8 +2755,6 @@
// FIXME: Add string speculation here.
- bool wasPrimitive = isKnownNumeric(node.child1().index()) || isKnownBoolean(node.child1().index());
-
JSValueOperand op1(this, node.child1());
GPRTemporary result(this, op1);
@@ -2764,7 +2763,7 @@
op1.use();
- if (wasPrimitive)
+ if (!(m_state.forNode(node.child1()).m_type & ~(PredictNumber | PredictBoolean)))
m_jit.move(op1GPR, resultGPR);
else {
MacroAssembler::JumpList alreadyPrimitive;
