- Revision
- 268783
- Author
- sbar...@apple.com
- Date
- 2020-10-20 21:00:05 -0700 (Tue, 20 Oct 2020)
Log Message
Don't OSR exit to bc#0 for FTL argument type checks during loop OSR entry
https://bugs.webkit.org/show_bug.cgi?id=217925
<rdar://problem/70369407>
Reviewed by Michael Saboff and Tadeu Zagallo.
JSTests:
* stress/ftl-osr-entry-should-not-exit-to-bc-zero.js: Added.
Source/_javascript_Core:
When the FTL was emitting type checks for the named arguments of a function,
it was always emitting these type checks with an exit origin of bc#0. It was
doing this even if we were an OSR entry compilation! This meant that type
checks for arguments that failed during loop OSR entry would incorrectly exit
back to bc#0.
This patch fixes this by having the OSR entry runtime code validate the
argument types before OSR entering. The current OSR entry compiled code in
the FTL is designed to only allow exiting after all ExtractOSREntryLocal and
MovHints have executed, so it is simpler to put the type checks in the runtime
instead of the compiled code.
This patch also makes it so we do exponential backoff when failing to OSR
enter. This is needed due to insufficient profiling where we never properly
profile the type of arguments. Before this, we'd OSR exit in the FTL code
itself, which does exponential backoff when recompiling. This patch builds
this same exponential backoff in for when we fail to OSR enter enough times
to give up on the OSR entry compilation.
* ftl/FTLForOSREntryJITCode.h:
* ftl/FTLLowerDFGToB3.cpp:
(JSC::FTL::DFG::LowerDFGToB3::lower):
* ftl/FTLOSREntry.cpp:
(JSC::FTL::prepareOSREntry):
Modified Paths
Added Paths
Diff
Modified: trunk/JSTests/ChangeLog (268782 => 268783)
--- trunk/JSTests/ChangeLog 2020-10-21 03:33:03 UTC (rev 268782)
+++ trunk/JSTests/ChangeLog 2020-10-21 04:00:05 UTC (rev 268783)
@@ -1,3 +1,13 @@
+2020-10-20 Saam Barati <sbar...@apple.com>
+
+ Don't OSR exit to bc#0 for FTL argument type checks during loop OSR entry
+ https://bugs.webkit.org/show_bug.cgi?id=217925
+ <rdar://problem/70369407>
+
+ Reviewed by Michael Saboff and Tadeu Zagallo.
+
+ * stress/ftl-osr-entry-should-not-exit-to-bc-zero.js: Added.
+
2020-10-20 Michael Saboff <msab...@apple.com>
[JSC] Update RegExp UCD to version 13.0
Added: trunk/JSTests/stress/ftl-osr-entry-should-not-exit-to-bc-zero.js (0 => 268783)
--- trunk/JSTests/stress/ftl-osr-entry-should-not-exit-to-bc-zero.js (rev 0)
+++ trunk/JSTests/stress/ftl-osr-entry-should-not-exit-to-bc-zero.js 2020-10-21 04:00:05 UTC (rev 268783)
@@ -0,0 +1,5 @@
+//@ runDefault("--verboseOSRExitFuzz=0", "--useOSRExitFuzz=1", "--fireOSRExitFuzzAtOrAfter=10", "--jitPolicyScale=0", "--useConcurrentJIT=0")
+
+let a = [0, 0, 0];
+a.sort();
+a.sort();
Modified: trunk/Source/_javascript_Core/ChangeLog (268782 => 268783)
--- trunk/Source/_javascript_Core/ChangeLog 2020-10-21 03:33:03 UTC (rev 268782)
+++ trunk/Source/_javascript_Core/ChangeLog 2020-10-21 04:00:05 UTC (rev 268783)
@@ -1,3 +1,36 @@
+2020-10-20 Saam Barati <sbar...@apple.com>
+
+ Don't OSR exit to bc#0 for FTL argument type checks during loop OSR entry
+ https://bugs.webkit.org/show_bug.cgi?id=217925
+ <rdar://problem/70369407>
+
+ Reviewed by Michael Saboff and Tadeu Zagallo.
+
+ When the FTL was emitting type checks for the named arguments of a function,
+ it was always emitting these type checks with an exit origin of bc#0. It was
+ doing this even if we were an OSR entry compilation! This meant that type
+ checks for arguments that failed during loop OSR entry would incorrectly exit
+ back to bc#0.
+
+ This patch fixes this by having the OSR entry runtime code validate the
+ argument types before OSR entering. The current OSR entry compiled code in
+ the FTL is designed to only allow exiting after all ExtractOSREntryLocal and
+ MovHints have executed, so it is simpler to put the type checks in the runtime
+ instead of the compiled code.
+
+ This patch also makes it so we do exponential backoff when failing to OSR
+ enter. This is needed due to insufficient profiling where we never properly
+ profile the type of arguments. Before this, we'd OSR exit in the FTL code
+ itself, which does exponential backoff when recompiling. This patch builds
+ this same exponential backoff in for when we fail to OSR enter enough times
+ to give up on the OSR entry compilation.
+
+ * ftl/FTLForOSREntryJITCode.h:
+ * ftl/FTLLowerDFGToB3.cpp:
+ (JSC::FTL::DFG::LowerDFGToB3::lower):
+ * ftl/FTLOSREntry.cpp:
+ (JSC::FTL::prepareOSREntry):
+
2020-10-20 Michael Saboff <msab...@apple.com>
[JSC] Update RegExp UCD to version 13.0
Modified: trunk/Source/_javascript_Core/dfg/DFGOperations.cpp (268782 => 268783)
--- trunk/Source/_javascript_Core/dfg/DFGOperations.cpp 2020-10-21 03:33:03 UTC (rev 268782)
+++ trunk/Source/_javascript_Core/dfg/DFGOperations.cpp 2020-10-21 04:00:05 UTC (rev 268783)
@@ -3768,6 +3768,23 @@
return nullptr;
}
+ auto failedOSREntry = [&] (CodeBlock* entryBlock) {
+ FTL::ForOSREntryJITCode* entryCode = entryBlock->jitCode()->ftlForOSREntry();
+ entryCode->countEntryFailure();
+ if (entryCode->entryFailureCount() <
+ Options::ftlOSREntryFailureCountForReoptimization()) {
+ CODEBLOCK_LOG_EVENT(codeBlock, "delayFTLCompile", ("OSR entry failed"));
+ jitCode->setOptimizationThresholdBasedOnCompilationResult(
+ codeBlock, CompilationDeferred);
+ return nullptr;
+ }
+
+ CODEBLOCK_LOG_EVENT(codeBlock, "delayFTLCompile", ("OSR entry failed too many times"));
+ codeBlock->baselineVersion()->countReoptimization();
+ jitCode->clearOSREntryBlockAndResetThresholds(codeBlock);
+ return nullptr;
+ };
+
// If we can OSR Enter, do it right away.
if (canOSREnterHere) {
auto iter = jitCode->bytecodeIndexToStreamIndex.find(originBytecodeIndex);
@@ -3781,6 +3798,8 @@
CODEBLOCK_LOG_EVENT(entryBlock, "osrEntry", ("at ", originBytecodeIndex));
return tagCodePtrWithStackPointerForJITCall(untagCodePtr<char*, JSEntryPtrTag>(address), callFrame);
}
+
+ return failedOSREntry(entryBlock);
}
}
}
@@ -3822,21 +3841,7 @@
return nullptr;
}
- FTL::ForOSREntryJITCode* entryCode = entryBlock->jitCode()->ftlForOSREntry();
- entryCode->countEntryFailure();
- if (entryCode->entryFailureCount() <
- Options::ftlOSREntryFailureCountForReoptimization()) {
- CODEBLOCK_LOG_EVENT(codeBlock, "delayFTLCompile", ("OSR entry failed"));
- jitCode->setOptimizationThresholdBasedOnCompilationResult(
- codeBlock, CompilationDeferred);
- return nullptr;
- }
-
- // OSR entry failed. Oh no! This implies that we need to retry. We retry
- // without exponential backoff and we only do this for the entry code block.
- CODEBLOCK_LOG_EVENT(codeBlock, "delayFTLCompile", ("OSR entry failed too many times"));
- jitCode->clearOSREntryBlockAndResetThresholds(codeBlock);
- return nullptr;
+ return failedOSREntry(entryBlock);
}
// It's time to try to compile code for OSR entry.
@@ -3933,7 +3938,7 @@
ASSERT(canOSREnterHere);
void* address = FTL::prepareOSREntry(vm, callFrame, codeBlock, jitCode->osrEntryBlock(), originBytecodeIndex, streamIndex);
if (!address)
- return nullptr;
+ return failedOSREntry(jitCode->osrEntryBlock());
return tagCodePtrWithStackPointerForJITCall(untagCodePtr<char*, JSEntryPtrTag>(address), callFrame);
}
Modified: trunk/Source/_javascript_Core/ftl/FTLForOSREntryJITCode.h (268782 => 268783)
--- trunk/Source/_javascript_Core/ftl/FTLForOSREntryJITCode.h 2020-10-21 03:33:03 UTC (rev 268782)
+++ trunk/Source/_javascript_Core/ftl/FTLForOSREntryJITCode.h 2020-10-21 04:00:05 UTC (rev 268783)
@@ -55,8 +55,10 @@
unsigned entryFailureCount() const { return m_entryFailureCount; }
ForOSREntryJITCode* ftlForOSREntry() final;
-
+ Vector<DFG::FlushFormat>& argumentFlushFormats() { return m_argumentFlushFormats; }
+
private:
+ Vector<DFG::FlushFormat> m_argumentFlushFormats;
ScratchBuffer* m_entryBuffer; // Only for OSR entry code blocks.
BytecodeIndex m_bytecodeIndex;
unsigned m_entryFailureCount;
Modified: trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp (268782 => 268783)
--- trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp 2020-10-21 03:33:03 UTC (rev 268782)
+++ trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp 2020-10-21 04:00:05 UTC (rev 268783)
@@ -358,28 +358,36 @@
Availability(FlushedAt(FlushedJSValue, virtualRegisterForArgumentIncludingThis(i)));
}
- for (unsigned i = codeBlock()->numParameters(); i--;) {
- MethodOfGettingAValueProfile profile(&m_graph.m_profiledBlock->valueProfileForArgument(i));
- VirtualRegister operand = virtualRegisterForArgumentIncludingThis(i);
- LValue jsValue = m_out.load64(addressFor(operand));
-
- switch (m_graph.m_argumentFormats[0][i]) {
- case FlushedInt32:
- speculate(BadType, jsValueValue(jsValue), profile, isNotInt32(jsValue));
- break;
- case FlushedBoolean:
- speculate(BadType, jsValueValue(jsValue), profile, isNotBoolean(jsValue));
- break;
- case FlushedCell:
- speculate(BadType, jsValueValue(jsValue), profile, isNotCell(jsValue));
- break;
- case FlushedJSValue:
- break;
- default:
- DFG_CRASH(m_graph, nullptr, "Bad flush format for argument");
- break;
+ if (m_graph.m_plan.mode() == FTLForOSREntryMode) {
+ auto* jitCode = m_ftlState.jitCode->ftlForOSREntry();
+ jitCode->argumentFlushFormats().reserveInitialCapacity(codeBlock()->numParameters());
+ for (unsigned i = codeBlock()->numParameters(); i--;)
+ jitCode->argumentFlushFormats().append(m_graph.m_argumentFormats[0][i]);
+ } else {
+ for (unsigned i = codeBlock()->numParameters(); i--;) {
+ MethodOfGettingAValueProfile profile(&m_graph.m_profiledBlock->valueProfileForArgument(i));
+ VirtualRegister operand = virtualRegisterForArgumentIncludingThis(i);
+ LValue jsValue = m_out.load64(addressFor(operand));
+
+ switch (m_graph.m_argumentFormats[0][i]) {
+ case FlushedInt32:
+ speculate(BadType, jsValueValue(jsValue), profile, isNotInt32(jsValue));
+ break;
+ case FlushedBoolean:
+ speculate(BadType, jsValueValue(jsValue), profile, isNotBoolean(jsValue));
+ break;
+ case FlushedCell:
+ speculate(BadType, jsValueValue(jsValue), profile, isNotCell(jsValue));
+ break;
+ case FlushedJSValue:
+ break;
+ default:
+ DFG_CRASH(m_graph, nullptr, "Bad flush format for argument");
+ break;
+ }
}
}
+
m_out.jump(firstDFGBasicBlock);
}
Modified: trunk/Source/_javascript_Core/ftl/FTLOSREntry.cpp (268782 => 268783)
--- trunk/Source/_javascript_Core/ftl/FTLOSREntry.cpp 2020-10-21 03:33:03 UTC (rev 268782)
+++ trunk/Source/_javascript_Core/ftl/FTLOSREntry.cpp 2020-10-21 04:00:05 UTC (rev 268783)
@@ -74,6 +74,31 @@
for (int argument = values.numberOfArguments(); argument--;) {
JSValue valueOnStack = callFrame->r(virtualRegisterForArgumentIncludingThis(argument)).asanUnsafeJSValue();
Optional<JSValue> reconstructedValue = values.argument(argument);
+ {
+ JSValue valueToValidate = reconstructedValue ? *reconstructedValue : valueOnStack;
+ auto flushFormat = entryCode->argumentFlushFormats()[argument];
+ switch (flushFormat) {
+ case DFG::FlushedInt32:
+ if (!valueToValidate.isInt32())
+ return nullptr;
+ break;
+ case DFG::FlushedBoolean:
+ if (!valueToValidate.isBoolean())
+ return nullptr;
+ break;
+ case DFG::FlushedCell:
+ if (!valueToValidate.isCell())
+ return nullptr;
+ break;
+ case DFG::FlushedJSValue:
+ break;
+ default:
+ dataLogLn("Unknown flush format for argument during FTL osr entry: ", flushFormat);
+ RELEASE_ASSERT_NOT_REACHED();
+ break;
+ }
+ }
+
if (!argument) {
// |this| argument can be unboxed. We should store boxed value instead for loop OSR entry since FTL assumes that all arguments are flushed JSValue.
// To make this valid, we will modify the stack on the fly: replacing the value with boxed value.
