DFG GetByVal for JSArrays shouldn't OSR exit every time that the index is out of bound
https://bugs.webkit.org/show_bug.cgi?id=95717
Reviewed by Oliver Hunt.
Rolling back in after fixing the negative index case.
Make GetByVal for JSArrayOutOfBounds do meaningful things. The profiling was already
there so we should just use it!
* bytecode/DFGExitProfile.h:
(JSC::DFG::exitKindToString):
* dfg/DFGAbstractState.cpp:
(JSC::DFG::AbstractState::execute):
* dfg/DFGOperations.cpp:
* dfg/DFGOperations.h:
* dfg/DFGSpeculativeJIT.h:
(JSC::DFG::SpeculativeJIT::callOperation):
* dfg/DFGSpeculativeJIT32_64.cpp:
(JSC::DFG::SpeculativeJIT::compile):
* dfg/DFGSpeculativeJIT64.cpp:
(JSC::DFG::SpeculativeJIT::compile):
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@127536 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
index 2b4e333..f0c12d1 100644
--- a/Source/JavaScriptCore/ChangeLog
+++ b/Source/JavaScriptCore/ChangeLog
@@ -1,3 +1,28 @@
+2012-09-04 Filip Pizlo <fpizlo@apple.com>
+
+ DFG GetByVal for JSArrays shouldn't OSR exit every time that the index is out of bound
+ https://bugs.webkit.org/show_bug.cgi?id=95717
+
+ Reviewed by Oliver Hunt.
+
+ Rolling back in after fixing the negative index case.
+
+ Make GetByVal for JSArrayOutOfBounds do meaningful things. The profiling was already
+ there so we should just use it!
+
+ * bytecode/DFGExitProfile.h:
+ (JSC::DFG::exitKindToString):
+ * dfg/DFGAbstractState.cpp:
+ (JSC::DFG::AbstractState::execute):
+ * dfg/DFGOperations.cpp:
+ * dfg/DFGOperations.h:
+ * dfg/DFGSpeculativeJIT.h:
+ (JSC::DFG::SpeculativeJIT::callOperation):
+ * dfg/DFGSpeculativeJIT32_64.cpp:
+ (JSC::DFG::SpeculativeJIT::compile):
+ * dfg/DFGSpeculativeJIT64.cpp:
+ (JSC::DFG::SpeculativeJIT::compile):
+
2012-09-04 Sheriff Bot <webkit.review.bot@gmail.com>
Unreviewed, rolling out r127503.
diff --git a/Source/JavaScriptCore/bytecode/DFGExitProfile.h b/Source/JavaScriptCore/bytecode/DFGExitProfile.h
index e0aeba2..45947c8 100644
--- a/Source/JavaScriptCore/bytecode/DFGExitProfile.h
+++ b/Source/JavaScriptCore/bytecode/DFGExitProfile.h
@@ -58,6 +58,8 @@
return "Overflow";
case NegativeZero:
return "NegativeZero";
+ case OutOfBounds:
+ return "OutOfBounds";
case InadequateCoverage:
return "InadequateCoverage";
case ArgumentsEscaped:
diff --git a/Source/JavaScriptCore/dfg/DFGAbstractState.cpp b/Source/JavaScriptCore/dfg/DFGAbstractState.cpp
index e63bed3..5f79f66 100644
--- a/Source/JavaScriptCore/dfg/DFGAbstractState.cpp
+++ b/Source/JavaScriptCore/dfg/DFGAbstractState.cpp
@@ -853,12 +853,14 @@
forNode(nodeIndex).makeTop();
break;
case Array::JSArray:
- case Array::JSArrayOutOfBounds:
- // FIXME: We should have more conservative handling of the out-of-bounds
- // case.
forNode(node.child2()).filter(SpecInt32);
forNode(nodeIndex).makeTop();
break;
+ case Array::JSArrayOutOfBounds:
+ forNode(node.child2()).filter(SpecInt32);
+ clobberWorld(node.codeOrigin, indexInBlock);
+ forNode(nodeIndex).makeTop();
+ break;
case Array::Int8Array:
forNode(node.child2()).filter(SpecInt32);
forNode(nodeIndex).set(SpecInt32);
diff --git a/Source/JavaScriptCore/dfg/DFGOperations.cpp b/Source/JavaScriptCore/dfg/DFGOperations.cpp
index 093418a..72fe983 100644
--- a/Source/JavaScriptCore/dfg/DFGOperations.cpp
+++ b/Source/JavaScriptCore/dfg/DFGOperations.cpp
@@ -409,6 +409,20 @@
return JSValue::encode(JSValue(base).get(exec, ident));
}
+EncodedJSValue DFG_OPERATION operationGetByValArrayInt(ExecState* exec, JSArray* base, int32_t index)
+{
+ JSGlobalData* globalData = &exec->globalData();
+ NativeCallFrameTracer tracer(globalData, exec);
+
+ if (index < 0) {
+ // Go the slowest way possible becase negative indices don't use indexed storage.
+ return JSValue::encode(JSValue(base).get(exec, Identifier::from(exec, index)));
+ }
+
+ // Use this since we know that the value is out of bounds.
+ return JSValue::encode(JSValue(base).get(exec, index));
+}
+
EncodedJSValue DFG_OPERATION operationGetById(ExecState* exec, EncodedJSValue base, Identifier* propertyName)
{
JSGlobalData* globalData = &exec->globalData();
diff --git a/Source/JavaScriptCore/dfg/DFGOperations.h b/Source/JavaScriptCore/dfg/DFGOperations.h
index 455c2bc..82babe8 100644
--- a/Source/JavaScriptCore/dfg/DFGOperations.h
+++ b/Source/JavaScriptCore/dfg/DFGOperations.h
@@ -62,6 +62,7 @@
*/
typedef EncodedJSValue DFG_OPERATION (*J_DFGOperation_E)(ExecState*);
typedef EncodedJSValue DFG_OPERATION (*J_DFGOperation_EA)(ExecState*, JSArray*);
+typedef EncodedJSValue DFG_OPERATION (*J_DFGOperation_EAZ)(ExecState*, JSArray*, int32_t);
typedef EncodedJSValue DFG_OPERATION (*J_DFGOperation_ECC)(ExecState*, JSCell*, JSCell*);
typedef EncodedJSValue DFG_OPERATION (*J_DFGOperation_ECI)(ExecState*, JSCell*, Identifier*);
typedef EncodedJSValue DFG_OPERATION (*J_DFGOperation_ECJ)(ExecState*, JSCell*, EncodedJSValue);
@@ -116,6 +117,7 @@
EncodedJSValue DFG_OPERATION operationValueAddNotNumber(ExecState*, EncodedJSValue encodedOp1, EncodedJSValue encodedOp2) WTF_INTERNAL;
EncodedJSValue DFG_OPERATION operationGetByVal(ExecState*, EncodedJSValue encodedBase, EncodedJSValue encodedProperty) WTF_INTERNAL;
EncodedJSValue DFG_OPERATION operationGetByValCell(ExecState*, JSCell*, EncodedJSValue encodedProperty) WTF_INTERNAL;
+EncodedJSValue DFG_OPERATION operationGetByValArrayInt(ExecState*, JSArray*, int32_t) WTF_INTERNAL;
EncodedJSValue DFG_OPERATION operationGetById(ExecState*, EncodedJSValue, Identifier*) WTF_INTERNAL;
EncodedJSValue DFG_OPERATION operationGetByIdBuildList(ExecState*, EncodedJSValue, Identifier*) WTF_INTERNAL;
EncodedJSValue DFG_OPERATION operationGetByIdProtoBuildList(ExecState*, EncodedJSValue, Identifier*) WTF_INTERNAL;
diff --git a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h
index 348540b..c5e49f7 100644
--- a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h
+++ b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h
@@ -1198,6 +1198,11 @@
m_jit.setupArgumentsWithExecState(arg1);
return appendCallWithExceptionCheckSetResult(operation, result);
}
+ JITCompiler::Call callOperation(J_DFGOperation_EAZ operation, GPRReg result, GPRReg arg1, GPRReg arg2)
+ {
+ m_jit.setupArgumentsWithExecState(arg1, arg2);
+ return appendCallWithExceptionCheckSetResult(operation, result);
+ }
JITCompiler::Call callOperation(J_DFGOperation_ESt operation, GPRReg result, Structure* structure)
{
m_jit.setupArgumentsWithExecState(TrustedImmPtr(structure));
@@ -1481,6 +1486,11 @@
m_jit.setupArgumentsWithExecState(arg1);
return appendCallWithExceptionCheckSetResult(operation, resultPayload, resultTag);
}
+ JITCompiler::Call callOperation(J_DFGOperation_EAZ operation, GPRReg resultTag, GPRReg resultPayload, GPRReg arg1, GPRReg arg2)
+ {
+ m_jit.setupArgumentsWithExecState(arg1, arg2);
+ return appendCallWithExceptionCheckSetResult(operation, resultPayload, resultTag);
+ }
JITCompiler::Call callOperation(J_DFGOperation_ESt operation, GPRReg resultTag, GPRReg resultPayload, Structure* structure)
{
m_jit.setupArgumentsWithExecState(TrustedImmPtr(structure));
diff --git a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp
index ee76fc9..34b8dae 100644
--- a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp
+++ b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp
@@ -2554,8 +2554,7 @@
jsValueResult(resultTag.gpr(), resultPayload.gpr(), m_compileIndex);
break;
}
- case Array::JSArray:
- case Array::JSArrayOutOfBounds: {
+ case Array::JSArray: {
SpeculateStrictInt32Operand property(this, node.child2());
StorageOperand storage(this, node.child3());
GPRReg propertyReg = property.gpr();
@@ -2570,19 +2569,59 @@
SpeculateCellOperand base(this, node.child1());
GPRReg baseReg = base.gpr();
// We've already speculated that it's some kind of array, at this point.
- speculationCheck(Uncountable, JSValueRegs(), NoNode, m_jit.branch32(MacroAssembler::AboveOrEqual, propertyReg, MacroAssembler::Address(baseReg, JSArray::vectorLengthOffset())));
+ speculationCheck(OutOfBounds, JSValueRegs(), NoNode, m_jit.branch32(MacroAssembler::AboveOrEqual, propertyReg, MacroAssembler::Address(baseReg, JSArray::vectorLengthOffset())));
}
GPRTemporary resultTag(this);
GPRTemporary resultPayload(this);
m_jit.load32(MacroAssembler::BaseIndex(storageReg, propertyReg, MacroAssembler::TimesEight, OBJECT_OFFSETOF(ArrayStorage, m_vector[0]) + OBJECT_OFFSETOF(JSValue, u.asBits.tag)), resultTag.gpr());
- speculationCheck(Uncountable, JSValueRegs(), NoNode, m_jit.branch32(MacroAssembler::Equal, resultTag.gpr(), TrustedImm32(JSValue::EmptyValueTag)));
+ speculationCheck(OutOfBounds, JSValueRegs(), NoNode, m_jit.branch32(MacroAssembler::Equal, resultTag.gpr(), TrustedImm32(JSValue::EmptyValueTag)));
m_jit.load32(MacroAssembler::BaseIndex(storageReg, propertyReg, MacroAssembler::TimesEight, OBJECT_OFFSETOF(ArrayStorage, m_vector[0]) + OBJECT_OFFSETOF(JSValue, u.asBits.payload)), resultPayload.gpr());
-
+
jsValueResult(resultTag.gpr(), resultPayload.gpr(), m_compileIndex);
break;
}
+ case Array::JSArrayOutOfBounds: {
+ SpeculateCellOperand base(this, node.child1());
+ SpeculateStrictInt32Operand property(this, node.child2());
+ StorageOperand storage(this, node.child3());
+ GPRReg propertyReg = property.gpr();
+ GPRReg storageReg = storage.gpr();
+
+ if (!m_compileOkay)
+ return;
+
+ GPRTemporary resultTag(this);
+ GPRTemporary resultPayload(this);
+ GPRReg resultTagReg = resultTag.gpr();
+ GPRReg resultPayloadReg = resultPayload.gpr();
+
+ // Check that base is an array, and that property is contained within m_vector (< m_vectorLength).
+ // If we have predicted the base to be type array, we can skip the check.
+ GPRReg baseReg = base.gpr();
+ // We've already speculated that it's some kind of array, at this point.
+ JITCompiler::Jump outOfBounds = m_jit.branch32(
+ MacroAssembler::AboveOrEqual, propertyReg,
+ MacroAssembler::Address(baseReg, JSArray::vectorLengthOffset()));
+
+ m_jit.load32(MacroAssembler::BaseIndex(storageReg, propertyReg, MacroAssembler::TimesEight, OBJECT_OFFSETOF(ArrayStorage, m_vector[0]) + OBJECT_OFFSETOF(JSValue, u.asBits.tag)), resultTagReg);
+ JITCompiler::Jump hole = m_jit.branch32(
+ MacroAssembler::Equal, resultTag.gpr(), TrustedImm32(JSValue::EmptyValueTag));
+ m_jit.load32(MacroAssembler::BaseIndex(storageReg, propertyReg, MacroAssembler::TimesEight, OBJECT_OFFSETOF(ArrayStorage, m_vector[0]) + OBJECT_OFFSETOF(JSValue, u.asBits.payload)), resultPayloadReg);
+
+ JITCompiler::JumpList slowCases;
+ slowCases.append(outOfBounds);
+ slowCases.append(hole);
+ addSlowPathGenerator(
+ slowPathCall(
+ slowCases, this, operationGetByValArrayInt,
+ JSValueRegs(resultTagReg, resultPayloadReg),
+ baseReg, propertyReg));
+
+ jsValueResult(resultTagReg, resultPayloadReg, m_compileIndex);
+ break;
+ }
case Array::String:
compileGetByValOnString(node);
break;
diff --git a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp
index bf1bd98..a1ac899 100644
--- a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp
+++ b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp
@@ -2600,11 +2600,24 @@
// We will have already speculated that the base is some kind of array,
// at this point.
- speculationCheck(Uncountable, JSValueRegs(), NoNode, m_jit.branch32(MacroAssembler::AboveOrEqual, propertyReg, MacroAssembler::Address(baseReg, JSArray::vectorLengthOffset())));
+ MacroAssembler::Jump outOfBounds = m_jit.branch32(MacroAssembler::AboveOrEqual, propertyReg, MacroAssembler::Address(baseReg, JSArray::vectorLengthOffset()));
+ if (node.arrayMode() == Array::JSArray)
+ speculationCheck(OutOfBounds, JSValueRegs(), NoNode, outOfBounds);
GPRTemporary result(this);
m_jit.loadPtr(MacroAssembler::BaseIndex(storageReg, propertyReg, MacroAssembler::ScalePtr, OBJECT_OFFSETOF(ArrayStorage, m_vector[0])), result.gpr());
- speculationCheck(Uncountable, JSValueRegs(), NoNode, m_jit.branchTestPtr(MacroAssembler::Zero, result.gpr()));
+ MacroAssembler::Jump hole = m_jit.branchTestPtr(MacroAssembler::Zero, result.gpr());
+ if (node.arrayMode() == Array::JSArray)
+ speculationCheck(OutOfBounds, JSValueRegs(), NoNode, hole);
+ else {
+ MacroAssembler::JumpList slowCases;
+ slowCases.append(outOfBounds);
+ slowCases.append(hole);
+ addSlowPathGenerator(
+ slowPathCall(
+ slowCases, this, operationGetByValArrayInt,
+ result.gpr(), baseReg, propertyReg));
+ }
jsValueResult(result.gpr(), m_compileIndex);
break;
