OSR entry: delay outer-loop compilation when at inner-loop

https://bugs.webkit.org/show_bug.cgi?id=167149

Reviewed by Filip Pizlo.

JSTests:

Try to be mean to OSR entry by using nested loops, and having
non-int32 types or truly varying types.

Mandelbrot currently never tiers up to FTL because it exits too
many times before this. That shouldn't happen because it's just
numbers and int32s. I'll file a bug to fix this.

* microbenchmarks/mandelbrot.js: Added.
(mandelbrot):
(printable):
* microbenchmarks/nonude.js: Added.
(Array.prototype.remove):
(const.u):
(const.load):
(const.scan):
(const.main):

Source/JavaScriptCore:

As of https://bugs.webkit.org/show_bug.cgi?id=155217 OSR
compilation can be kicked off for an entry into an outer-loop,
while executing an inner-loop. This is desirable because often the
codegen from an inner-entry isn't as good as the codegen from an
outer-entry, but execution from an inner-loop is often pretty hot
and likely to kick off compilation. This approach provided nice
speedups on Kraken because we'd select to enter to the outer-loop
very reliably, which reduces variability (the inner-loop was
selected roughly 1/5 times from my unscientific measurements).

When compilation starts we take a snapshot of the JSValues at the
current execution state using OSR's recovery mechanism. These
values are passed to the compiler and are used as way to perform
type profiling, and could be used to observe cell types as well as
to perform predictions such as through constant propagation.

It's therefore desired to enter from the outer-loop when we can,
but we need to be executing from that location to capture the
right JSValues, otherwise we're confusing the compiler and giving
it inaccurate JSValues which can lead it to predict the wrong
things, leading to suboptimal code or recompilation due to
misprediction, or in super-corner-cases a crash.

These effects are pretty hard to measure: Fil points out that
marsalis-osr-entry really needs mustHandleValues (the JSValues
from the point of execution) because right now it just happens to
correctly guess int32. I tried removing mustHandleValues entirely
and saw no slowdowns, but our benchmarks probably aren't
sufficient to reliably find issues, sometimes because we happen to
have sufficient mitigations.

DFG tier-up was added here:
https://bugs.webkit.org/show_bug.cgi?id=112838

* JavaScriptCore.xcodeproj/project.pbxproj:
* dfg/DFGJITCode.h:
* dfg/DFGJITCompiler.cpp:
(JSC::DFG::JITCompiler::JITCompiler):
* dfg/DFGOSREntry.cpp:
(JSC::DFG::prepareOSREntry):
* dfg/DFGOSREntry.h:
(JSC::DFG::prepareOSREntry):
* dfg/DFGOperations.cpp:
* dfg/DFGOperations.h:
* dfg/DFGSpeculativeJIT64.cpp:
(JSC::DFG::SpeculativeJIT::compile):
* dfg/DFGTierUpEntryTrigger.h: Copied from Source/JavaScriptCore/ftl/FTLOSREntry.h.
* dfg/DFGToFTLForOSREntryDeferredCompilationCallback.cpp:
(JSC::DFG::ToFTLForOSREntryDeferredCompilationCallback::ToFTLForOSREntryDeferredCompilationCallback):
(JSC::DFG::Ref<ToFTLForOSREntryDeferredCompilationCallback>ToFTLForOSREntryDeferredCompilationCallback::create):
(JSC::DFG::ToFTLForOSREntryDeferredCompilationCallback::compilationDidBecomeReadyAsynchronously):
(JSC::DFG::ToFTLForOSREntryDeferredCompilationCallback::compilationDidComplete):
* dfg/DFGToFTLForOSREntryDeferredCompilationCallback.h:
* ftl/FTLOSREntry.cpp:
(JSC::FTL::prepareOSREntry):
* ftl/FTLOSREntry.h:
* jit/JITOperations.cpp:


git-svn-id: http://svn.webkit.org/repository/webkit/trunk@211224 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Author: [email protected]

JSTests/ChangeLog

@@ -1,3 +1,27 @@
+2017-01-26  JF Bastien  <[email protected]>
+
+        OSR entry: delay outer-loop compilation when at inner-loop
+        https://bugs.webkit.org/show_bug.cgi?id=167149
+
+        Reviewed by Filip Pizlo.
+
+        Try to be mean to OSR entry by using nested loops, and having
+        non-int32 types or truly varying types.
+
+        Mandelbrot currently never tiers up to FTL because it exits too
+        many times before this. That shouldn't happen because it's just
+        numbers and int32s. I'll file a bug to fix this.
+
+        * microbenchmarks/mandelbrot.js: Added.
+        (mandelbrot):
+        (printable):
+        * microbenchmarks/nonude.js: Added.
+        (Array.prototype.remove):
+        (const.u):
+        (const.load):
+        (const.scan):
+        (const.main):
+
 2017-01-25  Saam Barati  <[email protected]>
 
         WebAssembly JS API: coerce return values from imports

JSTests/microbenchmarks/mandelbrot.js

@@ -0,0 +1,62 @@
+const max_s = 255;
+const hysterisis = 3;
+
+function mandelbrot(x, y, w, h, scene_i, scene_z, X, Y, iterations) {
+    const max_z = 2.0;
+    
+    for (let i = X; i; --i)
+        for (let j = Y; j; --j) {
+            let real = x + (i - 1) / (1.0 * (X - 1) * w);
+            let imag = y + (j - 1) / (1.0 * (Y - 1) * h);
+            let z_real = 0.0;
+            let z_imag = 0.0;
+            
+            let it = iterations;
+            while (--it && Math.sqrt(z_real**2 + z_imag**2) < max_z) {
+                const next_real = z_real**2 - z_imag**2 + real;
+                const next_imag = 2 * (z_real * z_imag) + imag;
+                z_real = next_real;
+                z_imag = next_imag;
+            }
+            
+            const idx = (j - 1) * X + (i - 1);
+            const new_i = max_s * it / (1.0 * iterations);
+            scene_i[idx] = ((scene_i[idx] + new_i * hysterisis) / (hysterisis + 1)) | 0;
+            const new_z = max_s * Math.min(max_z, Math.sqrt(z_real**2 + z_imag**2)) / max_z;
+            scene_z[idx] = ((scene_z[idx] + new_z * hysterisis) / (hysterisis + 1)) | 0;
+        }
+}
+
+const shades = ['█', '▉', '▊', '▋', '▌', '▍', '▎', '▏', '▓', '▒', '░'];
+const shade = i => shades[Math.round((i * (shades.length - 1)) / max_s)];
+
+function printable(scene_i, scene_z, X, Y) {
+    let s = "";
+    for (let i = 0; i < X; ++i) {
+        for (let j = 0; j < Y; ++j) {
+            const idx = j * X + i;
+            s += shade((scene_i[idx] + scene_z[idx]) / 2);
+            if (j == Y - 1)
+                s += '\n';
+        }
+    }
+    return s;
+}
+
+const width = 80, height = 80;
+let scene_i = Array.of(width * height).fill(0);
+let scene_z = Array.of(width * height).fill(0);
+
+for (let i = 1; i < 64; ++i) {
+    mandelbrot(-0.5, -0.5, 0.5, 0.5, scene_i, scene_z, width, height, i);
+}
+
+const result = printable(scene_i, scene_z, width, height);
+
+let seen = false;
+for (let i = 0; i < result.length; ++i)
+    if (result[i] === shades[0])
+        seen = true;
+
+if (!seen)
+    throw new Error(`Unexpected mandelbrot:\n${result}`);

JSTests/microbenchmarks/nonude.js

@@ -1,3 +1,68 @@
+2017-01-26  JF Bastien  <[email protected]>
+
+        OSR entry: delay outer-loop compilation when at inner-loop
+        https://bugs.webkit.org/show_bug.cgi?id=167149
+
+        Reviewed by Filip Pizlo.
+
+        As of https://bugs.webkit.org/show_bug.cgi?id=155217 OSR
+        compilation can be kicked off for an entry into an outer-loop,
+        while executing an inner-loop. This is desirable because often the
+        codegen from an inner-entry isn't as good as the codegen from an
+        outer-entry, but execution from an inner-loop is often pretty hot
+        and likely to kick off compilation. This approach provided nice
+        speedups on Kraken because we'd select to enter to the outer-loop
+        very reliably, which reduces variability (the inner-loop was
+        selected roughly 1/5 times from my unscientific measurements).
+
+        When compilation starts we take a snapshot of the JSValues at the
+        current execution state using OSR's recovery mechanism. These
+        values are passed to the compiler and are used as way to perform
+        type profiling, and could be used to observe cell types as well as
+        to perform predictions such as through constant propagation.
+
+        It's therefore desired to enter from the outer-loop when we can,
+        but we need to be executing from that location to capture the
+        right JSValues, otherwise we're confusing the compiler and giving
+        it inaccurate JSValues which can lead it to predict the wrong
+        things, leading to suboptimal code or recompilation due to
+        misprediction, or in super-corner-cases a crash.
+
+        These effects are pretty hard to measure: Fil points out that
+        marsalis-osr-entry really needs mustHandleValues (the JSValues
+        from the point of execution) because right now it just happens to
+        correctly guess int32. I tried removing mustHandleValues entirely
+        and saw no slowdowns, but our benchmarks probably aren't
+        sufficient to reliably find issues, sometimes because we happen to
+        have sufficient mitigations.
+
+        DFG tier-up was added here:
+        https://bugs.webkit.org/show_bug.cgi?id=112838
+
+        * JavaScriptCore.xcodeproj/project.pbxproj:
+        * dfg/DFGJITCode.h:
+        * dfg/DFGJITCompiler.cpp:
+        (JSC::DFG::JITCompiler::JITCompiler):
+        * dfg/DFGOSREntry.cpp:
+        (JSC::DFG::prepareOSREntry):
+        * dfg/DFGOSREntry.h:
+        (JSC::DFG::prepareOSREntry):
+        * dfg/DFGOperations.cpp:
+        * dfg/DFGOperations.h:
+        * dfg/DFGSpeculativeJIT64.cpp:
+        (JSC::DFG::SpeculativeJIT::compile):
+        * dfg/DFGTierUpEntryTrigger.h: Copied from Source/JavaScriptCore/ftl/FTLOSREntry.h.
+        * dfg/DFGToFTLForOSREntryDeferredCompilationCallback.cpp:
+        (JSC::DFG::ToFTLForOSREntryDeferredCompilationCallback::ToFTLForOSREntryDeferredCompilationCallback):
+        (JSC::DFG::Ref<ToFTLForOSREntryDeferredCompilationCallback>ToFTLForOSREntryDeferredCompilationCallback::create):
+        (JSC::DFG::ToFTLForOSREntryDeferredCompilationCallback::compilationDidBecomeReadyAsynchronously):
+        (JSC::DFG::ToFTLForOSREntryDeferredCompilationCallback::compilationDidComplete):
+        * dfg/DFGToFTLForOSREntryDeferredCompilationCallback.h:
+        * ftl/FTLOSREntry.cpp:
+        (JSC::FTL::prepareOSREntry):
+        * ftl/FTLOSREntry.h:
+        * jit/JITOperations.cpp:
+
 2017-01-25  Saam Barati  <[email protected]>
 
         WebAssembly JS API: coerce return values from imports

Source/JavaScriptCore/ChangeLog

@@ -2059,6 +2059,7 @@
 		ADE8029B1E08F1DE0058DE78 /* WebAssemblyLinkErrorPrototype.cpp in Sources */ = {isa = PBXBuildFile; fileRef = ADE802961E08F1C90058DE78 /* WebAssemblyLinkErrorPrototype.cpp */; };
 		ADE8029C1E08F1DE0058DE78 /* WebAssemblyLinkErrorPrototype.h in Headers */ = {isa = PBXBuildFile; fileRef = ADE802971E08F1C90058DE78 /* WebAssemblyLinkErrorPrototype.h */; settings = {ATTRIBUTES = (Private, ); }; };
 		ADE8029E1E08F2280058DE78 /* WebAssemblyLinkErrorConstructor.cpp in Sources */ = {isa = PBXBuildFile; fileRef = ADE8029D1E08F2260058DE78 /* WebAssemblyLinkErrorConstructor.cpp */; };
+		ADFF2F701E319DE3001EA54E /* DFGTierUpEntryTrigger.h in Headers */ = {isa = PBXBuildFile; fileRef = ADFF2F6F1E319DD0001EA54E /* DFGTierUpEntryTrigger.h */; };
 		B59F89391891F29F00D5CCDC /* UnlinkedInstructionStream.cpp in Sources */ = {isa = PBXBuildFile; fileRef = B59F89381891ADB500D5CCDC /* UnlinkedInstructionStream.cpp */; };
 		BC02E90D0E1839DB000F9297 /* ErrorConstructor.h in Headers */ = {isa = PBXBuildFile; fileRef = BC02E9050E1839DB000F9297 /* ErrorConstructor.h */; };
 		BC02E90F0E1839DB000F9297 /* ErrorPrototype.h in Headers */ = {isa = PBXBuildFile; fileRef = BC02E9070E1839DB000F9297 /* ErrorPrototype.h */; settings = {ATTRIBUTES = (Private, ); }; };
@@ -4571,6 +4572,7 @@
 		ADE802961E08F1C90058DE78 /* WebAssemblyLinkErrorPrototype.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = WebAssemblyLinkErrorPrototype.cpp; path = js/WebAssemblyLinkErrorPrototype.cpp; sourceTree = "<group>"; };
 		ADE802971E08F1C90058DE78 /* WebAssemblyLinkErrorPrototype.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = WebAssemblyLinkErrorPrototype.h; path = js/WebAssemblyLinkErrorPrototype.h; sourceTree = "<group>"; };
 		ADE8029D1E08F2260058DE78 /* WebAssemblyLinkErrorConstructor.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = WebAssemblyLinkErrorConstructor.cpp; path = js/WebAssemblyLinkErrorConstructor.cpp; sourceTree = "<group>"; };
+		ADFF2F6F1E319DD0001EA54E /* DFGTierUpEntryTrigger.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = DFGTierUpEntryTrigger.h; path = dfg/DFGTierUpEntryTrigger.h; sourceTree = "<group>"; };
 		B59F89371891AD3300D5CCDC /* UnlinkedInstructionStream.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = UnlinkedInstructionStream.h; sourceTree = "<group>"; };
 		B59F89381891ADB500D5CCDC /* UnlinkedInstructionStream.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = UnlinkedInstructionStream.cpp; sourceTree = "<group>"; };
 		BC021BF2136900C300FC5467 /* ToolExecutable.xcconfig */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.xcconfig; path = ToolExecutable.xcconfig; sourceTree = "<group>"; };
@@ -7129,6 +7131,7 @@
 				0FC097A0146B28C700CF2442 /* DFGThunks.h */,
 				0FD8A31F17D51F5700CA2C40 /* DFGTierUpCheckInjectionPhase.cpp */,
 				0FD8A32017D51F5700CA2C40 /* DFGTierUpCheckInjectionPhase.h */,
+				ADFF2F6F1E319DD0001EA54E /* DFGTierUpEntryTrigger.h */,
 				0FD8A32117D51F5700CA2C40 /* DFGToFTLDeferredCompilationCallback.cpp */,
 				0FD8A32217D51F5700CA2C40 /* DFGToFTLDeferredCompilationCallback.h */,
 				0FD8A32317D51F5700CA2C40 /* DFGToFTLForOSREntryDeferredCompilationCallback.cpp */,
@@ -8811,6 +8814,7 @@
 				86D3B2C610156BDE002865E7 /* MacroAssemblerARM.h in Headers */,
 				A1A009C01831A22D00CF8711 /* MacroAssemblerARM64.h in Headers */,
 				86ADD1460FDDEA980006EEC2 /* MacroAssemblerARMv7.h in Headers */,
+				ADFF2F701E319DE3001EA54E /* DFGTierUpEntryTrigger.h in Headers */,
 				863B23E00FC6118900703AA4 /* MacroAssemblerCodeRef.h in Headers */,
 				E32AB2441DCD75F400D7533A /* MacroAssemblerHelpers.h in Headers */,
 				86C568E111A213EE0007F7F0 /* MacroAssemblerMIPS.h in Headers */,

Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj

@@ -33,6 +33,7 @@
 #include "DFGMinifiedGraph.h"
 #include "DFGOSREntry.h"
 #include "DFGOSRExit.h"
+#include "DFGTierUpEntryTrigger.h"
 #include "DFGVariableEventStream.h"
 #include "ExecutionCounter.h"
 #include "JITCode.h"
@@ -154,7 +155,7 @@ class JITCode : public DirectJITCode {
     // Map each bytecode of CheckTierUpAndOSREnter to its trigger forcing OSR Entry.
     // This can never be modified after it has been initialized since the addresses of the triggers
     // are used by the JIT.
-    HashMap<unsigned, uint8_t> tierUpEntryTriggers;
+    HashMap<unsigned, TierUpEntryTrigger> tierUpEntryTriggers;
 
     // Set of bytecode that were the target of a TierUp operation.
     HashSet<unsigned, WTF::IntHash<unsigned>, WTF::UnsignedWithZeroKeyHashTraits<unsigned>> tierUpEntrySeen;

Source/JavaScriptCore/dfg/DFGJITCode.h

@@ -60,7 +60,7 @@ JITCompiler::JITCompiler(Graph& dfg)
 #if ENABLE(FTL_JIT)
     m_jitCode->tierUpInLoopHierarchy = WTFMove(m_graph.m_plan.tierUpInLoopHierarchy);
     for (unsigned tierUpBytecode : m_graph.m_plan.tierUpAndOSREnterBytecodes)
-        m_jitCode->tierUpEntryTriggers.add(tierUpBytecode, 0);
+        m_jitCode->tierUpEntryTriggers.add(tierUpBytecode, TierUpEntryTrigger::None);
 #endif
 }
 

Source/JavaScriptCore/dfg/DFGJITCompiler.cpp

@@ -91,15 +91,15 @@ void OSREntryData::dump(PrintStream& out) const
 }
 
 SUPPRESS_ASAN
-void* prepareOSREntry(ExecState* exec, CodeBlock* codeBlock, unsigned bytecodeIndex)
+Expected<void*, OSREntryFail> prepareOSREntry(ExecState* exec, CodeBlock* codeBlock, unsigned bytecodeIndex)
 {
     ASSERT(JITCode::isOptimizingJIT(codeBlock->jitType()));
     ASSERT(codeBlock->alternative());
     ASSERT(codeBlock->alternative()->jitType() == JITCode::BaselineJIT);
     ASSERT(!codeBlock->jitCodeMap());
 
     if (!Options::useOSREntryToDFG())
-        return 0;
+        return makeUnexpected(OSREntryFail::Disabled);
 
     if (Options::verboseOSR()) {
         dataLog(
@@ -136,7 +136,7 @@ void* prepareOSREntry(ExecState* exec, CodeBlock* codeBlock, unsigned bytecodeIn
 
         if (Options::verboseOSR())
             dataLog("    OSR failed because the target code block is not DFG.\n");
-        return 0;
+        return makeUnexpected(OSREntryFail::TargetNotDFG);
     }
 
     JITCode* jitCode = codeBlock->jitCode()->dfg();
@@ -145,7 +145,7 @@ void* prepareOSREntry(ExecState* exec, CodeBlock* codeBlock, unsigned bytecodeIn
     if (!entry) {
         if (Options::verboseOSR())
             dataLogF("    OSR failed because the entrypoint was optimized out.\n");
-        return 0;
+        return makeUnexpected(OSREntryFail::TargetOptimizedOut);
     }
 
     ASSERT(entry->m_bytecodeIndex == bytecodeIndex);
@@ -181,7 +181,7 @@ void* prepareOSREntry(ExecState* exec, CodeBlock* codeBlock, unsigned bytecodeIn
                 entry->m_expectedValues.argument(argument).dump(WTF::dataFile());
                 dataLogF(".\n");
             }
-            return 0;
+            return makeUnexpected(OSREntryFail::BadPrediction);
         }
 
         JSValue value;
@@ -196,7 +196,7 @@ void* prepareOSREntry(ExecState* exec, CodeBlock* codeBlock, unsigned bytecodeIn
                     "    OSR failed because argument ", argument, " is ", value,
                     ", expected ", entry->m_expectedValues.argument(argument), ".\n");
             }
-            return 0;
+            return makeUnexpected(OSREntryFail::BadPrediction);
         }
     }
 
@@ -209,7 +209,7 @@ void* prepareOSREntry(ExecState* exec, CodeBlock* codeBlock, unsigned bytecodeIn
                         "    OSR failed because variable ", localOffset, " is ",
                         exec->registers()[localOffset].asanUnsafeJSValue(), ", expected number.\n");
                 }
-                return 0;
+                return makeUnexpected(OSREntryFail::BadPrediction);
             }
             continue;
         }
@@ -221,7 +221,7 @@ void* prepareOSREntry(ExecState* exec, CodeBlock* codeBlock, unsigned bytecodeIn
                         exec->registers()[localOffset].asanUnsafeJSValue(), ", expected ",
                         "machine int.\n");
                 }
-                return 0;
+                return makeUnexpected(OSREntryFail::BadPrediction);
             }
             continue;
         }
@@ -232,7 +232,7 @@ void* prepareOSREntry(ExecState* exec, CodeBlock* codeBlock, unsigned bytecodeIn
                     exec->registers()[localOffset].asanUnsafeJSValue(), ", expected ",
                     entry->m_expectedValues.local(local), ".\n");
             }
-            return 0;
+            return makeUnexpected(OSREntryFail::BadPrediction);
         }
     }
 
@@ -247,7 +247,7 @@ void* prepareOSREntry(ExecState* exec, CodeBlock* codeBlock, unsigned bytecodeIn
     if (UNLIKELY(!vm->ensureStackCapacityFor(&exec->registers()[virtualRegisterForLocal(frameSizeForCheck - 1).offset()]))) {
         if (Options::verboseOSR())
             dataLogF("    OSR failed because stack growth failed.\n");
-        return 0;
+        return makeUnexpected(OSREntryFail::StackGrowthFailed);
     }
 
     if (Options::verboseOSR())

Source/JavaScriptCore/dfg/DFGOSREntry.cpp

@@ -28,6 +28,7 @@
 #include "DFGAbstractValue.h"
 #include "Operands.h"
 #include <wtf/BitVector.h>
+#include <wtf/Expected.h>
 
 namespace JSC {
 
@@ -36,6 +37,14 @@ class CodeBlock;
 
 namespace DFG {
 
+enum class OSREntryFail {
+    Disabled,
+    TargetNotDFG,
+    TargetOptimizedOut,
+    BadPrediction,
+    StackGrowthFailed,
+};
+
 #if ENABLE(DFG_JIT)
 struct OSREntryReshuffling {
     OSREntryReshuffling() { }
@@ -69,11 +78,10 @@ inline unsigned getOSREntryDataBytecodeIndex(OSREntryData* osrEntryData)
     return osrEntryData->m_bytecodeIndex;
 }
 
-// Returns a pointer to a data buffer that the OSR entry thunk will recognize and
-// parse. If this returns null, it means 
-void* prepareOSREntry(ExecState*, CodeBlock*, unsigned bytecodeIndex);
+// Returns a pointer to a data buffer that the OSR entry thunk will recognize and parse.
+Expected<void*, OSREntryFail> prepareOSREntry(ExecState*, CodeBlock*, unsigned bytecodeIndex);
 #else
-inline void* prepareOSREntry(ExecState*, CodeBlock*, unsigned) { return 0; }
+inline Expected<void*, OSREntryFail> prepareOSREntry(ExecState*, CodeBlock*, unsigned) { return makeUnexpected(OSREntryFail::Disabled); }
 #endif
 
 } } // namespace JSC::DFG