StringObjectUse should not be a structure check for the original string object structure
authorsbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 17 Jan 2019 17:50:27 +0000 (17:50 +0000)
committersbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 17 Jan 2019 17:50:27 +0000 (17:50 +0000)
https://bugs.webkit.org/show_bug.cgi?id=193483
<rdar://problem/47280522>

Reviewed by Yusuke Suzuki.

JSTests:

* stress/cant-eliminate-string-object-structure-check-when-string-object-is-proven.js: Added.
(foo):
(a.valueOf.0):

Source/JavaScriptCore:

Prior to this patch, the use kind for StringObjectUse implied that we
do a StructureCheck on the input operand for the *original* StringObject
structure. This is generally not how we use UseKinds, so it's no surprise
that this is buggy. A UseKind should map to a set of SpeculatedTypes, not an
actual set of structures. This patch changes the meaning of StringObjectUse
to mean an object where jsDynamicCast<StringObject*> would succeed.

This patch also fixes a bug that was caused by the old and weird usage of the
UseKind to mean StructureCheck. Consider a program like this:
```
S1 = Original StringObject structure
S2 = Original StringObject structure with the field "f" added

a: GetLocal()
b: CheckStructure(@a, {S2})
c: ToString(StringObject:@a)
```

According to AI, in the above program, we would exit at @c, since
StringObject:@a implies a structure check of {S1}, and the intersection
of {S1} and {S2} is {}. So, we'd convert the program to be:
```
a: GetLocal()
b: CheckStructure(@a, {S2})
c: Check(StringObject:@a)
d: Unreachable
```

However, AI would set the proof status of the StringObject:@a edge
to be proven, since the SpeculatedType for @a is SpecStringObject.
This was incorrect of AI to do because the SpeculatedType itself
didn't capture the full power of StringObjectUse. However, having
a UseKind mean CheckStructure is weird precisely because what AI was
doing is a natural fit to how we typically we think about UseKinds.

So the above program would then incorrectly be converted to this, and
we'd crash when reaching the Unreachable node:
```
a: GetLocal()
b: CheckStructure(@a, {S2})
d: Unreachable
```

This patch makes it so that StringObjectUse just means that the object that
filters through a StringObjectUse check must !!jsDynamicCast<StringObject*>.
This is now in line with all other UseKinds. It also lets us simplify a bunch
of other code that had weird checks for the StringObjectUse UseKind.

This patch also makes it so that anywhere where we used to rely on
StringObjectUse implying a structure check we actually emit an explicit
CheckStructure node.

* JavaScriptCore.xcodeproj/project.pbxproj:
* bytecode/ExitKind.cpp:
(JSC::exitKindToString):
* bytecode/ExitKind.h:
* dfg/DFGAbstractInterpreterInlines.h:
(JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
* dfg/DFGCSEPhase.cpp:
* dfg/DFGClobberize.h:
(JSC::DFG::clobberize):
* dfg/DFGEdgeUsesStructure.h: Removed.
* dfg/DFGFixupPhase.cpp:
(JSC::DFG::FixupPhase::attemptToForceStringArrayModeByToStringConversion):
(JSC::DFG::FixupPhase::addCheckStructureForOriginalStringObjectUse):
(JSC::DFG::FixupPhase::fixupToPrimitive):
(JSC::DFG::FixupPhase::fixupToStringOrCallStringConstructor):
(JSC::DFG::FixupPhase::attemptToMakeFastStringAdd):
(JSC::DFG::FixupPhase::isStringObjectUse): Deleted.
* dfg/DFGGraph.cpp:
(JSC::DFG::Graph::canOptimizeStringObjectAccess):
* dfg/DFGMayExit.cpp:
* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::compileToStringOrCallStringConstructorOrStringValueOf):
(JSC::DFG::SpeculativeJIT::speculateStringObject):
(JSC::DFG::SpeculativeJIT::speculateStringOrStringObject):
* dfg/DFGSpeculativeJIT.h:
(JSC::DFG::SpeculativeJIT::speculateStringObjectForStructure): Deleted.
* dfg/DFGUseKind.h:
(JSC::DFG::alreadyChecked):
(JSC::DFG::usesStructure): Deleted.
* ftl/FTLLowerDFGToB3.cpp:
(JSC::FTL::DFG::LowerDFGToB3::compileToStringOrCallStringConstructorOrStringValueOf):
(JSC::FTL::DFG::LowerDFGToB3::speculateStringObject):
(JSC::FTL::DFG::LowerDFGToB3::speculateStringOrStringObject):
(JSC::FTL::DFG::LowerDFGToB3::speculateStringObjectForCell):
(JSC::FTL::DFG::LowerDFGToB3::speculateStringObjectForStructureID): Deleted.
* runtime/JSType.cpp:
(WTF::printInternal):
* runtime/JSType.h:
* runtime/StringObject.h:
(JSC::StringObject::createStructure):
* runtime/StringPrototype.h:

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

21 files changed:
JSTests/ChangeLog
JSTests/stress/cant-eliminate-string-object-structure-check-when-string-object-is-proven.js [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj
Source/JavaScriptCore/bytecode/ExitKind.cpp
Source/JavaScriptCore/bytecode/ExitKind.h
Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h
Source/JavaScriptCore/dfg/DFGCSEPhase.cpp
Source/JavaScriptCore/dfg/DFGClobberize.h
Source/JavaScriptCore/dfg/DFGEdgeUsesStructure.h [deleted file]
Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
Source/JavaScriptCore/dfg/DFGGraph.cpp
Source/JavaScriptCore/dfg/DFGMayExit.cpp
Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h
Source/JavaScriptCore/dfg/DFGUseKind.h
Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
Source/JavaScriptCore/runtime/JSType.cpp
Source/JavaScriptCore/runtime/JSType.h
Source/JavaScriptCore/runtime/StringObject.h
Source/JavaScriptCore/runtime/StringPrototype.h

index e7b396d..777a853 100644 (file)
@@ -1,3 +1,15 @@
+2019-01-17  Saam barati  <sbarati@apple.com>
+
+        StringObjectUse should not be a structure check for the original string object structure
+        https://bugs.webkit.org/show_bug.cgi?id=193483
+        <rdar://problem/47280522>
+
+        Reviewed by Yusuke Suzuki.
+
+        * stress/cant-eliminate-string-object-structure-check-when-string-object-is-proven.js: Added.
+        (foo):
+        (a.valueOf.0):
+
 2019-01-17  Yusuke Suzuki  <yusukesuzuki@slowstart.org>
 
         [JSC] ToThis omission in DFGByteCodeParser is wrong
diff --git a/JSTests/stress/cant-eliminate-string-object-structure-check-when-string-object-is-proven.js b/JSTests/stress/cant-eliminate-string-object-structure-check-when-string-object-is-proven.js
new file mode 100644 (file)
index 0000000..eb20e5f
--- /dev/null
@@ -0,0 +1,11 @@
+//@ runDefault("--forceEagerCompilation=1", "--useConcurrentJIT=0")
+
+function foo(x) {
+    x.toString();
+}
+
+var a = new String();
+a.valueOf = 0
+for (var i = 0; i < 5; i++) {
+    foo(a)
+}
index b8a570e..f31d03e 100644 (file)
@@ -1,3 +1,105 @@
+2019-01-17  Saam barati  <sbarati@apple.com>
+
+        StringObjectUse should not be a structure check for the original string object structure
+        https://bugs.webkit.org/show_bug.cgi?id=193483
+        <rdar://problem/47280522>
+
+        Reviewed by Yusuke Suzuki.
+
+        Prior to this patch, the use kind for StringObjectUse implied that we
+        do a StructureCheck on the input operand for the *original* StringObject
+        structure. This is generally not how we use UseKinds, so it's no surprise
+        that this is buggy. A UseKind should map to a set of SpeculatedTypes, not an
+        actual set of structures. This patch changes the meaning of StringObjectUse
+        to mean an object where jsDynamicCast<StringObject*> would succeed.
+        
+        This patch also fixes a bug that was caused by the old and weird usage of the
+        UseKind to mean StructureCheck. Consider a program like this:
+        ```
+        S1 = Original StringObject structure
+        S2 = Original StringObject structure with the field "f" added
+        
+        a: GetLocal()
+        b: CheckStructure(@a, {S2})
+        c: ToString(StringObject:@a)
+        ```
+        
+        According to AI, in the above program, we would exit at @c, since
+        StringObject:@a implies a structure check of {S1}, and the intersection
+        of {S1} and {S2} is {}. So, we'd convert the program to be:
+        ```
+        a: GetLocal()
+        b: CheckStructure(@a, {S2})
+        c: Check(StringObject:@a)
+        d: Unreachable
+        ```
+        
+        However, AI would set the proof status of the StringObject:@a edge
+        to be proven, since the SpeculatedType for @a is SpecStringObject.
+        This was incorrect of AI to do because the SpeculatedType itself
+        didn't capture the full power of StringObjectUse. However, having
+        a UseKind mean CheckStructure is weird precisely because what AI was
+        doing is a natural fit to how we typically we think about UseKinds.
+        
+        So the above program would then incorrectly be converted to this, and
+        we'd crash when reaching the Unreachable node:
+        ```
+        a: GetLocal()
+        b: CheckStructure(@a, {S2})
+        d: Unreachable
+        ```
+        
+        This patch makes it so that StringObjectUse just means that the object that
+        filters through a StringObjectUse check must !!jsDynamicCast<StringObject*>.
+        This is now in line with all other UseKinds. It also lets us simplify a bunch
+        of other code that had weird checks for the StringObjectUse UseKind.
+        
+        This patch also makes it so that anywhere where we used to rely on
+        StringObjectUse implying a structure check we actually emit an explicit
+        CheckStructure node.
+
+        * JavaScriptCore.xcodeproj/project.pbxproj:
+        * bytecode/ExitKind.cpp:
+        (JSC::exitKindToString):
+        * bytecode/ExitKind.h:
+        * dfg/DFGAbstractInterpreterInlines.h:
+        (JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
+        * dfg/DFGCSEPhase.cpp:
+        * dfg/DFGClobberize.h:
+        (JSC::DFG::clobberize):
+        * dfg/DFGEdgeUsesStructure.h: Removed.
+        * dfg/DFGFixupPhase.cpp:
+        (JSC::DFG::FixupPhase::attemptToForceStringArrayModeByToStringConversion):
+        (JSC::DFG::FixupPhase::addCheckStructureForOriginalStringObjectUse):
+        (JSC::DFG::FixupPhase::fixupToPrimitive):
+        (JSC::DFG::FixupPhase::fixupToStringOrCallStringConstructor):
+        (JSC::DFG::FixupPhase::attemptToMakeFastStringAdd):
+        (JSC::DFG::FixupPhase::isStringObjectUse): Deleted.
+        * dfg/DFGGraph.cpp:
+        (JSC::DFG::Graph::canOptimizeStringObjectAccess):
+        * dfg/DFGMayExit.cpp:
+        * dfg/DFGSpeculativeJIT.cpp:
+        (JSC::DFG::SpeculativeJIT::compileToStringOrCallStringConstructorOrStringValueOf):
+        (JSC::DFG::SpeculativeJIT::speculateStringObject):
+        (JSC::DFG::SpeculativeJIT::speculateStringOrStringObject):
+        * dfg/DFGSpeculativeJIT.h:
+        (JSC::DFG::SpeculativeJIT::speculateStringObjectForStructure): Deleted.
+        * dfg/DFGUseKind.h:
+        (JSC::DFG::alreadyChecked):
+        (JSC::DFG::usesStructure): Deleted.
+        * ftl/FTLLowerDFGToB3.cpp:
+        (JSC::FTL::DFG::LowerDFGToB3::compileToStringOrCallStringConstructorOrStringValueOf):
+        (JSC::FTL::DFG::LowerDFGToB3::speculateStringObject):
+        (JSC::FTL::DFG::LowerDFGToB3::speculateStringOrStringObject):
+        (JSC::FTL::DFG::LowerDFGToB3::speculateStringObjectForCell):
+        (JSC::FTL::DFG::LowerDFGToB3::speculateStringObjectForStructureID): Deleted.
+        * runtime/JSType.cpp:
+        (WTF::printInternal):
+        * runtime/JSType.h:
+        * runtime/StringObject.h:
+        (JSC::StringObject::createStructure):
+        * runtime/StringPrototype.h:
+
 2019-01-17  Yusuke Suzuki  <yusukesuzuki@slowstart.org>
 
         [JSC] Add generateHeapSnapshotForGCDebugging function to dump GCDebugging data
index df7e6c0..3e744db 100644 (file)
                A78A9781179738D5009DF744 /* FTLJITFinalizer.h in Headers */ = {isa = PBXBuildFile; fileRef = A78A977E179738D5009DF744 /* FTLJITFinalizer.h */; };
                A790DD6E182F499700588807 /* SetIteratorPrototype.h in Headers */ = {isa = PBXBuildFile; fileRef = A790DD68182F499700588807 /* SetIteratorPrototype.h */; };
                A790DD70182F499700588807 /* JSSetIterator.h in Headers */ = {isa = PBXBuildFile; fileRef = A790DD6A182F499700588807 /* JSSetIterator.h */; settings = {ATTRIBUTES = (Private, ); }; };
-               A7986D5717A0BB1E00A95DD0 /* DFGEdgeUsesStructure.h in Headers */ = {isa = PBXBuildFile; fileRef = A7986D5617A0BB1E00A95DD0 /* DFGEdgeUsesStructure.h */; };
                A79D3ED9C5064DD0A8466A3A /* ModuleScopeData.h in Headers */ = {isa = PBXBuildFile; fileRef = 000BEAF0DF604481AF6AB68C /* ModuleScopeData.h */; settings = {ATTRIBUTES = (Private, ); }; };
                A7A8AF3517ADB5F3005AB174 /* ArrayBuffer.h in Headers */ = {isa = PBXBuildFile; fileRef = A7A8AF2617ADB5F3005AB174 /* ArrayBuffer.h */; settings = {ATTRIBUTES = (Private, ); }; };
                A7A8AF3717ADB5F3005AB174 /* ArrayBufferView.h in Headers */ = {isa = PBXBuildFile; fileRef = A7A8AF2817ADB5F3005AB174 /* ArrayBufferView.h */; settings = {ATTRIBUTES = (Private, ); }; };
                A790DD68182F499700588807 /* SetIteratorPrototype.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = SetIteratorPrototype.h; sourceTree = "<group>"; };
                A790DD69182F499700588807 /* JSSetIterator.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = JSSetIterator.cpp; sourceTree = "<group>"; };
                A790DD6A182F499700588807 /* JSSetIterator.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = JSSetIterator.h; sourceTree = "<group>"; };
-               A7986D5617A0BB1E00A95DD0 /* DFGEdgeUsesStructure.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = DFGEdgeUsesStructure.h; path = dfg/DFGEdgeUsesStructure.h; sourceTree = "<group>"; };
                A79E781E15EECBA80047C855 /* UnlinkedCodeBlock.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = UnlinkedCodeBlock.cpp; sourceTree = "<group>"; };
                A79E781F15EECBA80047C855 /* UnlinkedCodeBlock.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = UnlinkedCodeBlock.h; sourceTree = "<group>"; };
                A79EDB0811531CD60019E912 /* JSObjectRefPrivate.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = JSObjectRefPrivate.h; sourceTree = "<group>"; };
                                0FB4B51B16B62772003F696B /* DFGEdge.cpp */,
                                0F66E16914DF3F1300B7B2E4 /* DFGEdge.h */,
                                A7D9A29117A0BC7400EE2618 /* DFGEdgeDominates.h */,
-                               A7986D5617A0BB1E00A95DD0 /* DFGEdgeUsesStructure.h */,
                                0F8F142F1ADF090100ED792C /* DFGEpoch.cpp */,
                                0F8F14301ADF090100ED792C /* DFGEpoch.h */,
                                A78A976C179738B8009DF744 /* DFGFailedFinalizer.cpp */,
                                0FD3C82814115D4F00FD81CB /* DFGDriver.h in Headers */,
                                0F66E16C14DF3F1600B7B2E4 /* DFGEdge.h in Headers */,
                                A7D9A29617A0BC7400EE2618 /* DFGEdgeDominates.h in Headers */,
-                               A7986D5717A0BB1E00A95DD0 /* DFGEdgeUsesStructure.h in Headers */,
                                0F8F14341ADF090100ED792C /* DFGEpoch.h in Headers */,
                                0FBC0AE81496C7C700D4FBDD /* DFGExitProfile.h in Headers */,
                                A78A9775179738B8009DF744 /* DFGFailedFinalizer.h in Headers */,
index ea7eb61..ff27616 100644 (file)
@@ -70,8 +70,6 @@ const char* exitKindToString(ExitKind kind)
         return "ArgumentsEscaped";
     case ExoticObjectMode:
         return "ExoticObjectMode";
-    case NotStringObject:
-        return "NotStringObject";
     case VarargsOverflow:
         return "VarargsOverflow";
     case TDZFailure:
index a6c2e0e..c2f0e32 100644 (file)
@@ -46,7 +46,6 @@ enum ExitKind : uint8_t {
     InadequateCoverage, // We exited because we ended up in code that didn't have profiling coverage.
     ArgumentsEscaped, // We exited because arguments escaped but we didn't expect them to.
     ExoticObjectMode, // We exited because some exotic object that we were accessing was in an exotic mode (like Arguments with slow arguments).
-    NotStringObject, // We exited because we shouldn't have attempted to optimize string object access.
     VarargsOverflow, // We exited because a varargs call passed more arguments than we expected.
     TDZFailure, // We exited because we were in the TDZ and accessed the variable.
     HoistingFailed, // Something that was hoisted exited. So, assume that hoisting is a bad idea.
index f8b49b9..b966adc 100644 (file)
@@ -2421,12 +2421,6 @@ bool AbstractInterpreter<AbstractStateType>::executeEffects(unsigned clobberLimi
     case CallStringConstructor: {
         switch (node->child1().useKind()) {
         case StringObjectUse:
-            // This also filters that the StringObject has the primordial StringObject
-            // structure.
-            filter(
-                node->child1(),
-                m_graph.registerStructure(m_graph.globalObjectFor(node->origin.semantic)->stringObjectStructure()));
-            break;
         case StringOrStringObjectUse:
         case Int32Use:
         case Int52RepUse:
index a5af0f0..2ba5946 100644 (file)
@@ -33,7 +33,6 @@
 #include "DFGClobberSet.h"
 #include "DFGClobberize.h"
 #include "DFGDominators.h"
-#include "DFGEdgeUsesStructure.h"
 #include "DFGGraph.h"
 #include "DFGPhase.h"
 #include "JSCInlines.h"
index 58ec030..9c27073 100644 (file)
@@ -28,7 +28,6 @@
 #if ENABLE(DFG_JIT)
 
 #include "DFGAbstractHeap.h"
-#include "DFGEdgeUsesStructure.h"
 #include "DFGGraph.h"
 #include "DFGHeapLocation.h"
 #include "DFGLazyNode.h"
@@ -107,9 +106,6 @@ void clobberize(Graph& graph, Node* node, const ReadFunctor& read, const WriteFu
     // information that clobberize() passes to write() and def(). Other clients of clobberize() can
     // just ignore def() by using a NoOpClobberize functor.
 
-    if (edgesUseStructure(graph, node))
-        read(JSCell_structureID);
-    
     // We allow the runtime to perform a stack scan at any time. We don't model which nodes get implemented
     // by calls into the runtime. For debugging we might replace the implementation of any node with a call
     // to the runtime, and that call may walk stack. Therefore, each node must read() anything that a stack
@@ -1653,18 +1649,19 @@ void clobberize(Graph& graph, Node* node, const ReadFunctor& read, const WriteFu
     case ToString:
     case CallStringConstructor:
         switch (node->child1().useKind()) {
-        case StringObjectUse:
-        case StringOrStringObjectUse:
-            // These don't def a pure value, unfortunately. I'll avoid load-eliminating these for
-            // now.
-            return;
-            
         case CellUse:
         case UntypedUse:
             read(World);
             write(Heap);
             return;
 
+        case StringObjectUse:
+        case StringOrStringObjectUse:
+            // These two StringObjectUse's are pure because if we emit this node with either
+            // of these UseKinds, we'll first emit a StructureCheck ensuring that we're the
+            // original String or StringObject structure. Therefore, we don't have an overridden
+            // valueOf, etc.
+
         case Int32Use:
         case Int52RepUse:
         case DoubleRepUse:
diff --git a/Source/JavaScriptCore/dfg/DFGEdgeUsesStructure.h b/Source/JavaScriptCore/dfg/DFGEdgeUsesStructure.h
deleted file mode 100644 (file)
index 7307512..0000000
+++ /dev/null
@@ -1,61 +0,0 @@
-/*
- * Copyright (C) 2013 Apple Inc. All rights reserved.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
- * 1. Redistributions of source code must retain the above copyright
- *    notice, this list of conditions and the following disclaimer.
- * 2. Redistributions in binary form must reproduce the above copyright
- *    notice, this list of conditions and the following disclaimer in the
- *    documentation and/or other materials provided with the distribution.
- *
- * THIS SOFTWARE IS PROVIDED BY APPLE INC. ``AS IS'' AND ANY
- * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
- * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
- * PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL APPLE INC. OR
- * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
- * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
- * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
- * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY
- * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
- * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
- * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. 
- */
-
-#pragma once
-
-#if ENABLE(DFG_JIT)
-
-#include "DFGGraph.h"
-
-namespace JSC { namespace DFG {
-
-class EdgeUsesStructure {
-public:
-    EdgeUsesStructure()
-        : m_result(false)
-    {
-    }
-    
-    void operator()(Node*, Edge edge)
-    {
-        m_result |= usesStructure(edge.useKind());
-    }
-    
-    bool result() const { return m_result; }
-    
-private:
-    bool m_result;
-};
-
-inline bool edgesUseStructure(Graph& graph, Node* node)
-{
-    EdgeUsesStructure edgeUsesStructure;
-    DFG_NODE_DO_TO_CHILDREN(graph, node, edgeUsesStructure);
-    return edgeUsesStructure.result();
-}
-
-} } // namespace JSC::DFG
-
-#endif // ENABLE(DFG_JIT)
index 0ef1d1e..18b65a3 100644 (file)
@@ -2454,20 +2454,23 @@ private:
         if (!m_graph.canOptimizeStringObjectAccess(node->origin.semantic))
             return;
         
+        addCheckStructureForOriginalStringObjectUse(useKind, node->origin, node->child1().node());
         createToString<useKind>(node, node->child1());
         arrayMode = ArrayMode(Array::String, Array::Read);
     }
-    
-    template<UseKind useKind>
-    bool isStringObjectUse()
+
+    void addCheckStructureForOriginalStringObjectUse(UseKind useKind, const NodeOrigin& origin, Node* node)
     {
-        switch (useKind) {
-        case StringObjectUse:
-        case StringOrStringObjectUse:
-            return true;
-        default:
-            return false;
-        }
+        RELEASE_ASSERT(useKind == StringObjectUse || StringOrStringObjectUse);
+
+        StructureSet set;
+        set.add(m_graph.globalObjectFor(node->origin.semantic)->stringObjectStructure());
+        if (useKind == StringOrStringObjectUse)
+            set.add(vm().stringStructure.get());
+
+        m_insertionSet.insertNode(
+            m_indexInBlock, SpecNone, CheckStructure, origin,
+            OpInfo(m_graph.addStructureSet(set)), Edge(node, CellUse));
     }
     
     template<UseKind useKind>
@@ -2749,6 +2752,7 @@ private:
         
         if (node->child1()->shouldSpeculateStringObject()
             && m_graph.canOptimizeStringObjectAccess(node->origin.semantic)) {
+            addCheckStructureForOriginalStringObjectUse(StringObjectUse, node->origin, node->child1().node());
             fixEdge<StringObjectUse>(node->child1());
             node->convertToToString();
             return;
@@ -2756,6 +2760,7 @@ private:
         
         if (node->child1()->shouldSpeculateStringOrStringObject()
             && m_graph.canOptimizeStringObjectAccess(node->origin.semantic)) {
+            addCheckStructureForOriginalStringObjectUse(StringOrStringObjectUse, node->origin, node->child1().node());
             fixEdge<StringOrStringObjectUse>(node->child1());
             node->convertToToString();
             return;
@@ -2871,12 +2876,14 @@ private:
         
         if (node->child1()->shouldSpeculateStringObject()
             && m_graph.canOptimizeStringObjectAccess(node->origin.semantic)) {
+            addCheckStructureForOriginalStringObjectUse(StringObjectUse, node->origin, node->child1().node());
             fixEdge<StringObjectUse>(node->child1());
             return;
         }
         
         if (node->child1()->shouldSpeculateStringOrStringObject()
             && m_graph.canOptimizeStringObjectAccess(node->origin.semantic)) {
+            addCheckStructureForOriginalStringObjectUse(StringOrStringObjectUse, node->origin, node->child1().node());
             fixEdge<StringOrStringObjectUse>(node->child1());
             return;
         }
@@ -2975,12 +2982,15 @@ private:
                     convertStringAddUse<StringUse>(node, edge);
                     return;
                 }
-                ASSERT(m_graph.canOptimizeStringObjectAccess(node->origin.semantic));
+                if (!Options::useConcurrentJIT())
+                    ASSERT(m_graph.canOptimizeStringObjectAccess(node->origin.semantic));
                 if (edge->shouldSpeculateStringObject()) {
+                    addCheckStructureForOriginalStringObjectUse(StringObjectUse, node->origin, edge.node());
                     convertStringAddUse<StringObjectUse>(node, edge);
                     return;
                 }
                 if (edge->shouldSpeculateStringOrStringObject()) {
+                    addCheckStructureForOriginalStringObjectUse(StringOrStringObjectUse, node->origin, edge.node());
                     convertStringAddUse<StringOrStringObjectUse>(node, edge);
                     return;
                 }
index 4d72d77..83cc1d4 100644 (file)
@@ -1690,7 +1690,7 @@ bool Graph::isStringPrototypeMethodSane(JSGlobalObject* globalObject, UniquedStr
 
 bool Graph::canOptimizeStringObjectAccess(const CodeOrigin& codeOrigin)
 {
-    if (hasExitSite(codeOrigin, NotStringObject))
+    if (hasExitSite(codeOrigin, BadCache) || hasExitSite(codeOrigin, BadConstantCache))
         return false;
 
     JSGlobalObject* globalObject = globalObjectFor(codeOrigin);
index 5cbfae8..7ef5c4b 100644 (file)
@@ -175,13 +175,6 @@ ExitMode mayExitImpl(Graph& graph, Node* node, StateType& state)
                 result = Exits;
                 break;
                 
-            // These are shady because they check the structure even if the type of the child node
-            // passes the StringObject type filter.
-            case StringObjectUse:
-            case StringOrStringObjectUse:
-                result = Exits;
-                break;
-                
             default:
                 break;
             }
index 61269bc..948bb97 100644 (file)
@@ -9468,7 +9468,6 @@ void SpeculativeJIT::compileToStringOrCallStringConstructorOrStringValueOf(Node*
         GPRReg resultGPR = result.gpr();
         
         speculateStringObject(node->child1(), op1GPR);
-        m_interpreter.filter(node->child1(), SpecStringObject);
 
         m_jit.loadPtr(JITCompiler::Address(op1GPR, JSWrapperObject::internalValueCellOffset()), resultGPR);
         cellResult(resultGPR, node);
@@ -9479,17 +9478,13 @@ void SpeculativeJIT::compileToStringOrCallStringConstructorOrStringValueOf(Node*
         GPRTemporary result(this);
         GPRReg resultGPR = result.gpr();
 
-        m_jit.load32(JITCompiler::Address(op1GPR, JSCell::structureIDOffset()), resultGPR);
-        JITCompiler::Jump isString = m_jit.branchWeakStructure(
-            JITCompiler::Equal,
-            resultGPR,
-            m_jit.graph().registerStructure(m_jit.vm()->stringStructure.get()));
+        m_jit.load8(JITCompiler::Address(op1GPR, JSCell::typeInfoTypeOffset()), resultGPR);
+        JITCompiler::Jump isString = m_jit.branch32(JITCompiler::Equal, resultGPR, TrustedImm32(StringType));
 
-        speculateStringObjectForStructure(node->child1(), resultGPR);
-        
+        speculationCheck(BadType, JSValueSource::unboxedCell(op1GPR), node->child1().node(), m_jit.branch32(JITCompiler::NotEqual, resultGPR, TrustedImm32(StringObjectType)));
         m_jit.loadPtr(JITCompiler::Address(op1GPR, JSWrapperObject::internalValueCellOffset()), resultGPR);
-        
         JITCompiler::Jump done = m_jit.jump();
+
         isString.link(&m_jit);
         m_jit.move(op1GPR, resultGPR);
         done.link(&m_jit);
@@ -10192,9 +10187,9 @@ void SpeculativeJIT::speculateString(Edge edge)
     speculateString(edge, operand.gpr());
 }
 
-void SpeculativeJIT::speculateStringObject(Edge edge, GPRReg gpr)
+void SpeculativeJIT::speculateStringObject(Edge edge, GPRReg cellGPR)
 {
-    speculateStringObjectForStructure(edge, JITCompiler::Address(gpr, JSCell::structureIDOffset()));
+    DFG_TYPE_CHECK(JSValueSource::unboxedCell(cellGPR), edge, ~SpecCellCheck | SpecStringObject, m_jit.branchIfNotType(cellGPR, StringObjectType));
 }
 
 void SpeculativeJIT::speculateStringObject(Edge edge)
@@ -10204,11 +10199,7 @@ void SpeculativeJIT::speculateStringObject(Edge edge)
     
     SpeculateCellOperand operand(this, edge);
     GPRReg gpr = operand.gpr();
-    if (!needsTypeCheck(edge, SpecStringObject))
-        return;
-    
     speculateStringObject(edge, gpr);
-    m_interpreter.filter(edge, SpecStringObject);
 }
 
 void SpeculativeJIT::speculateStringOrStringObject(Edge edge)
@@ -10221,17 +10212,13 @@ void SpeculativeJIT::speculateStringOrStringObject(Edge edge)
     if (!needsTypeCheck(edge, SpecString | SpecStringObject))
         return;
 
-    GPRTemporary structureID(this);
-    GPRReg structureIDGPR = structureID.gpr();
+    GPRTemporary typeTemp(this);
+    GPRReg typeGPR = typeTemp.gpr();
 
-    m_jit.load32(JITCompiler::Address(gpr, JSCell::structureIDOffset()), structureIDGPR); 
-    JITCompiler::Jump isString = m_jit.branchWeakStructure(
-        JITCompiler::Equal,
-        structureIDGPR, 
-        m_jit.graph().registerStructure(m_jit.vm()->stringStructure.get()));
-    
-    speculateStringObjectForStructure(edge, structureIDGPR);
-    
+    m_jit.load8(JITCompiler::Address(gpr, JSCell::typeInfoTypeOffset()), typeGPR);
+
+    JITCompiler::Jump isString = m_jit.branch32(JITCompiler::Equal, typeGPR, TrustedImm32(StringType));
+    speculationCheck(BadType, JSValueSource::unboxedCell(gpr), edge.node(), m_jit.branch32(JITCompiler::NotEqual, typeGPR, TrustedImm32(StringObjectType)));
     isString.link(&m_jit);
     
     m_interpreter.filter(edge, SpecString | SpecStringObject);
index 2836910..a2ad6be 100644 (file)
@@ -1631,8 +1631,6 @@ public:
     void speculateStringOrOther(Edge);
     void speculateNotStringVar(Edge);
     void speculateNotSymbol(Edge);
-    template<typename StructureLocationType>
-    void speculateStringObjectForStructure(Edge, StructureLocationType);
     void speculateStringObject(Edge, GPRReg);
     void speculateStringObject(Edge);
     void speculateStringOrStringObject(Edge);
@@ -2604,20 +2602,6 @@ private:
     GPRReg m_gprOrInvalid;
 };
 
-template<typename StructureLocationType>
-void SpeculativeJIT::speculateStringObjectForStructure(Edge edge, StructureLocationType structureLocation)
-{
-    RegisteredStructure stringObjectStructure =
-        m_jit.graph().registerStructure(m_jit.globalObjectFor(m_currentNode->origin.semantic)->stringObjectStructure());
-    
-    if (!m_state.forNode(edge).m_structure.isSubsetOf(RegisteredStructureSet(stringObjectStructure))) {
-        speculationCheck(
-            NotStringObject, JSValueRegs(), 0,
-            m_jit.branchWeakStructure(
-                JITCompiler::NotEqual, structureLocation, stringObjectStructure));
-    }
-}
-
 #define DFG_TYPE_CHECK_WITH_EXIT_KIND(exitKind, source, edge, typesPassedThrough, jumpToFail) do { \
         JSValueSource _dtc_source = (source);                           \
         Edge _dtc_edge = (edge);                                        \
index 9fb17cc..99aa0cd 100644 (file)
@@ -268,27 +268,9 @@ inline bool isCell(UseKind kind)
     }
 }
 
-// Returns true if it uses structure in a way that could be clobbered by
-// things that change the structure.
-inline bool usesStructure(UseKind kind)
-{
-    switch (kind) {
-    case StringObjectUse:
-    case StringOrStringObjectUse:
-        return true;
-    default:
-        return false;
-    }
-}
-
 // Returns true if we've already guaranteed the type 
 inline bool alreadyChecked(UseKind kind, SpeculatedType type)
 {
-    // If the check involves the structure then we need to know more than just the type to be sure
-    // that the check is done.
-    if (usesStructure(kind))
-        return false;
-    
     return !(type & ~typeFilterFor(kind));
 }
 
index 3e6f408..2a941af 100644 (file)
@@ -6326,26 +6326,26 @@ private:
         case StringObjectUse: {
             LValue cell = lowCell(m_node->child1());
             speculateStringObjectForCell(m_node->child1(), cell);
-            m_interpreter.filter(m_node->child1(), SpecStringObject);
-            
             setJSValue(m_out.loadPtr(cell, m_heaps.JSWrapperObject_internalValue));
             return;
         }
             
         case StringOrStringObjectUse: {
             LValue cell = lowCell(m_node->child1());
-            LValue structureID = m_out.load32(cell, m_heaps.JSCell_structureID);
+            LValue type = m_out.load8ZeroExt32(cell, m_heaps.JSCell_typeInfoType);
             
             LBasicBlock notString = m_out.newBlock();
             LBasicBlock continuation = m_out.newBlock();
             
             ValueFromBlock simpleResult = m_out.anchor(cell);
             m_out.branch(
-                m_out.equal(structureID, m_out.constInt32(vm().stringStructure->id())),
+                m_out.equal(type, m_out.constInt32(StringType)),
                 unsure(continuation), unsure(notString));
             
             LBasicBlock lastNext = m_out.appendTo(notString, continuation);
-            speculateStringObjectForStructureID(m_node->child1(), structureID);
+            speculate(
+                BadType, jsValueValue(cell), m_node->child1().node(),
+                m_out.notEqual(type, m_out.constInt32(StringObjectType)));
             ValueFromBlock unboxedResult = m_out.anchor(
                 m_out.loadPtr(cell, m_heaps.JSWrapperObject_internalValue));
             m_out.jump(continuation);
@@ -16051,49 +16051,44 @@ private:
             return;
         
         speculateStringObjectForCell(edge, lowCell(edge));
-        m_interpreter.filter(edge, SpecStringObject);
     }
     
     void speculateStringOrStringObject(Edge edge)
     {
         if (!m_interpreter.needsTypeCheck(edge, SpecString | SpecStringObject))
             return;
+
+        LValue cellBase = lowCell(edge);
+        if (!m_interpreter.needsTypeCheck(edge, SpecString | SpecStringObject))
+            return;
         
         LBasicBlock notString = m_out.newBlock();
         LBasicBlock continuation = m_out.newBlock();
         
-        LValue structureID = m_out.load32(lowCell(edge), m_heaps.JSCell_structureID);
+        LValue type = m_out.load8ZeroExt32(cellBase, m_heaps.JSCell_typeInfoType);
         m_out.branch(
-            m_out.equal(structureID, m_out.constInt32(vm().stringStructure->id())),
+            m_out.equal(type, m_out.constInt32(StringType)),
             unsure(continuation), unsure(notString));
         
         LBasicBlock lastNext = m_out.appendTo(notString, continuation);
-        speculateStringObjectForStructureID(edge, structureID);
+        speculate(
+            BadType, jsValueValue(cellBase), edge.node(),
+            m_out.notEqual(type, m_out.constInt32(StringObjectType)));
         m_out.jump(continuation);
         
         m_out.appendTo(continuation, lastNext);
-        
         m_interpreter.filter(edge, SpecString | SpecStringObject);
     }
     
     void speculateStringObjectForCell(Edge edge, LValue cell)
     {
-        speculateStringObjectForStructureID(edge, m_out.load32(cell, m_heaps.JSCell_structureID));
-    }
-    
-    void speculateStringObjectForStructureID(Edge edge, LValue structureID)
-    {
-        RegisteredStructure stringObjectStructure =
-            m_graph.registerStructure(m_graph.globalObjectFor(m_node->origin.semantic)->stringObjectStructure());
-
-        if (abstractStructure(edge).isSubsetOf(RegisteredStructureSet(stringObjectStructure)))
+        if (!m_interpreter.needsTypeCheck(edge, SpecStringObject))
             return;
-        
-        speculate(
-            NotStringObject, noValue(), 0,
-            m_out.notEqual(structureID, weakStructureID(stringObjectStructure)));
-    }
 
+        LValue type = m_out.load8ZeroExt32(cell, m_heaps.JSCell_typeInfoType);
+        FTL_TYPE_CHECK(jsValueValue(cell), edge, SpecStringObject, m_out.notEqual(type, m_out.constInt32(StringObjectType)));
+    }
+    
     void speculateSymbol(Edge edge, LValue cell)
     {
         FTL_TYPE_CHECK(jsValueValue(cell), edge, SpecSymbol, isNotSymbol(cell));
index 704a16f..054df78 100644 (file)
@@ -100,6 +100,7 @@ void printInternal(PrintStream& out, JSC::JSType type)
     CASE(JSWeakMapType)
     CASE(JSWeakSetType)
     CASE(WebAssemblyToJSCalleeType)
+    CASE(StringObjectType)
     CASE(MaxJSType)
     }
 }
index ee9092b..38aaf51 100644 (file)
@@ -110,10 +110,10 @@ enum JSType : uint8_t {
     JSSetType,
     JSWeakMapType,
     JSWeakSetType,
-
     WebAssemblyToJSCalleeType,
+    StringObjectType,
 
-    LastJSCObjectType = WebAssemblyToJSCalleeType, // This is the last "JSC" Object type. After this, we have embedder's (e.g., WebCore) extended object types.
+    LastJSCObjectType = StringObjectType, // This is the last "JSC" Object type. After this, we have embedder's (e.g., WebCore) extended object types.
     MaxJSType = 0b11111111,
 };
 
index 506c4c5..86a0fab 100644 (file)
@@ -63,7 +63,7 @@ public:
 
     static Structure* createStructure(VM& vm, JSGlobalObject* globalObject, JSValue prototype)
     {
-        return Structure::create(vm, globalObject, prototype, TypeInfo(ObjectType, StructureFlags), info());
+        return Structure::create(vm, globalObject, prototype, TypeInfo(StringObjectType, StructureFlags), info());
     }
 
 protected:
index 96ffd26..ced342a 100644 (file)
@@ -41,7 +41,7 @@ public:
 
     static Structure* createStructure(VM& vm, JSGlobalObject* globalObject, JSValue prototype)
     {
-        return Structure::create(vm, globalObject, prototype, TypeInfo(ObjectType, StructureFlags), info());
+        return Structure::create(vm, globalObject, prototype, TypeInfo(StringObjectType, StructureFlags), info());
     }
 
     DECLARE_INFO;