SpeculativeJIT::compileTryGetById needs to pass in NeedsToSpill along both the cell...
authorsbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 21 Oct 2016 22:38:22 +0000 (22:38 +0000)
committersbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 21 Oct 2016 22:38:22 +0000 (22:38 +0000)
https://bugs.webkit.org/show_bug.cgi?id=163622

Reviewed by Yusuke Suzuki.

JSTests:

* stress/try-get-by-id-should-spill-registers-dfg.js: Added.
(let.f.createBuiltin):

Source/JavaScriptCore:

We were passing in DontSpill in the Untyped:child1() case, which caused us
not to spill the base register. This is obviously wrong because the
DFG's register allocator expected the base to still be in the register
that it allocated for it after the TryGetById node executed.

* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::compileTryGetById):

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

JSTests/ChangeLog
JSTests/stress/try-get-by-id-should-spill-registers-dfg.js [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp

index 8789445..8a9a89d 100644 (file)
@@ -1,3 +1,13 @@
+2016-10-21  Saam Barati  <sbarati@apple.com>
+
+        SpeculativeJIT::compileTryGetById needs to pass in NeedsToSpill along both the cell speculation and untyped speculation path
+        https://bugs.webkit.org/show_bug.cgi?id=163622
+
+        Reviewed by Yusuke Suzuki.
+
+        * stress/try-get-by-id-should-spill-registers-dfg.js: Added.
+        (let.f.createBuiltin):
+
 2016-10-21  Caitlin Potter  <caitp@igalia.com>
 
         [JSC] don't crash when arguments to `new Function()` produce unexpected AST
diff --git a/JSTests/stress/try-get-by-id-should-spill-registers-dfg.js b/JSTests/stress/try-get-by-id-should-spill-registers-dfg.js
new file mode 100644 (file)
index 0000000..48c4559
--- /dev/null
@@ -0,0 +1,10 @@
+let f = createBuiltin(`(function (arg) { 
+    let r = @tryGetById(arg, "prototype");
+    if (arg !== true) throw new @Error("Bad clobber of arg");
+    return r;
+})`);
+noInline(f);
+
+for (let i = 0; i < 10000; i++) {
+    f(true);
+}
index f310343..b8b1426 100644 (file)
@@ -1,3 +1,18 @@
+2016-10-21  Saam Barati  <sbarati@apple.com>
+
+        SpeculativeJIT::compileTryGetById needs to pass in NeedsToSpill along both the cell speculation and untyped speculation path
+        https://bugs.webkit.org/show_bug.cgi?id=163622
+
+        Reviewed by Yusuke Suzuki.
+
+        We were passing in DontSpill in the Untyped:child1() case, which caused us
+        not to spill the base register. This is obviously wrong because the
+        DFG's register allocator expected the base to still be in the register
+        that it allocated for it after the TryGetById node executed.
+
+        * dfg/DFGSpeculativeJIT.cpp:
+        (JSC::DFG::SpeculativeJIT::compileTryGetById):
+
 2016-10-21  Keith Miller  <keith_miller@apple.com>
 
         Expand Trunc in B3 to support Double to Float
index ae84563..5e64baf 100644 (file)
@@ -1020,7 +1020,7 @@ void SpeculativeJIT::compileTryGetById(Node* node)
 
         JITCompiler::Jump notCell = m_jit.branchIfNotCell(baseRegs);
 
-        cachedGetById(node->origin.semantic, baseRegs, resultRegs, node->identifierNumber(), notCell, DontSpill, AccessType::GetPure);
+        cachedGetById(node->origin.semantic, baseRegs, resultRegs, node->identifierNumber(), notCell, NeedToSpill, AccessType::GetPure);
 
         jsValueResult(resultRegs, node, DataFormatJS, UseChildrenCalledExplicitly);
         break;