<1/100 probability FTL failure: v8-v6/v8-deltablue.js.ftl-eager: Exception: TypeError...
authorfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 10 Feb 2014 17:04:28 +0000 (17:04 +0000)
committerfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 10 Feb 2014 17:04:28 +0000 (17:04 +0000)
https://bugs.webkit.org/show_bug.cgi?id=128278

Reviewed by Mark Hahnenberg.

Fix another FTL flake due to bytecode liveness corner cases. Hopefully it's the last
one.

* dfg/DFGByteCodeParser.cpp:
(JSC::DFG::ByteCodeParser::parseBlock): Make sure that inside a constructor, the 'this' result is always set. This makes it easier to unify the treatment of 'this' for OSR exit: we just say that it's always live.
* dfg/DFGGraph.cpp:
(JSC::DFG::Graph::isLiveInBytecode): Assume that 'this' is live. We were already sort of doing this for calls because the callsite would claim it to be live. But we didn't do it for constructors. It's true that *at the callsite* 'this' won't be live, but inside the inlined constructor, it almost certainly will be.
* dfg/DFGTierUpCheckInjectionPhase.cpp:
(JSC::DFG::TierUpCheckInjectionPhase::run): I just noticed this benign bug. We should only return 'true' if we actually injected checks.
* ftl/FTLOSRExitCompiler.cpp:
(JSC::FTL::compileStub): Make it easier to just dump disassembly for FTL OSR exits.
* runtime/Options.h: Ditto.
* tests/stress/inlined-constructor-this-liveness.js: Added.
(Foo):
(foo):
* tests/stress/inlined-function-this-liveness.js: Added.
(bar):
(foo):

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

PerformanceTests/SunSpider/tests/v8-v6/v8-deltablue.js
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
Source/JavaScriptCore/dfg/DFGGraph.cpp
Source/JavaScriptCore/dfg/DFGTierUpCheckInjectionPhase.cpp
Source/JavaScriptCore/ftl/FTLOSRExitCompiler.cpp
Source/JavaScriptCore/runtime/Options.h
Source/JavaScriptCore/tests/stress/inlined-constructor-this-liveness.js [new file with mode: 0644]
Source/JavaScriptCore/tests/stress/inlined-function-this-liveness.js [new file with mode: 0644]

index 5c3e233..55504f4 100644 (file)
@@ -669,6 +669,8 @@ Planner.prototype.extractPlanFromConstraints = function (constraints) {
   return this.makePlan(sources);
 }
 
+noInline(Planner.prototype.extractPlanFromConstraints);
+
 /**
  * Recompute the walkabout strengths and stay flags of all variables
  * downstream of the given constraint and recompute the actual
index 62b1f6a..72b7919 100644 (file)
@@ -1,5 +1,31 @@
 2014-02-10  Filip Pizlo  <fpizlo@apple.com>
 
+        <1/100 probability FTL failure: v8-v6/v8-deltablue.js.ftl-eager: Exception: TypeError: undefined is not an object (evaluating 'c.isInput')
+        https://bugs.webkit.org/show_bug.cgi?id=128278
+
+        Reviewed by Mark Hahnenberg.
+        
+        Fix another FTL flake due to bytecode liveness corner cases. Hopefully it's the last
+        one.
+
+        * dfg/DFGByteCodeParser.cpp:
+        (JSC::DFG::ByteCodeParser::parseBlock): Make sure that inside a constructor, the 'this' result is always set. This makes it easier to unify the treatment of 'this' for OSR exit: we just say that it's always live.
+        * dfg/DFGGraph.cpp:
+        (JSC::DFG::Graph::isLiveInBytecode): Assume that 'this' is live. We were already sort of doing this for calls because the callsite would claim it to be live. But we didn't do it for constructors. It's true that *at the callsite* 'this' won't be live, but inside the inlined constructor, it almost certainly will be.
+        * dfg/DFGTierUpCheckInjectionPhase.cpp:
+        (JSC::DFG::TierUpCheckInjectionPhase::run): I just noticed this benign bug. We should only return 'true' if we actually injected checks.
+        * ftl/FTLOSRExitCompiler.cpp:
+        (JSC::FTL::compileStub): Make it easier to just dump disassembly for FTL OSR exits.
+        * runtime/Options.h: Ditto.
+        * tests/stress/inlined-constructor-this-liveness.js: Added.
+        (Foo):
+        (foo):
+        * tests/stress/inlined-function-this-liveness.js: Added.
+        (bar):
+        (foo):
+
+2014-02-10  Filip Pizlo  <fpizlo@apple.com>
+
         Actually register those DFG::Safepoints
         https://bugs.webkit.org/show_bug.cgi?id=128521
 
index 84cba4d..1efdd56 100644 (file)
@@ -1956,6 +1956,8 @@ bool ByteCodeParser::parseBlock(unsigned limit)
             // Initialize all locals to undefined.
             for (int i = 0; i < m_inlineStackTop->m_codeBlock->m_numVars; ++i)
                 set(virtualRegisterForLocal(i), constantUndefined(), ImmediateSet);
+            if (m_inlineStackTop->m_codeBlock->specializationKind() == CodeForConstruct)
+                set(virtualRegisterForArgument(0), constantUndefined(), ImmediateSet);
             NEXT_OPCODE(op_enter);
             
         case op_touch_entry:
index 388bb0d..b1c13db 100644 (file)
@@ -711,8 +711,9 @@ bool Graph::isLiveInBytecode(VirtualRegister operand, CodeOrigin codeOrigin)
 
         // Arguments are always live. This would be redundant if it wasn't for our
         // op_call_varargs inlining.
+        // FIXME: 'this' might not be live, but we don't have a way of knowing.
+        // https://bugs.webkit.org/show_bug.cgi?id=128519
         if (reg.isArgument()
-            && reg.toArgument()
             && static_cast<size_t>(reg.toArgument()) < inlineCallFrame->arguments.size())
             return true;
         
index ff6385c..82c8c04 100644 (file)
@@ -52,7 +52,7 @@ public:
             return false;
         
         if (m_graph.m_profiledBlock->m_didFailFTLCompilation)
-            return true;
+            return false;
         
 #if ENABLE(FTL_JIT)
         FTL::CapabilityLevel level = FTL::canCompile(m_graph);
index 502d11d..3763cc9 100644 (file)
@@ -341,7 +341,7 @@ static void compileStub(
     
     LinkBuffer patchBuffer(*vm, &jit, codeBlock);
     exit.m_code = FINALIZE_CODE_IF(
-        shouldShowDisassembly() || Options::verboseOSR(),
+        shouldShowDisassembly() || Options::verboseOSR() || Options::verboseFTLOSRExit(),
         patchBuffer,
         ("FTL OSR exit #%u (%s, %s) from %s, with operands = %s, and record = %s",
             exitID, toCString(exit.m_codeOrigin).data(),
index 97c4486..a472c75 100644 (file)
@@ -122,6 +122,7 @@ typedef OptionRange optionRange;
     v(bool, validateGraph, false) \
     v(bool, validateGraphAtEachPhase, false) \
     v(bool, verboseOSR, false) \
+    v(bool, verboseFTLOSRExit, false) \
     v(bool, verboseCallLink, false) \
     v(bool, verboseCompilationQueue, false) \
     v(bool, reportCompileTimes, false) \
diff --git a/Source/JavaScriptCore/tests/stress/inlined-constructor-this-liveness.js b/Source/JavaScriptCore/tests/stress/inlined-constructor-this-liveness.js
new file mode 100644 (file)
index 0000000..bded332
--- /dev/null
@@ -0,0 +1,24 @@
+function Foo(a, b) {
+    this.f = a.f;
+    this.g = b.f + 1;
+}
+
+function foo(a, b) {
+    return new Foo(a, b);
+}
+
+noInline(foo);
+
+for (var i = 0; i < 100000; ++i) {
+    var result = foo({f:1}, {f:2});
+    if (result.f != 1)
+        throw "Error: bad result.f: " + result.f;
+    if (result.g != 3)
+        throw "Error: bad result.g: " + result.g;
+}
+
+var result = foo({f:1}, {f:2.5});
+if (result.f != 1)
+    throw "Error: bad result.f: " + result.f;
+if (result.g != 3.5)
+    throw "Error: bad result.f: " + result.g;
diff --git a/Source/JavaScriptCore/tests/stress/inlined-function-this-liveness.js b/Source/JavaScriptCore/tests/stress/inlined-function-this-liveness.js
new file mode 100644 (file)
index 0000000..5d667c6
--- /dev/null
@@ -0,0 +1,26 @@
+function bar(a, b) {
+    this.f = a.f;
+    this.g = b.f + 1;
+    return this;
+}
+
+function foo(a, b) {
+    var o = {f:bar};
+    return o.f(a, b);
+}
+
+noInline(foo);
+
+for (var i = 0; i < 100000; ++i) {
+    var result = foo({f:1}, {f:2});
+    if (result.f != 1)
+        throw "Error: bad result.f: " + result.f;
+    if (result.g != 3)
+        throw "Error: bad result.g: " + result.g;
+}
+
+var result = foo({f:1}, {f:2.5});
+if (result.f != 1)
+    throw "Error: bad result.f: " + result.f;
+if (result.g != 3.5)
+    throw "Error: bad result.f: " + result.g;