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.
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@127503 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
index 1f7aa2e7..9b951a1 100644
--- a/Source/JavaScriptCore/ChangeLog
+++ b/Source/JavaScriptCore/ChangeLog
@@ -1,3 +1,26 @@
+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.
+
+ 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 Zoltan Horvath <zoltan@webkit.org>
Extend the coverage of the Custom Allocation Framework in WTF and in JavaScriptCore
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..894d11b 100644
--- a/Source/JavaScriptCore/dfg/DFGOperations.cpp
+++ b/Source/JavaScriptCore/dfg/DFGOperations.cpp
@@ -409,6 +409,15 @@
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);
+
+ // 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;
