We are too conservative about the effects of PushWithScope
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 16 Aug 2017 02:49:04 +0000 (02:49 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 16 Aug 2017 02:49:04 +0000 (02:49 +0000)
https://bugs.webkit.org/show_bug.cgi?id=175584

Patch by Robin Morisset <rmorisset@apple.com> on 2017-08-15
Reviewed by Saam Barati.

PushWithScope converts its argument to an object (this can throw a type error,
but has no other observable effect), and allocates a new scope, that it then
makes the new current scope. We were a bit too
conservative in saying that it clobbers the world.

* dfg/DFGAbstractInterpreterInlines.h:
(JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
* dfg/DFGClobberize.h:
(JSC::DFG::clobberize):
* dfg/DFGDoesGC.cpp:
(JSC::DFG::doesGC):

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

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h
Source/JavaScriptCore/dfg/DFGClobberize.h
Source/JavaScriptCore/dfg/DFGDoesGC.cpp

index da5a42564ecff1b1f39010653032f334cb90e056..87c178fb93ddb16293691b3148dfe9e9e24c42ab 100644 (file)
@@ -1,3 +1,22 @@
+2017-08-15  Robin Morisset  <rmorisset@apple.com>
+
+        We are too conservative about the effects of PushWithScope
+        https://bugs.webkit.org/show_bug.cgi?id=175584
+
+        Reviewed by Saam Barati.
+
+        PushWithScope converts its argument to an object (this can throw a type error,
+        but has no other observable effect), and allocates a new scope, that it then
+        makes the new current scope. We were a bit too
+        conservative in saying that it clobbers the world.
+
+        * dfg/DFGAbstractInterpreterInlines.h:
+        (JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
+        * dfg/DFGClobberize.h:
+        (JSC::DFG::clobberize):
+        * dfg/DFGDoesGC.cpp:
+        (JSC::DFG::doesGC):
+
 2017-08-15  Ryosuke Niwa  <rniwa@webkit.org>
 
         Make DataTransferItemList work with plain text entries
index 8a2986b89d850342aadeb5aa98214aac1a4f29d0..4d1e0a7e328d755cb489301b87e2db51d7ead5bd 100644 (file)
@@ -2064,8 +2064,6 @@ bool AbstractInterpreter<AbstractStateType>::executeEffects(unsigned clobberLimi
     }
 
     case PushWithScope:
-        clobberWorld(node->origin.semantic, clobberLimit);
-
         // We don't use the more precise withScopeStructure() here because it is a LazyProperty and may not yet be allocated.
         forNode(node).setType(m_graph, SpecObjectOther);
         break;
index 3feab350639778144f0c9e3fc76c1e566321521d..438b9207bbccb8e00f9d1284faddeb831e9ab134 100644 (file)
@@ -471,6 +471,12 @@ void clobberize(Graph& graph, Node* node, const ReadFunctor& read, const WriteFu
         write(SideState);
         return;
 
+    case PushWithScope: {
+        read(World);
+        write(HeapObjectCount);
+        return;
+    }
+
     case CreateActivation: {
         SymbolTable* table = node->castOperand<SymbolTable*>();
         if (table->singletonScope()->isStillValid())
@@ -605,7 +611,6 @@ void clobberize(Graph& graph, Node* node, const ReadFunctor& read, const WriteFu
     case PutDynamicVar:
     case ResolveScopeForHoistingFuncDeclInEval:
     case ResolveScope:
-    case PushWithScope:
         read(World);
         write(Heap);
         return;
index c5edb55416ee543bc511f6f094e374f38dbd02fa..b4adbfdd08adb355fd67722aa025736c338638d4 100644 (file)
@@ -280,9 +280,9 @@ bool doesGC(Graph& graph, Node* node)
     case AtomicsSub:
     case AtomicsXor:
     case AtomicsIsLockFree:
-    case PushWithScope:
         return false;
 
+    case PushWithScope:
     case CreateActivation:
     case CreateDirectArguments:
     case CreateScopedArguments: