- Revision
- 100829
- Author
- [email protected]
- Date
- 2011-11-18 17:00:34 -0800 (Fri, 18 Nov 2011)
Log Message
DFG JIT fails speculation on InstanceOf if the base is not an object
https://bugs.webkit.org/show_bug.cgi?id=72709
Reviewed by Geoff Garen.
InstanceOf already leverages the fact that we only allow the default
hasInstance implementation. So, if the base is predicted to possibly
be not an object and the CFA has not yet proven otherwise, InstanceOf
will abstain from speculating cell and instead return false if the
base is not a cell.
This appears to be a 1% speed-up on V8 on the V8 harness. 3-4% or so
speed-up in earley-boyer. Neutral according to bencher on SunSpider,
V8, and Kraken. In 32-bit, it's a 0.5% win on SunSpider and a 1.9%
win on V8 even on my harness, due to a 12.5% win on earley-boyer.
I also took this opportunity to make the code for InstanceOf common
between the two JITs. This was partially successful, in that the
"common code" has a bunch of #if's, but overall it seems like a code
size reduction.
* dfg/DFGAbstractState.cpp:
(JSC::DFG::AbstractState::execute):
* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::compileInstanceOfForObject):
(JSC::DFG::SpeculativeJIT::compileInstanceOf):
* dfg/DFGSpeculativeJIT.h:
* dfg/DFGSpeculativeJIT32_64.cpp:
(JSC::DFG::SpeculativeJIT::compile):
* dfg/DFGSpeculativeJIT64.cpp:
(JSC::DFG::SpeculativeJIT::compile):
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (100828 => 100829)
--- trunk/Source/_javascript_Core/ChangeLog 2011-11-19 00:46:42 UTC (rev 100828)
+++ trunk/Source/_javascript_Core/ChangeLog 2011-11-19 01:00:34 UTC (rev 100829)
@@ -1,3 +1,37 @@
+2011-11-18 Filip Pizlo <[email protected]>
+
+ DFG JIT fails speculation on InstanceOf if the base is not an object
+ https://bugs.webkit.org/show_bug.cgi?id=72709
+
+ Reviewed by Geoff Garen.
+
+ InstanceOf already leverages the fact that we only allow the default
+ hasInstance implementation. So, if the base is predicted to possibly
+ be not an object and the CFA has not yet proven otherwise, InstanceOf
+ will abstain from speculating cell and instead return false if the
+ base is not a cell.
+
+ This appears to be a 1% speed-up on V8 on the V8 harness. 3-4% or so
+ speed-up in earley-boyer. Neutral according to bencher on SunSpider,
+ V8, and Kraken. In 32-bit, it's a 0.5% win on SunSpider and a 1.9%
+ win on V8 even on my harness, due to a 12.5% win on earley-boyer.
+
+ I also took this opportunity to make the code for InstanceOf common
+ between the two JITs. This was partially successful, in that the
+ "common code" has a bunch of #if's, but overall it seems like a code
+ size reduction.
+
+ * dfg/DFGAbstractState.cpp:
+ (JSC::DFG::AbstractState::execute):
+ * dfg/DFGSpeculativeJIT.cpp:
+ (JSC::DFG::SpeculativeJIT::compileInstanceOfForObject):
+ (JSC::DFG::SpeculativeJIT::compileInstanceOf):
+ * dfg/DFGSpeculativeJIT.h:
+ * dfg/DFGSpeculativeJIT32_64.cpp:
+ (JSC::DFG::SpeculativeJIT::compile):
+ * dfg/DFGSpeculativeJIT64.cpp:
+ (JSC::DFG::SpeculativeJIT::compile):
+
2011-11-18 Mark Hahnenberg <[email protected]>
Forgot to completely de-virtualize isDynamicScope
Modified: trunk/Source/_javascript_Core/dfg/DFGAbstractState.cpp (100828 => 100829)
--- trunk/Source/_javascript_Core/dfg/DFGAbstractState.cpp 2011-11-19 00:46:42 UTC (rev 100828)
+++ trunk/Source/_javascript_Core/dfg/DFGAbstractState.cpp 2011-11-19 01:00:34 UTC (rev 100829)
@@ -627,8 +627,10 @@
case InstanceOf:
// Again, sadly, we don't propagate the fact that we've done InstanceOf
- forNode(node.child1()).filter(PredictCell);
- forNode(node.child2()).filter(PredictCell);
+ if (!(m_graph[node.child1()].prediction() & ~PredictCell) && !(forNode(node.child1()).m_type & ~PredictCell))
+ forNode(node.child1()).filter(PredictCell);
+ forNode(node.child3()).filter(PredictCell);
+ forNode(nodeIndex).set(PredictBoolean);
break;
case Phi:
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp (100828 => 100829)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2011-11-19 00:46:42 UTC (rev 100828)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2011-11-19 01:00:34 UTC (rev 100829)
@@ -1473,6 +1473,106 @@
integerResult(storageReg, m_compileIndex);
}
+void SpeculativeJIT::compileInstanceOfForObject(Node&, GPRReg valueReg, GPRReg prototypeReg, GPRReg scratchReg)
+{
+ // Check that prototype is an object.
+ m_jit.loadPtr(MacroAssembler::Address(prototypeReg, JSCell::structureOffset()), scratchReg);
+ speculationCheck(JSValueRegs(), NoNode, m_jit.branchIfNotObject(scratchReg));
+
+ // Initialize scratchReg with the value being checked.
+ m_jit.move(valueReg, scratchReg);
+
+ // Walk up the prototype chain of the value (in scratchReg), comparing to prototypeReg.
+ MacroAssembler::Label loop(&m_jit);
+ m_jit.loadPtr(MacroAssembler::Address(scratchReg, JSCell::structureOffset()), scratchReg);
+#if USE(JSVALUE64)
+ m_jit.loadPtr(MacroAssembler::Address(scratchReg, Structure::prototypeOffset()), scratchReg);
+#else
+ m_jit.load32(MacroAssembler::Address(scratchReg, Structure::prototypeOffset() + OBJECT_OFFSETOF(JSValue, u.asBits.payload)), scratchReg);
+#endif
+ MacroAssembler::Jump isInstance = m_jit.branchPtr(MacroAssembler::Equal, scratchReg, prototypeReg);
+#if USE(JSVALUE64)
+ m_jit.branchTestPtr(MacroAssembler::Zero, scratchReg, GPRInfo::tagMaskRegister).linkTo(loop, &m_jit);
+#else
+ m_jit.branchTest32(MacroAssembler::NonZero, scratchReg).linkTo(loop, &m_jit);
+#endif
+
+ // No match - result is false.
+#if USE(JSVALUE64)
+ m_jit.move(MacroAssembler::TrustedImmPtr(JSValue::encode(jsBoolean(false))), scratchReg);
+#else
+ m_jit.move(MacroAssembler::TrustedImm32(0), scratchReg);
+#endif
+ MacroAssembler::Jump putResult = m_jit.jump();
+
+ isInstance.link(&m_jit);
+#if USE(JSVALUE64)
+ m_jit.move(MacroAssembler::TrustedImmPtr(JSValue::encode(jsBoolean(true))), scratchReg);
+#else
+ m_jit.move(MacroAssembler::TrustedImm32(1), scratchReg);
+#endif
+
+ putResult.link(&m_jit);
+}
+
+void SpeculativeJIT::compileInstanceOf(Node& node)
+{
+ if (!!(at(node.child1()).prediction() & ~PredictCell) && !!(m_state.forNode(node.child1()).m_type & ~PredictCell)) {
+ // It might not be a cell. Speculate less aggressively.
+
+ JSValueOperand value(this, node.child1());
+ SpeculateCellOperand prototype(this, node.child3());
+ GPRTemporary scratch(this);
+
+ GPRReg prototypeReg = prototype.gpr();
+ GPRReg scratchReg = scratch.gpr();
+
+#if USE(JSVALUE64)
+ GPRReg valueReg = value.gpr();
+ MacroAssembler::Jump isCell = m_jit.branchTestPtr(MacroAssembler::Zero, valueReg, GPRInfo::tagMaskRegister);
+ m_jit.move(MacroAssembler::TrustedImmPtr(JSValue::encode(jsBoolean(false))), scratchReg);
+#else
+ GPRReg valueTagReg = value.tagGPR();
+ GPRReg valueReg = value.payloadGPR();
+ MacroAssembler::Jump isCell = m_jit.branch32(MacroAssembler::Equal, valueTagReg, TrustedImm32(JSValue::CellTag));
+ m_jit.move(MacroAssembler::TrustedImm32(0), scratchReg);
+#endif
+
+ MacroAssembler::Jump done = m_jit.jump();
+
+ isCell.link(&m_jit);
+
+ compileInstanceOfForObject(node, valueReg, prototypeReg, scratchReg);
+
+ done.link(&m_jit);
+
+#if USE(JSVALUE64)
+ jsValueResult(scratchReg, m_compileIndex, DataFormatJSBoolean);
+#else
+ booleanResult(scratchReg, m_compileIndex);
+#endif
+ return;
+ }
+
+ SpeculateCellOperand value(this, node.child1());
+ // Base unused since we speculate default InstanceOf behaviour in CheckHasInstance.
+ SpeculateCellOperand prototype(this, node.child3());
+
+ GPRTemporary scratch(this);
+
+ GPRReg valueReg = value.gpr();
+ GPRReg prototypeReg = prototype.gpr();
+ GPRReg scratchReg = scratch.gpr();
+
+ compileInstanceOfForObject(node, valueReg, prototypeReg, scratchReg);
+
+#if USE(JSVALUE64)
+ jsValueResult(scratchReg, m_compileIndex, DataFormatJSBoolean);
+#else
+ booleanResult(scratchReg, m_compileIndex);
+#endif
+}
+
} } // namespace JSC::DFG
#endif
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.h (100828 => 100829)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.h 2011-11-19 00:46:42 UTC (rev 100828)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.h 2011-11-19 01:00:34 UTC (rev 100829)
@@ -889,6 +889,9 @@
void nonSpeculativeNonPeepholeStrictEq(Node&, bool invert = false);
bool nonSpeculativeStrictEq(Node&, bool invert = false);
+ void compileInstanceOfForObject(Node&, GPRReg valueReg, GPRReg prototypeReg, GPRReg scratchAndResultReg);
+ void compileInstanceOf(Node&);
+
MacroAssembler::Address addressOfCallData(int idx)
{
return MacroAssembler::Address(GPRInfo::callFrameRegister, (m_jit.codeBlock()->m_numCalleeRegisters + idx) * static_cast<int>(sizeof(Register)));
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp (100828 => 100829)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp 2011-11-19 00:46:42 UTC (rev 100828)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp 2011-11-19 01:00:34 UTC (rev 100829)
@@ -3723,39 +3723,7 @@
}
case InstanceOf: {
- SpeculateCellOperand value(this, node.child1());
- // Base unused since we speculate default InstanceOf behaviour in CheckHasInstance.
- SpeculateCellOperand prototype(this, node.child3());
-
- GPRTemporary scratch(this);
-
- GPRReg valueReg = value.gpr();
- GPRReg prototypeReg = prototype.gpr();
- GPRReg scratchReg = scratch.gpr();
-
- // Check that prototype is an object.
- m_jit.loadPtr(MacroAssembler::Address(prototypeReg, JSCell::structureOffset()), scratchReg);
- speculationCheck(JSValueRegs(), NoNode, m_jit.branch8(MacroAssembler::NotEqual, MacroAssembler::Address(scratchReg, Structure::typeInfoTypeOffset()), MacroAssembler::TrustedImm32(ObjectType)));
-
- // Initialize scratchReg with the value being checked.
- m_jit.move(valueReg, scratchReg);
-
- // Walk up the prototype chain of the value (in scratchReg), comparing to prototypeReg.
- MacroAssembler::Label loop(&m_jit);
- m_jit.loadPtr(MacroAssembler::Address(scratchReg, JSCell::structureOffset()), scratchReg);
- m_jit.load32(MacroAssembler::Address(scratchReg, Structure::prototypeOffset() + OBJECT_OFFSETOF(JSValue, u.asBits.payload)), scratchReg);
- MacroAssembler::Jump isInstance = m_jit.branch32(MacroAssembler::Equal, scratchReg, prototypeReg);
- m_jit.branchTest32(MacroAssembler::NonZero, scratchReg).linkTo(loop, &m_jit);
-
- // No match - result is false.
- m_jit.move(MacroAssembler::TrustedImm32(0), scratchReg);
- MacroAssembler::Jump putResult = m_jit.jump();
-
- isInstance.link(&m_jit);
- m_jit.move(MacroAssembler::TrustedImm32(1), scratchReg);
-
- putResult.link(&m_jit);
- booleanResult(scratchReg, m_compileIndex);
+ compileInstanceOf(node);
break;
}
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp (100828 => 100829)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp 2011-11-19 00:46:42 UTC (rev 100828)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp 2011-11-19 01:00:34 UTC (rev 100829)
@@ -3632,39 +3632,7 @@
}
case InstanceOf: {
- SpeculateCellOperand value(this, node.child1());
- // Base unused since we speculate default InstanceOf behaviour in CheckHasInstance.
- SpeculateCellOperand prototype(this, node.child3());
-
- GPRTemporary scratch(this);
-
- GPRReg valueReg = value.gpr();
- GPRReg prototypeReg = prototype.gpr();
- GPRReg scratchReg = scratch.gpr();
-
- // Check that prototype is an object.
- m_jit.loadPtr(MacroAssembler::Address(prototypeReg, JSCell::structureOffset()), scratchReg);
- speculationCheck(JSValueRegs(), NoNode, m_jit.branchIfNotObject(scratchReg));
-
- // Initialize scratchReg with the value being checked.
- m_jit.move(valueReg, scratchReg);
-
- // Walk up the prototype chain of the value (in scratchReg), comparing to prototypeReg.
- MacroAssembler::Label loop(&m_jit);
- m_jit.loadPtr(MacroAssembler::Address(scratchReg, JSCell::structureOffset()), scratchReg);
- m_jit.loadPtr(MacroAssembler::Address(scratchReg, Structure::prototypeOffset()), scratchReg);
- MacroAssembler::Jump isInstance = m_jit.branchPtr(MacroAssembler::Equal, scratchReg, prototypeReg);
- m_jit.branchTestPtr(MacroAssembler::Zero, scratchReg, GPRInfo::tagMaskRegister).linkTo(loop, &m_jit);
-
- // No match - result is false.
- m_jit.move(MacroAssembler::TrustedImmPtr(JSValue::encode(jsBoolean(false))), scratchReg);
- MacroAssembler::Jump putResult = m_jit.jump();
-
- isInstance.link(&m_jit);
- m_jit.move(MacroAssembler::TrustedImmPtr(JSValue::encode(jsBoolean(true))), scratchReg);
-
- putResult.link(&m_jit);
- jsValueResult(scratchReg, m_compileIndex, DataFormatJSBoolean);
+ compileInstanceOf(node);
break;
}
