Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (197491 => 197492)
--- trunk/Source/_javascript_Core/ChangeLog 2016-03-03 05:29:16 UTC (rev 197491)
+++ trunk/Source/_javascript_Core/ChangeLog 2016-03-03 05:58:59 UTC (rev 197492)
@@ -1,3 +1,43 @@
+2016-03-02 Filip Pizlo <[email protected]>
+
+ RegExpExec/RegExpTest should not unconditionally speculate cell
+ https://bugs.webkit.org/show_bug.cgi?id=154901
+
+ Reviewed by Benjamin Poulain.
+
+ This is a three part change. It all started with a simple goal: end the rage-recompiles in
+ Octane/regexp by enabling the DFG and FTL to do untyped RegExpExec/RegExpTest. This keeps us
+ in the optimized code when you do a regexp match on a number, for example.
+
+ While implementing this, I realized that DFGOperations.cpp was bad at exception checking. When
+ it did check for exceptions, it used exec->hadException() instead of vm.exception(). So I
+ fixed that. I also made sure that the regexp operations checked for exception after doing
+ toString().
+
+ Unfortunately, the introduction of untyped RegExpExec/RegExpTest caused a regression on
+ Octane/regexp. This was because we were simultaneously scheduling replacement and OSR compiles
+ of some large functions with the FTL JIT. The OSR compiles were not useful. This was a
+ regression from the previous changes to make OSR compiles happen sooner. The problem is that
+ this change also removed the throttling of OSR compiles even in those cases where we suspect
+ that replacement is more likely. This patch reintroduces that throttling, but only in the
+ replacement path.
+
+ This change ends up being neutral overall.
+
+ * dfg/DFGFixupPhase.cpp:
+ (JSC::DFG::FixupPhase::fixupNode):
+ * dfg/DFGOperations.cpp:
+ * dfg/DFGOperations.h:
+ * dfg/DFGSpeculativeJIT32_64.cpp:
+ (JSC::DFG::SpeculativeJIT::compile):
+ * dfg/DFGSpeculativeJIT64.cpp:
+ (JSC::DFG::SpeculativeJIT::compile):
+ * ftl/FTLLowerDFGToB3.cpp:
+ (JSC::FTL::DFG::LowerDFGToB3::compileRegExpExec):
+ (JSC::FTL::DFG::LowerDFGToB3::compileRegExpTest):
+ (JSC::FTL::DFG::LowerDFGToB3::compileNewRegexp):
+ * tests/stress/regexp-exec-effect-after-exception.js: Added.
+
2016-03-02 Benjamin Poulain <[email protected]>
[JSC] JSCell_freeListNext and JSCell_structureID are considered not overlapping
Modified: trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp (197491 => 197492)
--- trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp 2016-03-03 05:29:16 UTC (rev 197491)
+++ trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp 2016-03-03 05:58:59 UTC (rev 197492)
@@ -879,8 +879,14 @@
case RegExpExec:
case RegExpTest: {
- fixEdge<CellUse>(node->child1());
- fixEdge<CellUse>(node->child2());
+ // FIXME: These should probably speculate something stronger than cell.
+ // https://bugs.webkit.org/show_bug.cgi?id=154900
+ if (node->child1()->shouldSpeculateCell()
+ && node->child2()->shouldSpeculateCell()) {
+ fixEdge<CellUse>(node->child1());
+ fixEdge<CellUse>(node->child2());
+ break;
+ }
break;
}
Modified: trunk/Source/_javascript_Core/dfg/DFGOperations.cpp (197491 => 197492)
--- trunk/Source/_javascript_Core/dfg/DFGOperations.cpp 2016-03-03 05:29:16 UTC (rev 197491)
+++ trunk/Source/_javascript_Core/dfg/DFGOperations.cpp 2016-03-03 05:58:59 UTC (rev 197492)
@@ -361,10 +361,10 @@
}
baseValue.requireObjectCoercible(exec);
- if (exec->hadException())
+ if (vm.exception())
return JSValue::encode(jsUndefined());
auto propertyName = property.toPropertyKey(exec);
- if (exec->hadException())
+ if (vm.exception())
return JSValue::encode(jsUndefined());
return JSValue::encode(baseValue.get(exec, propertyName));
}
@@ -394,7 +394,7 @@
}
auto propertyName = property.toPropertyKey(exec);
- if (exec->hadException())
+ if (vm.exception())
return JSValue::encode(jsUndefined());
return JSValue::encode(JSValue(base).get(exec, propertyName));
}
@@ -622,11 +622,34 @@
if (!base->inherits(RegExpObject::info()))
return throwVMTypeError(exec);
- ASSERT(argument->isString() || argument->isObject());
- JSString* input = argument->isString() ? asString(argument) : asObject(argument)->toString(exec);
+ JSString* input;
+ if (argument->isString())
+ input = asString(argument);
+ else {
+ input = JSValue(argument).toStringOrNull(exec);
+ if (!input)
+ return JSValue::encode(jsUndefined());
+ }
return JSValue::encode(asRegExpObject(base)->exec(exec, input));
}
+EncodedJSValue JIT_OPERATION operationRegExpExecGeneric(ExecState* exec, EncodedJSValue encodedBase, EncodedJSValue encodedArgument)
+{
+ VM& vm = exec->vm();
+ NativeCallFrameTracer tracer(&vm, exec);
+
+ JSValue base = JSValue::decode(encodedBase);
+ JSValue argument = JSValue::decode(encodedArgument);
+
+ if (!base.inherits(RegExpObject::info()))
+ return throwVMTypeError(exec);
+
+ JSString* input = argument.toStringOrNull(exec);
+ if (!input)
+ return JSValue::encode(jsUndefined());
+ return JSValue::encode(asRegExpObject(base)->exec(exec, input));
+}
+
size_t JIT_OPERATION operationRegExpTest(ExecState* exec, JSCell* base, JSCell* argument)
{
VM& vm = exec->vm();
@@ -637,11 +660,36 @@
return false;
}
- ASSERT(argument->isString() || argument->isObject());
- JSString* input = argument->isString() ? asString(argument) : asObject(argument)->toString(exec);
+ JSString* input;
+ if (argument->isString())
+ input = asString(argument);
+ else {
+ input = JSValue(argument).toStringOrNull(exec);
+ if (!input)
+ return false;
+ }
return asRegExpObject(base)->test(exec, input);
}
+size_t JIT_OPERATION operationRegExpTestGeneric(ExecState* exec, EncodedJSValue encodedBase, EncodedJSValue encodedArgument)
+{
+ VM& vm = exec->vm();
+ NativeCallFrameTracer tracer(&vm, exec);
+
+ JSValue base = JSValue::decode(encodedBase);
+ JSValue argument = JSValue::decode(encodedArgument);
+
+ if (!base.inherits(RegExpObject::info())) {
+ throwTypeError(exec);
+ return false;
+ }
+
+ JSString* input = argument.toStringOrNull(exec);
+ if (!input)
+ return false;
+ return asRegExpObject(base)->test(exec, input);
+}
+
size_t JIT_OPERATION operationCompareStrictEqCell(ExecState* exec, EncodedJSValue encodedOp1, EncodedJSValue encodedOp2)
{
VM* vm = &exec->vm();
@@ -1182,9 +1230,9 @@
NativeCallFrameTracer tracer(&vm, exec);
JSString* str1 = JSValue::decode(a).toString(exec);
- ASSERT(!exec->hadException()); // Impossible, since we must have been given primitives.
+ ASSERT(!vm.exception()); // Impossible, since we must have been given primitives.
JSString* str2 = JSValue::decode(b).toString(exec);
- ASSERT(!exec->hadException());
+ ASSERT(!vm.exception());
if (sumOverflows<int32_t>(str1->length(), str2->length())) {
throwOutOfMemoryError(exec);
@@ -1200,11 +1248,11 @@
NativeCallFrameTracer tracer(&vm, exec);
JSString* str1 = JSValue::decode(a).toString(exec);
- ASSERT(!exec->hadException()); // Impossible, since we must have been given primitives.
+ ASSERT(!vm.exception()); // Impossible, since we must have been given primitives.
JSString* str2 = JSValue::decode(b).toString(exec);
- ASSERT(!exec->hadException());
+ ASSERT(!vm.exception());
JSString* str3 = JSValue::decode(c).toString(exec);
- ASSERT(!exec->hadException());
+ ASSERT(!vm.exception());
if (sumOverflows<int32_t>(str1->length(), str2->length(), str3->length())) {
throwOutOfMemoryError(exec);
@@ -1560,6 +1608,11 @@
if (!codeBlock->hasOptimizedReplacement())
return nullptr;
+
+ if (jitCode->osrEntryRetry < Options::ftlOSREntryRetryThreshold()) {
+ jitCode->osrEntryRetry++;
+ return nullptr;
+ }
}
// It's time to try to compile code for OSR entry.
Modified: trunk/Source/_javascript_Core/dfg/DFGOperations.h (197491 => 197492)
--- trunk/Source/_javascript_Core/dfg/DFGOperations.h 2016-03-03 05:29:16 UTC (rev 197491)
+++ trunk/Source/_javascript_Core/dfg/DFGOperations.h 2016-03-03 05:58:59 UTC (rev 197492)
@@ -102,8 +102,10 @@
EncodedJSValue JIT_OPERATION operationArrayPop(ExecState*, JSArray*) WTF_INTERNAL;
EncodedJSValue JIT_OPERATION operationArrayPopAndRecoverLength(ExecState*, JSArray*) WTF_INTERNAL;
EncodedJSValue JIT_OPERATION operationRegExpExec(ExecState*, JSCell*, JSCell*) WTF_INTERNAL;
+EncodedJSValue JIT_OPERATION operationRegExpExecGeneric(ExecState*, EncodedJSValue, EncodedJSValue) WTF_INTERNAL;
// These comparisons return a boolean within a size_t such that the value is zero extended to fill the register.
size_t JIT_OPERATION operationRegExpTest(ExecState*, JSCell*, JSCell*) WTF_INTERNAL;
+size_t JIT_OPERATION operationRegExpTestGeneric(ExecState*, EncodedJSValue, EncodedJSValue) WTF_INTERNAL;
size_t JIT_OPERATION operationCompareStrictEqCell(ExecState*, EncodedJSValue encodedOp1, EncodedJSValue encodedOp2) WTF_INTERNAL;
size_t JIT_OPERATION operationCompareStrictEq(ExecState*, EncodedJSValue encodedOp1, EncodedJSValue encodedOp2) WTF_INTERNAL;
JSCell* JIT_OPERATION operationCreateActivationDirect(ExecState*, Structure*, JSScope*, SymbolTable*, EncodedJSValue);
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp (197491 => 197492)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp 2016-03-03 05:29:16 UTC (rev 197491)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp 2016-03-03 05:58:59 UTC (rev 197492)
@@ -2836,15 +2836,34 @@
}
case RegExpExec: {
- SpeculateCellOperand base(this, node->child1());
- SpeculateCellOperand argument(this, node->child2());
- GPRReg baseGPR = base.gpr();
- GPRReg argumentGPR = argument.gpr();
+ if (node->child1().useKind() == CellUse
+ && node->child2().useKind() == CellUse) {
+ SpeculateCellOperand base(this, node->child1());
+ SpeculateCellOperand argument(this, node->child2());
+ GPRReg baseGPR = base.gpr();
+ GPRReg argumentGPR = argument.gpr();
+ flushRegisters();
+ GPRFlushedCallResult2 resultTag(this);
+ GPRFlushedCallResult resultPayload(this);
+ callOperation(operationRegExpExec, resultTag.gpr(), resultPayload.gpr(), baseGPR, argumentGPR);
+ m_jit.exceptionCheck();
+
+ jsValueResult(resultTag.gpr(), resultPayload.gpr(), node);
+ break;
+ }
+
+ JSValueOperand base(this, node->child1());
+ JSValueOperand argument(this, node->child2());
+ GPRReg baseTagGPR = base.tagGPR();
+ GPRReg basePayloadGPR = base.payloadGPR();
+ GPRReg argumentTagGPR = argument.tagGPR();
+ GPRReg argumentPayloadGPR = argument.payloadGPR();
+
flushRegisters();
GPRFlushedCallResult2 resultTag(this);
GPRFlushedCallResult resultPayload(this);
- callOperation(operationRegExpExec, resultTag.gpr(), resultPayload.gpr(), baseGPR, argumentGPR);
+ callOperation(operationRegExpExecGeneric, resultTag.gpr(), resultPayload.gpr(), baseTagGPR, basePayloadGPR, argumentTagGPR, argumentPayloadGPR);
m_jit.exceptionCheck();
jsValueResult(resultTag.gpr(), resultPayload.gpr(), node);
@@ -2852,17 +2871,34 @@
}
case RegExpTest: {
- SpeculateCellOperand base(this, node->child1());
- SpeculateCellOperand argument(this, node->child2());
- GPRReg baseGPR = base.gpr();
- GPRReg argumentGPR = argument.gpr();
+ if (node->child1().useKind() == CellUse
+ && node->child2().useKind() == CellUse) {
+ SpeculateCellOperand base(this, node->child1());
+ SpeculateCellOperand argument(this, node->child2());
+ GPRReg baseGPR = base.gpr();
+ GPRReg argumentGPR = argument.gpr();
+ flushRegisters();
+ GPRFlushedCallResult result(this);
+ callOperation(operationRegExpTest, result.gpr(), baseGPR, argumentGPR);
+ m_jit.exceptionCheck();
+
+ booleanResult(result.gpr(), node);
+ break;
+ }
+
+ JSValueOperand base(this, node->child1());
+ JSValueOperand argument(this, node->child2());
+ GPRReg baseTagGPR = base.tagGPR();
+ GPRReg basePayloadGPR = base.payloadGPR();
+ GPRReg argumentTagGPR = argument.tagGPR();
+ GPRReg argumentPayloadGPR = argument.payloadGPR();
+
flushRegisters();
GPRFlushedCallResult result(this);
- callOperation(operationRegExpTest, result.gpr(), baseGPR, argumentGPR);
+ callOperation(operationRegExpTestGeneric, result.gpr(), baseTagGPR, basePayloadGPR, argumentTagGPR, argumentPayloadGPR);
m_jit.exceptionCheck();
- // If we add a DataFormatBool, we should use it here.
booleanResult(result.gpr(), node);
break;
}
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp (197491 => 197492)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp 2016-03-03 05:29:16 UTC (rev 197491)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp 2016-03-03 05:58:59 UTC (rev 197492)
@@ -2961,14 +2961,30 @@
}
case RegExpExec: {
- SpeculateCellOperand base(this, node->child1());
- SpeculateCellOperand argument(this, node->child2());
+ if (node->child1().useKind() == CellUse
+ && node->child2().useKind() == CellUse) {
+ SpeculateCellOperand base(this, node->child1());
+ SpeculateCellOperand argument(this, node->child2());
+ GPRReg baseGPR = base.gpr();
+ GPRReg argumentGPR = argument.gpr();
+
+ flushRegisters();
+ GPRFlushedCallResult result(this);
+ callOperation(operationRegExpExec, result.gpr(), baseGPR, argumentGPR);
+ m_jit.exceptionCheck();
+
+ jsValueResult(result.gpr(), node);
+ break;
+ }
+
+ JSValueOperand base(this, node->child1());
+ JSValueOperand argument(this, node->child2());
GPRReg baseGPR = base.gpr();
GPRReg argumentGPR = argument.gpr();
flushRegisters();
GPRFlushedCallResult result(this);
- callOperation(operationRegExpExec, result.gpr(), baseGPR, argumentGPR);
+ callOperation(operationRegExpExecGeneric, result.gpr(), baseGPR, argumentGPR);
m_jit.exceptionCheck();
jsValueResult(result.gpr(), node);
@@ -2976,14 +2992,32 @@
}
case RegExpTest: {
- SpeculateCellOperand base(this, node->child1());
- SpeculateCellOperand argument(this, node->child2());
+ if (node->child1().useKind() == CellUse
+ && node->child2().useKind() == CellUse) {
+ SpeculateCellOperand base(this, node->child1());
+ SpeculateCellOperand argument(this, node->child2());
+ GPRReg baseGPR = base.gpr();
+ GPRReg argumentGPR = argument.gpr();
+
+ flushRegisters();
+ GPRFlushedCallResult result(this);
+ callOperation(operationRegExpTest, result.gpr(), baseGPR, argumentGPR);
+ m_jit.exceptionCheck();
+
+ // If we add a DataFormatBool, we should use it here.
+ m_jit.or32(TrustedImm32(ValueFalse), result.gpr());
+ jsValueResult(result.gpr(), node, DataFormatJSBoolean);
+ break;
+ }
+
+ JSValueOperand base(this, node->child1());
+ JSValueOperand argument(this, node->child2());
GPRReg baseGPR = base.gpr();
GPRReg argumentGPR = argument.gpr();
flushRegisters();
GPRFlushedCallResult result(this);
- callOperation(operationRegExpTest, result.gpr(), baseGPR, argumentGPR);
+ callOperation(operationRegExpTestGeneric, result.gpr(), baseGPR, argumentGPR);
m_jit.exceptionCheck();
// If we add a DataFormatBool, we should use it here.
Modified: trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp (197491 => 197492)
--- trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp 2016-03-03 05:29:16 UTC (rev 197491)
+++ trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp 2016-03-03 05:58:59 UTC (rev 197492)
@@ -6434,18 +6434,36 @@
void compileRegExpExec()
{
- LValue base = lowCell(m_node->child1());
- LValue argument = lowCell(m_node->child2());
+ if (m_node->child1().useKind() == CellUse
+ && m_node->child2().useKind() == CellUse) {
+ LValue base = lowCell(m_node->child1());
+ LValue argument = lowCell(m_node->child2());
+ setJSValue(
+ vmCall(Int64, m_out.operation(operationRegExpExec), m_callFrame, base, argument));
+ return;
+ }
+
+ LValue base = lowJSValue(m_node->child1());
+ LValue argument = lowJSValue(m_node->child2());
setJSValue(
- vmCall(Int64, m_out.operation(operationRegExpExec), m_callFrame, base, argument));
+ vmCall(Int64, m_out.operation(operationRegExpExecGeneric), m_callFrame, base, argument));
}
void compileRegExpTest()
{
- LValue base = lowCell(m_node->child1());
- LValue argument = lowCell(m_node->child2());
+ if (m_node->child1().useKind() == CellUse
+ && m_node->child2().useKind() == CellUse) {
+ LValue base = lowCell(m_node->child1());
+ LValue argument = lowCell(m_node->child2());
+ setBoolean(
+ vmCall(Int32, m_out.operation(operationRegExpTest), m_callFrame, base, argument));
+ return;
+ }
+
+ LValue base = lowJSValue(m_node->child1());
+ LValue argument = lowJSValue(m_node->child2());
setBoolean(
- vmCall(Int32, m_out.operation(operationRegExpTest), m_callFrame, base, argument));
+ vmCall(Int32, m_out.operation(operationRegExpTestGeneric), m_callFrame, base, argument));
}
void compileNewRegexp()
Added: trunk/Source/_javascript_Core/tests/stress/regexp-exec-effect-after-exception.js (0 => 197492)
--- trunk/Source/_javascript_Core/tests/stress/regexp-exec-effect-after-exception.js (rev 0)
+++ trunk/Source/_javascript_Core/tests/stress/regexp-exec-effect-after-exception.js 2016-03-03 05:58:59 UTC (rev 197492)
@@ -0,0 +1,26 @@
+function foo(s) {
+ return /.*/.exec(s);
+}
+
+noInline(foo);
+
+for (var i = 0; i < 10000; ++i)
+ foo("foo bar");
+
+RegExp.input = "blah";
+
+var didFinish = false;
+try {
+ foo({toString: function() {
+ throw "error";
+ }});
+ didFinish = true;
+} catch (e) {
+ if (e != "error")
+ throw "Error: bad exception at end: " + e;
+ if (RegExp.input != "blah")
+ throw "Error: bad value of input: " + RegExp.input;
+}
+
+if (didFinish)
+ throw "Error: did not throw exception.";
