Changeset 194820 in webkit

Timestamp:

Jan 9, 2016 7:23:56 AM (8 years ago)

Author:

mark.lam@apple.com

Message:

The FTL allocated spill slots for BinaryOps is sometimes inaccurate.
​https://bugs.webkit.org/show_bug.cgi?id=152918

Reviewed by Filip Pizlo and Saam Barati.

• ftl/FTLCompile.cpp:
• Updated a comment.
• ftl/FTLLowerDFGToLLVM.cpp:

(JSC::FTL::DFG::LowerDFGToLLVM::lower):

• The code to compute maxNumberOfCatchSpills was unnecessarily allocating an extra slot for BinaryOps that don't have Untyped operands, and failing to allocate that extra slot for some binary ops. This is now fixed.

• tests/stress/ftl-shr-exception.js:
• tests/stress/ftl-xor-exception.js:
• Un-skipped these tests. They now pass with this patch.

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• ftl/FTLCompile.cpp (modified) (1 diff)
• ftl/FTLLowerDFGToLLVM.cpp (modified) (1 diff)
• tests/stress/ftl-shr-exception.js (modified) (1 diff)
• tests/stress/ftl-xor-exception.js (modified) (1 diff)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2016-01-09  Mark Lam  <mark.lam@apple.com>
+
+        The FTL allocated spill slots for BinaryOps is sometimes inaccurate.
+        https://bugs.webkit.org/show_bug.cgi?id=152918
+
+        Reviewed by Filip Pizlo and Saam Barati.
+
+        * ftl/FTLCompile.cpp:
+        - Updated a comment.
+        * ftl/FTLLowerDFGToLLVM.cpp:
+        (JSC::FTL::DFG::LowerDFGToLLVM::lower):
+        - The code to compute maxNumberOfCatchSpills was unnecessarily allocating an
+          extra slot for BinaryOps that don't have Untyped operands, and failing to
+          allocate that extra slot for some binary ops.  This is now fixed.
+
+        * tests/stress/ftl-shr-exception.js:
+        * tests/stress/ftl-xor-exception.js:
+        - Un-skipped these tests.  They now pass with this patch.
+
 2016-01-09  Andreas Kling  <akling@apple.com>
 

trunk/Source/JavaScriptCore/ftl/FTLCompile.cpp

@@ -544 +544 @@
                 }
 
-                // Subs and GetByIds have an interesting register preservation story,
+                // BinaryOps and GetByIds have an interesting register preservation story,
                 // see comment below at GetById to read about it.
                 //

trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp

@@ -263 +263 @@
                     case ArithMul:
                     case ArithSub:
+                    case ValueAdd:
+                    case DFG::BitAnd:
+                    case DFG::BitOr:
+                    case DFG::BitXor:
+                    case BitLShift:
+                    case BitRShift:
+                    case BitURShift:
+                        if (!node->isBinaryUseKind(UntypedUse))
+                            break; // We only compile patchpoints for UntypedUse.
+                        FALLTHROUGH;
+
                     case GetById:
-                    case GetByIdFlush:
-                    case ValueAdd: {
-                        // We may have to flush one thing for GetByIds/ArithSubs when the base and result or the left/right and the result
+                    case GetByIdFlush: {
+                        // We may have to flush one thing for GetByIds/ binary ops when the base and result or the left/right and the result
                         // are assigned the same register. For a more comprehensive overview, look at the comment in FTLCompile.cpp
-                        if (node->op() == ArithSub && node->binaryUseKind() != UntypedUse)
-                            break; // We only compile patchpoints for ArithSub UntypedUse.
                         CodeOrigin opCatchOrigin;
                         HandlerInfo* exceptionHandler;

trunk/Source/JavaScriptCore/tests/stress/ftl-shr-exception.js

@@ -1 @@
-//@ skip
-
 function foo(a, b) {
     try {

trunk/Source/JavaScriptCore/tests/stress/ftl-xor-exception.js

@@ -1 @@
-//@ skip
-
 function foo(a, b) {
     try {