Attachment 357106 Details for Bug 192615 – Patch

[patch] Patch

bug-192615-20181212152606.patch (text/plain), 12.68 KB, created by Yusuke Suzuki on 2018-12-11 22:26:07 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Yusuke Suzuki

Created: 2018-12-11 22:26:07 PST

Size: 12.68 KB

patch

obsolete

>Subversion Revision: 239097
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index 1d30f53ef8fa6d31b9d3cf6ee1a2b3fb40d5a762..4f26712c137e388dead4cbd3b618452d4580dd76 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,30 @@
>+2018-12-11  Yusuke Suzuki  <yusukesuzuki@slowstart.org>
>+
>+        [BigInt] Simplify boolean context evaluation by leveraging JSString::offsetOfLength() == JSBigInt::offsetOfLength()
>+        https://bugs.webkit.org/show_bug.cgi?id=192615
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        JSString and JSBigInt have similar concept in terms of the implementation.
>+        Both are immutable, JSCells, and have length information. m_length is located
>+        just after JSCell header part, we can ensure `JSString::offsetOfLength() == JSBigInt::offsetOfLength()`,
>+        and it allows us to optimize the boolean context evaluation.
>+
>+        This patch leverages the above information to reduce the code size for the boolean context evaluation.
>+
>+        * ftl/FTLAbstractHeapRepository.cpp:
>+        (JSC::FTL::AbstractHeapRepository::AbstractHeapRepository):
>+        * ftl/FTLAbstractHeapRepository.h:
>+        * ftl/FTLLowerDFGToB3.cpp:
>+        (JSC::FTL::DFG::LowerDFGToB3::boolify):
>+        * jit/AssemblyHelpers.cpp:
>+        (JSC::AssemblyHelpers::emitConvertValueToBoolean):
>+        (JSC::AssemblyHelpers::branchIfValue):
>+        * runtime/JSBigInt.cpp:
>+        (JSC::JSBigInt::JSBigInt):
>+        (JSC::JSBigInt::offsetOfLength): Deleted.
>+        * runtime/JSBigInt.h:
>+
> 2018-12-10  Don Olmstead  <don.olmstead@sony.com>
> 
>         Move ENABLE_RESOURCE_LOAD_STATISTICS to FeatureDefines.xcconfig
>diff --git a/Source/JavaScriptCore/ftl/FTLAbstractHeapRepository.cpp b/Source/JavaScriptCore/ftl/FTLAbstractHeapRepository.cpp
>index fcee94df446b00a526258c7262330c5fa8322085..cec28609251a0135d58f236d700745156d385902 100644
>--- a/Source/JavaScriptCore/ftl/FTLAbstractHeapRepository.cpp
>+++ b/Source/JavaScriptCore/ftl/FTLAbstractHeapRepository.cpp
>@@ -61,6 +61,7 @@ AbstractHeapRepository::AbstractHeapRepository()
>     , JSCell_freeListNext(JSCell_header)
>     , ArrayStorage_publicLength(Butterfly_publicLength)
>     , ArrayStorage_vectorLength(Butterfly_vectorLength)
>+    , JSBigInt_length(JSString_length)
>     
> #define INDEXED_ABSTRACT_HEAP_INITIALIZATION(name, offset, size) , name(&root, #name, offset, size)
>     FOR_EACH_INDEXED_ABSTRACT_HEAP(INDEXED_ABSTRACT_HEAP_INITIALIZATION)
>@@ -78,6 +79,8 @@ AbstractHeapRepository::AbstractHeapRepository()
>     RELEASE_ASSERT(JSCell_indexingTypeAndMisc.offset() + 2 == JSCell_typeInfoFlags.offset());
>     RELEASE_ASSERT(JSCell_indexingTypeAndMisc.offset() + 3 == JSCell_cellState.offset());
> 
>+    RELEASE_ASSERT(JSBigInt::offsetOfLength() == JSString::offsetOfLength());
>+
>     JSCell_structureID.changeParent(&JSCell_header);
>     JSCell_usefulBytes.changeParent(&JSCell_header);
>     JSCell_indexingTypeAndMisc.changeParent(&JSCell_usefulBytes);
>diff --git a/Source/JavaScriptCore/ftl/FTLAbstractHeapRepository.h b/Source/JavaScriptCore/ftl/FTLAbstractHeapRepository.h
>index 3ffca17049671cfc6e4817f1ea56ed6181d1a7d3..9ef5c3c2ae46a3e075977253631547e089ebf48f 100644
>--- a/Source/JavaScriptCore/ftl/FTLAbstractHeapRepository.h
>+++ b/Source/JavaScriptCore/ftl/FTLAbstractHeapRepository.h
>@@ -85,7 +85,6 @@ namespace JSC { namespace FTL {
>     macro(JSString_flags, JSString::offsetOfFlags()) \
>     macro(JSString_length, JSString::offsetOfLength()) \
>     macro(JSString_value, JSString::offsetOfValue()) \
>-    macro(JSBigInt_length, JSBigInt::offsetOfLength()) \
>     macro(JSSymbolTableObject_symbolTable, JSSymbolTableObject::offsetOfSymbolTable()) \
>     macro(JSWrapperObject_internalValue, JSWrapperObject::internalValueOffset()) \
>     macro(RegExpConstructor_cachedResult_lastRegExp, RegExpConstructor::offsetOfCachedResult() + RegExpCachedResult::offsetOfLastRegExp()) \
>@@ -178,6 +177,7 @@ class AbstractHeapRepository {
>     AbstractHeap& JSCell_freeListNext;
>     AbstractHeap& ArrayStorage_publicLength;
>     AbstractHeap& ArrayStorage_vectorLength;
>+    AbstractHeap& JSBigInt_length;
>     
> #define INDEXED_ABSTRACT_HEAP_DECLARATION(name, offset, size) IndexedAbstractHeap name;
>     FOR_EACH_INDEXED_ABSTRACT_HEAP(INDEXED_ABSTRACT_HEAP_DECLARATION)
>diff --git a/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp b/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
>index 0823fc8f78058daf34b59081d73cc7af3173ba9c..75e70bf600c9d9f201548ed2c6e78af100f69a7b 100644
>--- a/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
>+++ b/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
>@@ -13605,10 +13605,9 @@ class LowerDFGToB3 {
>             // }
>             
>             LBasicBlock cellCase = m_out.newBlock();
>-            LBasicBlock stringCase = m_out.newBlock();
>             LBasicBlock notStringCase = m_out.newBlock();
>-            LBasicBlock bigIntCase = m_out.newBlock();
>-            LBasicBlock notBigIntCase = m_out.newBlock();
>+            LBasicBlock stringOrBigIntCase = m_out.newBlock();
>+            LBasicBlock notStringOrBigIntCase = m_out.newBlock();
>             LBasicBlock notCellCase = m_out.newBlock();
>             LBasicBlock int32Case = m_out.newBlock();
>             LBasicBlock notInt32Case = m_out.newBlock();
>@@ -13620,29 +13619,23 @@ class LowerDFGToB3 {
>             
>             m_out.branch(isCell(value, provenType(edge)), unsure(cellCase), unsure(notCellCase));
>             
>-            LBasicBlock lastNext = m_out.appendTo(cellCase, stringCase);
>+            LBasicBlock lastNext = m_out.appendTo(cellCase, notStringCase);
>             m_out.branch(
>                 isString(value, provenType(edge) & SpecCell),
>-                unsure(stringCase), unsure(notStringCase));
>+                unsure(stringOrBigIntCase), unsure(notStringCase));
>             
>-            m_out.appendTo(stringCase, notStringCase);
>-            LValue nonEmptyString = m_out.notZero32(
>-                m_out.load32NonNegative(value, m_heaps.JSString_length));
>-            results.append(m_out.anchor(nonEmptyString));
>-            m_out.jump(continuation);
>-
>-            m_out.appendTo(notStringCase, bigIntCase);
>+            m_out.appendTo(notStringCase, stringOrBigIntCase);
>             m_out.branch(
>                 isBigInt(value, provenType(edge) & SpecCell),
>-                unsure(bigIntCase), unsure(notBigIntCase));
>+                unsure(stringOrBigIntCase), unsure(notStringOrBigIntCase));
> 
>-            m_out.appendTo(bigIntCase, notBigIntCase);
>-            LValue nonZeroBigInt = m_out.notZero32(
>+            m_out.appendTo(stringOrBigIntCase, notStringOrBigIntCase);
>+            LValue nonZeroCell = m_out.notZero32(
>                 m_out.load32NonNegative(value, m_heaps.JSBigInt_length));
>-            results.append(m_out.anchor(nonZeroBigInt));
>+            results.append(m_out.anchor(nonZeroCell));
>             m_out.jump(continuation);
>             
>-            m_out.appendTo(notBigIntCase, notCellCase);
>+            m_out.appendTo(notStringOrBigIntCase, notCellCase);
>             LValue isTruthyObject;
>             if (masqueradesAsUndefinedWatchpointIsStillValid())
>                 isTruthyObject = m_out.booleanTrue;
>diff --git a/Source/JavaScriptCore/jit/AssemblyHelpers.cpp b/Source/JavaScriptCore/jit/AssemblyHelpers.cpp
>index 262d51660891d0ce8198a18c77a399ad9c633c70..22de3d26bc567a0b292257291e407045ecc8701f 100644
>--- a/Source/JavaScriptCore/jit/AssemblyHelpers.cpp
>+++ b/Source/JavaScriptCore/jit/AssemblyHelpers.cpp
>@@ -775,18 +775,9 @@ void AssemblyHelpers::emitConvertValueToBoolean(VM& vm, JSValueRegs value, GPRRe
>     JumpList done;
> 
>     auto notCell = branchIfNotCell(value);
>-    auto isCellButNotString = branchIfNotString(value.payloadGPR());
>-    load32(Address(value.payloadGPR(), JSString::offsetOfLength()), result);
>-    compare32(invert ? Equal : NotEqual, result, TrustedImm32(0), result);
>-    done.append(jump());
>+    auto isString = branchIfString(value.payloadGPR());
>+    auto isBigInt = branchIfBigInt(value.payloadGPR());
> 
>-    isCellButNotString.link(this);
>-    auto isCellButNotBigIntOrString = branchIfNotBigInt(value.payloadGPR());
>-    load32(Address(value.payloadGPR(), JSBigInt::offsetOfLength()), result);
>-    compare32(invert ? Equal : NotEqual, result, TrustedImm32(0), result);
>-    done.append(jump());
>-
>-    isCellButNotBigIntOrString.link(this);
>     if (shouldCheckMasqueradesAsUndefined) {
>         ASSERT(scratchIfShouldCheckMasqueradesAsUndefined != InvalidGPRReg);
>         JumpList isNotMasqueradesAsUndefined;
>@@ -803,6 +794,13 @@ void AssemblyHelpers::emitConvertValueToBoolean(VM& vm, JSValueRegs value, GPRRe
>     move(invert ? TrustedImm32(0) : TrustedImm32(1), result);
>     done.append(jump());
> 
>+    isString.link(this);
>+    isBigInt.link(this);
>+    ASSERT(JSString::offsetOfLength() == JSBigInt::offsetOfLength());
>+    load32(Address(value.payloadGPR(), JSBigInt::offsetOfLength()), result);
>+    compare32(invert ? Equal : NotEqual, result, TrustedImm32(0), result);
>+    done.append(jump());
>+
>     notCell.link(this);
>     auto notInt32 = branchIfNotInt32(value);
>     compare32(invert ? Equal : NotEqual, value.payloadGPR(), TrustedImm32(0), result);
>@@ -855,16 +853,10 @@ AssemblyHelpers::JumpList AssemblyHelpers::branchIfValue(VM& vm, JSValueRegs val
>     JumpList truthy;
> 
>     auto notCell = branchIfNotCell(value);
>-    auto isCellButNotString = branchIfNotString(value.payloadGPR());
>-    truthy.append(branchTest32(invert ? Zero : NonZero, Address(value.payloadGPR(), JSString::offsetOfLength())));
>-    done.append(jump());
>-    isCellButNotString.link(this);
>-    auto isCellButNotBigIntOrString = branchIfNotBigInt(value.payloadGPR());
>-    truthy.append(branchTest32(invert ? Zero : NonZero, Address(value.payloadGPR(), JSBigInt::offsetOfLength())));
>-    done.append(jump());
>+    auto isString = branchIfString(value.payloadGPR());
>+    auto isBigInt = branchIfBigInt(value.payloadGPR());
> 
>     if (shouldCheckMasqueradesAsUndefined) {
>-        isCellButNotBigIntOrString.link(this);
>         ASSERT(scratchIfShouldCheckMasqueradesAsUndefined != InvalidGPRReg);
>         JumpList isNotMasqueradesAsUndefined;
>         isNotMasqueradesAsUndefined.append(branchTest8(Zero, Address(value.payloadGPR(), JSCell::typeInfoFlagsOffset()), TrustedImm32(MasqueradesAsUndefined)));
>@@ -884,11 +876,17 @@ AssemblyHelpers::JumpList AssemblyHelpers::branchIfValue(VM& vm, JSValueRegs val
>             truthy.append(isNotMasqueradesAsUndefined);
>     } else {
>         if (invert)
>-            done.append(isCellButNotBigIntOrString);
>+            done.append(jump());
>         else
>-            truthy.append(isCellButNotBigIntOrString);
>+            truthy.append(jump());
>     }
> 
>+    isString.link(this);
>+    isBigInt.link(this);
>+    ASSERT(JSString::offsetOfLength() == JSBigInt::offsetOfLength());
>+    truthy.append(branchTest32(invert ? Zero : NonZero, Address(value.payloadGPR(), JSBigInt::offsetOfLength())));
>+    done.append(jump());
>+
>     notCell.link(this);
>     auto notInt32 = branchIfNotInt32(value);
>     truthy.append(branchTest32(invert ? Zero : NonZero, value.payloadGPR()));
>diff --git a/Source/JavaScriptCore/runtime/JSBigInt.cpp b/Source/JavaScriptCore/runtime/JSBigInt.cpp
>index 35363ee2183412198b59dbe842219ca4bdaef658..afe3cfde0036c9e6b2551abdea0564228500df14 100644
>--- a/Source/JavaScriptCore/runtime/JSBigInt.cpp
>+++ b/Source/JavaScriptCore/runtime/JSBigInt.cpp
>@@ -65,7 +65,6 @@ const ClassInfo JSBigInt::s_info =
> JSBigInt::JSBigInt(VM& vm, Structure* structure, unsigned length)
>     : Base(vm, structure)
>     , m_length(length)
>-    , m_sign(false)
> { }
> 
> void JSBigInt::initialize(InitializationType initType)
>@@ -1694,11 +1693,6 @@ inline size_t JSBigInt::offsetOfData()
>     return WTF::roundUpToMultipleOf<sizeof(Digit)>(sizeof(JSBigInt));
> }
> 
>-size_t JSBigInt::offsetOfLength()
>-{
>-    return OBJECT_OFFSETOF(JSBigInt, m_length);
>-}
>-
> template <typename CharType>
> JSBigInt* JSBigInt::parseInt(ExecState* exec, CharType*  data, unsigned length, ErrorParseMode errorParseMode)
> {
>diff --git a/Source/JavaScriptCore/runtime/JSBigInt.h b/Source/JavaScriptCore/runtime/JSBigInt.h
>index db1f4d0105f048cd05cbcd8d1e546fa3c1ada4ca..8e14ca51230c554e2d4c1e829bd4d6c0a3a5365c 100644
>--- a/Source/JavaScriptCore/runtime/JSBigInt.h
>+++ b/Source/JavaScriptCore/runtime/JSBigInt.h
>@@ -58,7 +58,10 @@ class JSBigInt final : public JSCell {
>     static JSBigInt* createFrom(VM&, int64_t value);
>     static JSBigInt* createFrom(VM&, bool value);
> 
>-    static size_t offsetOfLength();
>+    static size_t offsetOfLength()
>+    {
>+        return OBJECT_OFFSETOF(JSBigInt, m_length);
>+    }
> 
>     DECLARE_EXPORT_INFO;
> 
>@@ -232,7 +235,7 @@ class JSBigInt final : public JSCell {
>     void setDigit(unsigned, Digit);
>         
>     unsigned m_length;
>-    bool m_sign;
>+    bool m_sign { false };
> };
> 
> inline JSBigInt* asBigInt(JSValue value)

Flags:

saam: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 192615: 357105 | 357106