Attachment 359354 Details for Bug 193483 – patch

[patch] patch

a-backup.diff (text/plain), 35.69 KB, created by Saam Barati on 2019-01-16 23:16:08 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Saam Barati

Created: 2019-01-16 23:16:08 PST

Size: 35.69 KB

patch

obsolete

>Index: JSTests/ChangeLog
>===================================================================
>--- JSTests/ChangeLog	(revision 240104)
>+++ JSTests/ChangeLog	(working copy)
>@@ -1,3 +1,15 @@
>+2019-01-16  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 NOBODY (OOPS!).
>+
>+        * stress/cant-eliminate-string-object-structure-check-when-string-object-is-proven.js: Added.
>+        (foo):
>+        (a.valueOf.0):
>+
> 2019-01-15  Mark Lam  <mark.lam@apple.com>
> 
>         JSFunction::canUseAllocationProfile() should account for builtin functions with no own prototypes.
>Index: JSTests/stress/cant-eliminate-string-object-structure-check-when-string-object-is-proven.js
>===================================================================
>--- JSTests/stress/cant-eliminate-string-object-structure-check-when-string-object-is-proven.js	(nonexistent)
>+++ JSTests/stress/cant-eliminate-string-object-structure-check-when-string-object-is-proven.js	(working copy)
>@@ -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: Source/JavaScriptCore/ChangeLog
>===================================================================
>--- Source/JavaScriptCore/ChangeLog	(revision 240102)
>+++ Source/JavaScriptCore/ChangeLog	(working copy)
>@@ -1,3 +1,105 @@
>+2019-01-16  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 NOBODY (OOPS!).
>+
>+        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-16  Mark Lam  <mark.lam@apple.com>
> 
>         Refactor new bytecode structs so that the fields are prefixed with "m_".
>Index: Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj
>===================================================================
>--- Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj	(revision 240102)
>+++ Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj	(working copy)
>@@ -1464,7 +1464,6 @@
> 		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, ); }; };
>@@ -4268,7 +4267,6 @@
> 		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>"; };
>@@ -7369,7 +7367,6 @@
> 				0FB4B51B16B62772003F696B /* DFGEdge.cpp */,
> 				0F66E16914DF3F1300B7B2E4 /* DFGEdge.h */,
> 				A7D9A29117A0BC7400EE2618 /* DFGEdgeDominates.h */,
>-				A7986D5617A0BB1E00A95DD0 /* DFGEdgeUsesStructure.h */,
> 				0F8F142F1ADF090100ED792C /* DFGEpoch.cpp */,
> 				0F8F14301ADF090100ED792C /* DFGEpoch.h */,
> 				A78A976C179738B8009DF744 /* DFGFailedFinalizer.cpp */,
>@@ -8750,7 +8747,6 @@
> 				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: Source/JavaScriptCore/bytecode/ExitKind.cpp
>===================================================================
>--- Source/JavaScriptCore/bytecode/ExitKind.cpp	(revision 240102)
>+++ Source/JavaScriptCore/bytecode/ExitKind.cpp	(working copy)
>@@ -70,8 +70,6 @@ const char* exitKindToString(ExitKind ki
>         return "ArgumentsEscaped";
>     case ExoticObjectMode:
>         return "ExoticObjectMode";
>-    case NotStringObject:
>-        return "NotStringObject";
>     case VarargsOverflow:
>         return "VarargsOverflow";
>     case TDZFailure:
>Index: Source/JavaScriptCore/bytecode/ExitKind.h
>===================================================================
>--- Source/JavaScriptCore/bytecode/ExitKind.h	(revision 240102)
>+++ Source/JavaScriptCore/bytecode/ExitKind.h	(working copy)
>@@ -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: Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h	(revision 240102)
>+++ Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h	(working copy)
>@@ -2421,12 +2421,6 @@ bool AbstractInterpreter<AbstractStateTy
>     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: Source/JavaScriptCore/dfg/DFGCSEPhase.cpp
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGCSEPhase.cpp	(revision 240102)
>+++ Source/JavaScriptCore/dfg/DFGCSEPhase.cpp	(working copy)
>@@ -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: Source/JavaScriptCore/dfg/DFGClobberize.h
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGClobberize.h	(revision 240102)
>+++ Source/JavaScriptCore/dfg/DFGClobberize.h	(working copy)
>@@ -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
>     // 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
>     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:
>Index: Source/JavaScriptCore/dfg/DFGEdgeUsesStructure.h
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGEdgeUsesStructure.h	(revision 240102)
>+++ Source/JavaScriptCore/dfg/DFGEdgeUsesStructure.h	(nonexistent)
>@@ -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: Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGFixupPhase.cpp	(revision 240102)
>+++ Source/JavaScriptCore/dfg/DFGFixupPhase.cpp	(working copy)
>@@ -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: Source/JavaScriptCore/dfg/DFGGraph.cpp
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGGraph.cpp	(revision 240102)
>+++ Source/JavaScriptCore/dfg/DFGGraph.cpp	(working copy)
>@@ -1690,7 +1690,7 @@ bool Graph::isStringPrototypeMethodSane(
> 
> bool Graph::canOptimizeStringObjectAccess(const CodeOrigin& codeOrigin)
> {
>-    if (hasExitSite(codeOrigin, NotStringObject))
>+    if (hasExitSite(codeOrigin, BadCache) || hasExitSite(codeOrigin, BadConstantCache))
>         return false;
> 
>     JSGlobalObject* globalObject = globalObjectFor(codeOrigin);
>Index: Source/JavaScriptCore/dfg/DFGMayExit.cpp
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGMayExit.cpp	(revision 240102)
>+++ Source/JavaScriptCore/dfg/DFGMayExit.cpp	(working copy)
>@@ -175,13 +175,6 @@ ExitMode mayExitImpl(Graph& graph, Node*
>                 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: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp	(revision 240102)
>+++ Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp	(working copy)
>@@ -9468,7 +9468,6 @@ void SpeculativeJIT::compileToStringOrCa
>         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::compileToStringOrCa
>         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(Edg
>     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::speculateStringObje
>     
>     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::speculateStringOrSt
>     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: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h	(revision 240102)
>+++ Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h	(working copy)
>@@ -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: Source/JavaScriptCore/dfg/DFGUseKind.h
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGUseKind.h	(revision 240102)
>+++ Source/JavaScriptCore/dfg/DFGUseKind.h	(working copy)
>@@ -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: Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
>===================================================================
>--- Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp	(revision 240102)
>+++ Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp	(working copy)
>@@ -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.load32(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.load32(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.load32(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: Source/JavaScriptCore/runtime/JSType.cpp
>===================================================================
>--- Source/JavaScriptCore/runtime/JSType.cpp	(revision 240102)
>+++ Source/JavaScriptCore/runtime/JSType.cpp	(working copy)
>@@ -100,6 +100,7 @@ void printInternal(PrintStream& out, JSC
>     CASE(JSWeakMapType)
>     CASE(JSWeakSetType)
>     CASE(WebAssemblyToJSCalleeType)
>+    CASE(StringObjectType)
>     CASE(MaxJSType)
>     }
> }
>Index: Source/JavaScriptCore/runtime/JSType.h
>===================================================================
>--- Source/JavaScriptCore/runtime/JSType.h	(revision 240102)
>+++ Source/JavaScriptCore/runtime/JSType.h	(working copy)
>@@ -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: Source/JavaScriptCore/runtime/StringObject.h
>===================================================================
>--- Source/JavaScriptCore/runtime/StringObject.h	(revision 240102)
>+++ Source/JavaScriptCore/runtime/StringObject.h	(working copy)
>@@ -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: Source/JavaScriptCore/runtime/StringPrototype.h
>===================================================================
>--- Source/JavaScriptCore/runtime/StringPrototype.h	(revision 240102)
>+++ Source/JavaScriptCore/runtime/StringPrototype.h	(working copy)
>@@ -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;

Flags:

ysuzuki: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 193483: 359354 | 359359