<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[180956] trunk/Source/JavaScriptCore</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/180956">180956</a></dd>
<dt>Author</dt> <dd>fpizlo@apple.com</dd>
<dt>Date</dt> <dd>2015-03-03 14:35:14 -0800 (Tue, 03 Mar 2015)</dd>
</dl>
<h3>Log Message</h3>
<pre>JIT debugging features that selectively disable the JITs for code blocks need to stay out of the way of the critical path of JIT management
https://bugs.webkit.org/show_bug.cgi?id=142234
Reviewed by Mark Lam and Benjamin Poulain.
Long ago, we used to selectively disable compilation of CodeBlocks for debugging purposes by
adding hacks to DFGDriver.cpp. This was all well and good. It used the existing
CompilationFailed mode of the DFG driver to signal failure of CodeBlocks that we didn't want
to compile. That's great because CompilationFailed is a well-supported return value on the
critical path, usually used for when we run out of JIT memory.
Later, this was moved into DFGCapabilities. This was basically incorrect. It introduced a bug
where disabling compiling of a CodeBlock meant that we stopped inlining it as well. So if
you had a compiler bug that arose if foo was inlined into bar, and you bisected down to bar,
then foo would no longer get inlined and you wouldn't see the bug. That's busted.
So then we changed the code in DFGCapabilities to mark bar as CanCompile and foo as
CanInline. Now, foo wouldn't get compiled alone but it would get inlined.
But then we removed CanCompile because that capability mode only existed for the purpose of
our old varargs hacks. After that removal, "CanInline" became CannotCompile. This means
that if you bisect down on bar in the "foo inlined into bar" case, you'll crash in the DFG
because the baseline JIT wouldn't have known to insert profiling on foo.
We could fix this by bringing back CanInline.
But this is all a pile of nonsense. The debug support to selectively disable compilation of
some CodeBlocks shouldn't cross-cut our entire engine and should most certainly never involve
adding new capability modes. This support is a hack at best and is for use by JSC hackers
only. It should be as unintrusive as possible.
So, as in the ancient times, the only proper place to put this hack is in DFGDriver.cpp, and
return CompilationFailed. This is correct not just because it takes capability modes out of
the picture (and obviates the need to introduce new ones), but also because it means that
disabling compilation doesn't change the profiling mode of other CodeBlocks in the Baseline
JIT. Capability mode influences profiling mode which in turn influences code generation in
the Baseline JIT, sometimes in very significant ways - like, we sometimes do additional
double-to-int conversions in Baseline if we know that we might tier-up into the DFG, since
this buys us more precise profiling.
This change reduces the intrusiveness of debugging hacks by making them use the very simple
CompilationFailed mechanism rather than trying to influence capability modes. Capability
modes have very subtle effects on the whole engine, while CompilationFailed just makes the
engine pretend like the DFG compilation will happen at timelike infinity. That makes these
hacks much more likely to continue working as we make other changes to the system.
This brings back the ability to bisect down onto a function bar when bar inlines foo. Prior
to this change, we would crash in that case.
* dfg/DFGCapabilities.cpp:
(JSC::DFG::isSupported):
(JSC::DFG::mightCompileEval):
(JSC::DFG::mightCompileProgram):
(JSC::DFG::mightCompileFunctionForCall):
(JSC::DFG::mightCompileFunctionForConstruct):
* dfg/DFGCapabilities.h:
* dfg/DFGDriver.cpp:
(JSC::DFG::compileImpl):</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCoredfgDFGCapabilitiescpp">trunk/Source/JavaScriptCore/dfg/DFGCapabilities.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoredfgDFGCapabilitiesh">trunk/Source/JavaScriptCore/dfg/DFGCapabilities.h</a></li>
<li><a href="#trunkSourceJavaScriptCoredfgDFGDrivercpp">trunk/Source/JavaScriptCore/dfg/DFGDriver.cpp</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (180955 => 180956)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog 2015-03-03 22:12:49 UTC (rev 180955)
+++ trunk/Source/JavaScriptCore/ChangeLog 2015-03-03 22:35:14 UTC (rev 180956)
</span><span class="lines">@@ -1,3 +1,64 @@
</span><ins>+2015-03-03 Filip Pizlo <fpizlo@apple.com>
+
+ JIT debugging features that selectively disable the JITs for code blocks need to stay out of the way of the critical path of JIT management
+ https://bugs.webkit.org/show_bug.cgi?id=142234
+
+ Reviewed by Mark Lam and Benjamin Poulain.
+
+ Long ago, we used to selectively disable compilation of CodeBlocks for debugging purposes by
+ adding hacks to DFGDriver.cpp. This was all well and good. It used the existing
+ CompilationFailed mode of the DFG driver to signal failure of CodeBlocks that we didn't want
+ to compile. That's great because CompilationFailed is a well-supported return value on the
+ critical path, usually used for when we run out of JIT memory.
+
+ Later, this was moved into DFGCapabilities. This was basically incorrect. It introduced a bug
+ where disabling compiling of a CodeBlock meant that we stopped inlining it as well. So if
+ you had a compiler bug that arose if foo was inlined into bar, and you bisected down to bar,
+ then foo would no longer get inlined and you wouldn't see the bug. That's busted.
+
+ So then we changed the code in DFGCapabilities to mark bar as CanCompile and foo as
+ CanInline. Now, foo wouldn't get compiled alone but it would get inlined.
+
+ But then we removed CanCompile because that capability mode only existed for the purpose of
+ our old varargs hacks. After that removal, "CanInline" became CannotCompile. This means
+ that if you bisect down on bar in the "foo inlined into bar" case, you'll crash in the DFG
+ because the baseline JIT wouldn't have known to insert profiling on foo.
+
+ We could fix this by bringing back CanInline.
+
+ But this is all a pile of nonsense. The debug support to selectively disable compilation of
+ some CodeBlocks shouldn't cross-cut our entire engine and should most certainly never involve
+ adding new capability modes. This support is a hack at best and is for use by JSC hackers
+ only. It should be as unintrusive as possible.
+
+ So, as in the ancient times, the only proper place to put this hack is in DFGDriver.cpp, and
+ return CompilationFailed. This is correct not just because it takes capability modes out of
+ the picture (and obviates the need to introduce new ones), but also because it means that
+ disabling compilation doesn't change the profiling mode of other CodeBlocks in the Baseline
+ JIT. Capability mode influences profiling mode which in turn influences code generation in
+ the Baseline JIT, sometimes in very significant ways - like, we sometimes do additional
+ double-to-int conversions in Baseline if we know that we might tier-up into the DFG, since
+ this buys us more precise profiling.
+
+ This change reduces the intrusiveness of debugging hacks by making them use the very simple
+ CompilationFailed mechanism rather than trying to influence capability modes. Capability
+ modes have very subtle effects on the whole engine, while CompilationFailed just makes the
+ engine pretend like the DFG compilation will happen at timelike infinity. That makes these
+ hacks much more likely to continue working as we make other changes to the system.
+
+ This brings back the ability to bisect down onto a function bar when bar inlines foo. Prior
+ to this change, we would crash in that case.
+
+ * dfg/DFGCapabilities.cpp:
+ (JSC::DFG::isSupported):
+ (JSC::DFG::mightCompileEval):
+ (JSC::DFG::mightCompileProgram):
+ (JSC::DFG::mightCompileFunctionForCall):
+ (JSC::DFG::mightCompileFunctionForConstruct):
+ * dfg/DFGCapabilities.h:
+ * dfg/DFGDriver.cpp:
+ (JSC::DFG::compileImpl):
+
</ins><span class="cx"> 2015-03-03 peavo@outlook.com <peavo@outlook.com>
</span><span class="cx">
</span><span class="cx"> [Win64] JSC compile error.
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoredfgDFGCapabilitiescpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/dfg/DFGCapabilities.cpp (180955 => 180956)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/dfg/DFGCapabilities.cpp 2015-03-03 22:12:49 UTC (rev 180955)
+++ trunk/Source/JavaScriptCore/dfg/DFGCapabilities.cpp 2015-03-03 22:35:14 UTC (rev 180956)
</span><span class="lines">@@ -30,19 +30,16 @@
</span><span class="cx">
</span><span class="cx"> #include "CodeBlock.h"
</span><span class="cx"> #include "DFGCommon.h"
</span><del>-#include "DFGFunctionWhitelist.h"
</del><span class="cx"> #include "Interpreter.h"
</span><span class="cx"> #include "JSCInlines.h"
</span><span class="cx"> #include "Options.h"
</span><span class="cx">
</span><span class="cx"> namespace JSC { namespace DFG {
</span><span class="cx">
</span><del>-bool isSupported(CodeBlock* codeBlock)
</del><ins>+bool isSupported()
</ins><span class="cx"> {
</span><span class="cx"> return Options::useDFGJIT()
</span><del>- && MacroAssembler::supportsFloatingPoint()
- && Options::bytecodeRangeToDFGCompile().isInRange(codeBlock->instructionCount())
- && FunctionWhitelist::ensureGlobalWhitelist().contains(codeBlock);
</del><ins>+ && MacroAssembler::supportsFloatingPoint();
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> bool isSupportedForInlining(CodeBlock* codeBlock)
</span><span class="lines">@@ -53,22 +50,22 @@
</span><span class="cx">
</span><span class="cx"> bool mightCompileEval(CodeBlock* codeBlock)
</span><span class="cx"> {
</span><del>- return isSupported(codeBlock)
</del><ins>+ return isSupported()
</ins><span class="cx"> && codeBlock->instructionCount() <= Options::maximumOptimizationCandidateInstructionCount();
</span><span class="cx"> }
</span><span class="cx"> bool mightCompileProgram(CodeBlock* codeBlock)
</span><span class="cx"> {
</span><del>- return isSupported(codeBlock)
</del><ins>+ return isSupported()
</ins><span class="cx"> && codeBlock->instructionCount() <= Options::maximumOptimizationCandidateInstructionCount();
</span><span class="cx"> }
</span><span class="cx"> bool mightCompileFunctionForCall(CodeBlock* codeBlock)
</span><span class="cx"> {
</span><del>- return isSupported(codeBlock)
</del><ins>+ return isSupported()
</ins><span class="cx"> && codeBlock->instructionCount() <= Options::maximumOptimizationCandidateInstructionCount();
</span><span class="cx"> }
</span><span class="cx"> bool mightCompileFunctionForConstruct(CodeBlock* codeBlock)
</span><span class="cx"> {
</span><del>- return isSupported(codeBlock)
</del><ins>+ return isSupported()
</ins><span class="cx"> && codeBlock->instructionCount() <= Options::maximumOptimizationCandidateInstructionCount();
</span><span class="cx"> }
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoredfgDFGCapabilitiesh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/dfg/DFGCapabilities.h (180955 => 180956)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/dfg/DFGCapabilities.h 2015-03-03 22:12:49 UTC (rev 180955)
+++ trunk/Source/JavaScriptCore/dfg/DFGCapabilities.h 2015-03-03 22:35:14 UTC (rev 180956)
</span><span class="lines">@@ -38,7 +38,7 @@
</span><span class="cx"> #if ENABLE(DFG_JIT)
</span><span class="cx"> // Fast check functions; if they return true it is still necessary to
</span><span class="cx"> // check opcodes.
</span><del>-bool isSupported(CodeBlock*);
</del><ins>+bool isSupported();
</ins><span class="cx"> bool isSupportedForInlining(CodeBlock*);
</span><span class="cx"> bool mightCompileEval(CodeBlock*);
</span><span class="cx"> bool mightCompileProgram(CodeBlock*);
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoredfgDFGDrivercpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/dfg/DFGDriver.cpp (180955 => 180956)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/dfg/DFGDriver.cpp 2015-03-03 22:12:49 UTC (rev 180955)
+++ trunk/Source/JavaScriptCore/dfg/DFGDriver.cpp 2015-03-03 22:35:14 UTC (rev 180956)
</span><span class="lines">@@ -30,6 +30,7 @@
</span><span class="cx"> #include "JSString.h"
</span><span class="cx">
</span><span class="cx"> #include "CodeBlock.h"
</span><ins>+#include "DFGFunctionWhitelist.h"
</ins><span class="cx"> #include "DFGJITCode.h"
</span><span class="cx"> #include "DFGPlan.h"
</span><span class="cx"> #include "DFGThunks.h"
</span><span class="lines">@@ -62,6 +63,10 @@
</span><span class="cx"> {
</span><span class="cx"> SamplingRegion samplingRegion("DFG Compilation (Driver)");
</span><span class="cx">
</span><ins>+ if (!Options::bytecodeRangeToDFGCompile().isInRange(codeBlock->instructionCount())
+ || !FunctionWhitelist::ensureGlobalWhitelist().contains(codeBlock))
+ return CompilationFailed;
+
</ins><span class="cx"> numCompilations++;
</span><span class="cx">
</span><span class="cx"> ASSERT(codeBlock);
</span></span></pre>
</div>
</div>
</body>
</html>