Attachment 356482 Details for Bug 179903 – Patch

[patch] Patch

bug-179903-20181204093326.patch (text/plain), 15.78 KB, created by Caio Lima on 2018-12-04 03:33:28 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Caio Lima

Created: 2018-12-04 03:33:28 PST

Size: 15.78 KB

patch

obsolete

>Subversion Revision: 238851
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index afb52aa9917857ce3e6b8219675f6965c1658f38..88097719819bcfaefe85fe48b72f36cc09615ca0 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,37 @@
>+2018-12-04  Caio Lima  <ticaiolima@gmail.com>
>+
>+        [ESNext][BigInt] Support logic operations
>+        https://bugs.webkit.org/show_bug.cgi?id=179903
>+
>+        Reviewed by Yusuke Suzuki.
>+
>+        We are introducing in this patch the ToBoolean support for JSBigInt.
>+        With this change, we can implement the correct behavior of BigInt as
>+        operand of logical opertions. During JIT genertion into DFG and FTL,
>+        we are using JSBigInt::m_length to verify if the number is 0n or not,
>+        following the same approach used by JSString. This is also safe in the case
>+        of BigInt, because only 0n has m_length == 0.
>+
>+        We are not including BigInt speculation into Branch nodes in this
>+        patch, but the plan is to implement it in further patches.
>+
>+        * ftl/FTLAbstractHeapRepository.h:
>+        * ftl/FTLLowerDFGToB3.cpp:
>+        (JSC::FTL::DFG::LowerDFGToB3::boolify):
>+        (JSC::FTL::DFG::LowerDFGToB3::isBigInt):
>+        * jit/AssemblyHelpers.cpp:
>+        (JSC::AssemblyHelpers::emitConvertValueToBoolean):
>+        (JSC::AssemblyHelpers::branchIfValue):
>+        * runtime/JSBigInt.cpp:
>+        (JSC::JSBigInt::isZero const):
>+        (JSC::JSBigInt::offsetOfLength):
>+        (JSC::JSBigInt::toBoolean const):
>+        (JSC::JSBigInt::isZero): Deleted.
>+        * runtime/JSBigInt.h:
>+        * runtime/JSCellInlines.h:
>+        (JSC::JSCell::toBoolean const):
>+        (JSC::JSCell::pureToBoolean const):
>+
> 2018-12-04  Devin Rousso  <drousso@apple.com>
> 
>         Web Inspector: Audit: tests should support async operations
>diff --git a/Source/JavaScriptCore/ftl/FTLAbstractHeapRepository.h b/Source/JavaScriptCore/ftl/FTLAbstractHeapRepository.h
>index 8c24a440c652dc54663784ac78860ffad8a6eaf3..3ffca17049671cfc6e4817f1ea56ed6181d1a7d3 100644
>--- a/Source/JavaScriptCore/ftl/FTLAbstractHeapRepository.h
>+++ b/Source/JavaScriptCore/ftl/FTLAbstractHeapRepository.h
>@@ -85,6 +85,7 @@ 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()) \
>diff --git a/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp b/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
>index c254892721f547271d730b1eb9fad203d46551de..fb0439732c43a4b3d4cb5afa3a5deca3910ea945 100644
>--- a/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
>+++ b/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
>@@ -13578,7 +13578,7 @@ private:
>             
>             // Implements the following control flow structure:
>             // if (value is cell) {
>-            //     if (value is string)
>+            //     if (value is string or value is BigInt)
>             //         result = !!value->length
>             //     else {
>             //         do evil things for masquerades-as-undefined
>@@ -13595,6 +13595,8 @@ private:
>             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 notCellCase = m_out.newBlock();
>             LBasicBlock int32Case = m_out.newBlock();
>             LBasicBlock notInt32Case = m_out.newBlock();
>@@ -13616,8 +13618,19 @@ private:
>                 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.branch(
>+                isBigInt(value, provenType(edge) & SpecCell),
>+                unsure(bigIntCase), unsure(notBigIntCase));
>+
>+            m_out.appendTo(bigIntCase, notBigIntCase);
>+            LValue nonZeroBigInt = m_out.notZero32(
>+                m_out.load32NonNegative(value, m_heaps.JSBigInt_length));
>+            results.append(m_out.anchor(nonZeroBigInt));
>+            m_out.jump(continuation);
>             
>-            m_out.appendTo(notStringCase, notCellCase);
>+            m_out.appendTo(notBigIntCase, notCellCase);
>             LValue isTruthyObject;
>             if (masqueradesAsUndefinedWatchpointIsStillValid())
>                 isTruthyObject = m_out.booleanTrue;
>@@ -15611,6 +15624,15 @@ private:
>             m_out.constInt32(vm().bigIntStructure->id()));
>     }
> 
>+    LValue isBigInt(LValue cell, SpeculatedType type = SpecFullTop)
>+    {
>+        if (LValue proven = isProvenValue(type & SpecCell, SpecBigInt))
>+            return proven;
>+        return m_out.equal(
>+            m_out.load32(cell, m_heaps.JSCell_structureID),
>+            m_out.constInt32(vm().bigIntStructure->id()));
>+    }
>+
>     LValue isArrayTypeForArrayify(LValue cell, ArrayMode arrayMode)
>     {
>         switch (arrayMode.type()) {
>diff --git a/Source/JavaScriptCore/jit/AssemblyHelpers.cpp b/Source/JavaScriptCore/jit/AssemblyHelpers.cpp
>index 5f38b4f56038f1c2c557f77924f7fb13ab325481..262d51660891d0ce8198a18c77a399ad9c633c70 100644
>--- a/Source/JavaScriptCore/jit/AssemblyHelpers.cpp
>+++ b/Source/JavaScriptCore/jit/AssemblyHelpers.cpp
>@@ -758,7 +758,7 @@ void AssemblyHelpers::emitConvertValueToBoolean(VM& vm, JSValueRegs value, GPRRe
> {
>     // Implements the following control flow structure:
>     // if (value is cell) {
>-    //     if (value is string)
>+    //     if (value is string or value is BigInt)
>     //         result = !!value->length
>     //     else {
>     //         do evil things for masquerades-as-undefined
>@@ -781,6 +781,12 @@ void AssemblyHelpers::emitConvertValueToBoolean(VM& vm, JSValueRegs value, GPRRe
>     done.append(jump());
> 
>     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;
>@@ -831,7 +837,7 @@ AssemblyHelpers::JumpList AssemblyHelpers::branchIfValue(VM& vm, JSValueRegs val
> {
>     // Implements the following control flow structure:
>     // if (value is cell) {
>-    //     if (value is string)
>+    //     if (value is string or value is BigInt)
>     //         result = !!value->length
>     //     else {
>     //         do evil things for masquerades-as-undefined
>@@ -852,9 +858,13 @@ AssemblyHelpers::JumpList AssemblyHelpers::branchIfValue(VM& vm, JSValueRegs val
>     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());
> 
>     if (shouldCheckMasqueradesAsUndefined) {
>-        isCellButNotString.link(this);
>+        isCellButNotBigIntOrString.link(this);
>         ASSERT(scratchIfShouldCheckMasqueradesAsUndefined != InvalidGPRReg);
>         JumpList isNotMasqueradesAsUndefined;
>         isNotMasqueradesAsUndefined.append(branchTest8(Zero, Address(value.payloadGPR(), JSCell::typeInfoFlagsOffset()), TrustedImm32(MasqueradesAsUndefined)));
>@@ -874,9 +884,9 @@ AssemblyHelpers::JumpList AssemblyHelpers::branchIfValue(VM& vm, JSValueRegs val
>             truthy.append(isNotMasqueradesAsUndefined);
>     } else {
>         if (invert)
>-            done.append(isCellButNotString);
>+            done.append(isCellButNotBigIntOrString);
>         else
>-            truthy.append(isCellButNotString);
>+            truthy.append(isCellButNotBigIntOrString);
>     }
> 
>     notCell.link(this);
>diff --git a/Source/JavaScriptCore/runtime/JSBigInt.cpp b/Source/JavaScriptCore/runtime/JSBigInt.cpp
>index 2f25a2411edf1e4713a87f000aa12917ff47a83f..35363ee2183412198b59dbe842219ca4bdaef658 100644
>--- a/Source/JavaScriptCore/runtime/JSBigInt.cpp
>+++ b/Source/JavaScriptCore/runtime/JSBigInt.cpp
>@@ -232,12 +232,6 @@ String JSBigInt::toString(ExecState* exec, unsigned radix)
>     return toStringGeneric(exec, this, radix);
> }
> 
>-inline bool JSBigInt::isZero()
>-{
>-    ASSERT(length() || !sign());
>-    return length() == 0;
>-}
>-
> // Multiplies {this} with {factor} and adds {summand} to the result.
> void JSBigInt::inplaceMultiplyAdd(Digit factor, Digit summand)
> {
>@@ -1700,6 +1694,11 @@ 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)
> {
>@@ -1818,6 +1817,7 @@ inline void JSBigInt::setDigit(unsigned n, Digit value)
>     ASSERT(n < length());
>     dataStorage()[n] = value;
> }
>+
> JSObject* JSBigInt::toObject(ExecState* exec, JSGlobalObject* globalObject) const
> {
>     return BigIntObject::create(exec->vm(), globalObject, const_cast<JSBigInt*>(this));
>diff --git a/Source/JavaScriptCore/runtime/JSBigInt.h b/Source/JavaScriptCore/runtime/JSBigInt.h
>index 4e708de956893ef1c13da13d1639b149e9af5e02..db1f4d0105f048cd05cbcd8d1e546fa3c1ada4ca 100644
>--- a/Source/JavaScriptCore/runtime/JSBigInt.h
>+++ b/Source/JavaScriptCore/runtime/JSBigInt.h
>@@ -28,7 +28,6 @@
> #include "CPU.h"
> #include "ExceptionHelpers.h"
> #include "JSObject.h"
>-#include "ParseInt.h"
> #include <wtf/CagedPtr.h>
> #include <wtf/text/StringBuilder.h>
> #include <wtf/text/StringView.h>
>@@ -59,6 +58,8 @@ public:
>     static JSBigInt* createFrom(VM&, int64_t value);
>     static JSBigInt* createFrom(VM&, bool value);
> 
>+    static size_t offsetOfLength();
>+
>     DECLARE_EXPORT_INFO;
> 
>     JSValue toPrimitive(ExecState*, PreferredPrimitiveType) const;
>@@ -104,6 +105,7 @@ public:
>     double toNumber(ExecState*) const;
> 
>     JSObject* toObject(ExecState*, JSGlobalObject*) const;
>+    inline bool toBoolean() const { return !isZero(); }
> 
>     static JSBigInt* multiply(ExecState*, JSBigInt* x, JSBigInt* y);
>     
>@@ -194,7 +196,11 @@ private:
>     static String toStringBasePowerOfTwo(ExecState*, JSBigInt*, unsigned radix);
>     static String toStringGeneric(ExecState*, JSBigInt*, unsigned radix);
> 
>-    bool isZero();
>+    inline bool isZero() const
>+    {
>+        ASSERT(length() || !sign());
>+        return length() == 0;
>+    }
> 
>     template <typename CharType>
>     static JSBigInt* parseInt(ExecState*, CharType*  data, unsigned length, ErrorParseMode);
>diff --git a/Source/JavaScriptCore/runtime/JSCellInlines.h b/Source/JavaScriptCore/runtime/JSCellInlines.h
>index f69b3803d6a224fc11a66b895c1dc6601ce52bbf..3c5ad64857c65356d06c4cfd3a38dc436fcbc3fa 100644
>--- a/Source/JavaScriptCore/runtime/JSCellInlines.h
>+++ b/Source/JavaScriptCore/runtime/JSCellInlines.h
>@@ -33,6 +33,7 @@
> #include "FreeListInlines.h"
> #include "Handle.h"
> #include "IsoSubspaceInlines.h"
>+#include "JSBigInt.h"
> #include "JSCast.h"
> #include "JSDestructibleObject.h"
> #include "JSObject.h"
>@@ -339,6 +340,8 @@ inline bool JSCell::toBoolean(ExecState* exec) const
> {
>     if (isString())
>         return static_cast<const JSString*>(this)->toBoolean();
>+    if (isBigInt())
>+        return static_cast<const JSBigInt*>(this)->toBoolean();
>     return !structure(exec->vm())->masqueradesAsUndefined(exec->lexicalGlobalObject());
> }
> 
>@@ -346,6 +349,8 @@ inline TriState JSCell::pureToBoolean() const
> {
>     if (isString())
>         return static_cast<const JSString*>(this)->toBoolean() ? TrueTriState : FalseTriState;
>+    if (isBigInt())
>+        return static_cast<const JSBigInt*>(this)->toBoolean() ? TrueTriState : FalseTriState;
>     if (isSymbol())
>         return TrueTriState;
>     return MixedTriState;
>diff --git a/JSTests/ChangeLog b/JSTests/ChangeLog
>index 16dc9780621d24af9232aeceb52f4b8990802607..684db424333adb5a767937c4c0f167e3e7a21713 100644
>--- a/JSTests/ChangeLog
>+++ b/JSTests/ChangeLog
>@@ -1,3 +1,15 @@
>+2018-12-04  Caio Lima  <ticaiolima@gmail.com>
>+
>+        [ESNext][BigInt] Support logic operations
>+        https://bugs.webkit.org/show_bug.cgi?id=179903
>+
>+        Reviewed by Yusuke Suzuki.
>+
>+        * stress/big-int-branch-usage.js: Added.
>+        * stress/big-int-logical-and.js: Added.
>+        * stress/big-int-logical-not.js: Added.
>+        * stress/big-int-logical-or.js: Added.
>+
> 2018-12-03  Ryan Haddad  <ryanhaddad@apple.com>
> 
>         Unreviewed, rolling out r238833.
>diff --git a/JSTests/stress/big-int-branch-usage.js b/JSTests/stress/big-int-branch-usage.js
>new file mode 100644
>index 0000000000000000000000000000000000000000..fa6125f0dbe318551d1429dc91140a5e62e94fa7
>--- /dev/null
>+++ b/JSTests/stress/big-int-branch-usage.js
>@@ -0,0 +1,23 @@
>+//@ runBigIntEnabled
>+
>+function assert(a, e) {
>+    if (a !== e) {
>+        throw new Error("Bad!");
>+    }
>+}
>+
>+function branchTest(a) {
>+    if (a)
>+        return a;
>+    else
>+        return false;
>+}
>+noInline(branchTest);
>+
>+for (let i = 0; i < 100000; i++) {
>+    assert(branchTest(10n), 10n);
>+    assert(branchTest(1n), 1n);
>+    assert(branchTest(0n), false);
>+    assert(branchTest(-1n), -1n);
>+}
>+
>diff --git a/JSTests/stress/big-int-logical-and.js b/JSTests/stress/big-int-logical-and.js
>new file mode 100644
>index 0000000000000000000000000000000000000000..619b866ea8cda3a005f773d9c98db57ca72637ef
>--- /dev/null
>+++ b/JSTests/stress/big-int-logical-and.js
>@@ -0,0 +1,20 @@
>+//@ runBigIntEnabled
>+
>+function assert(a, e) {
>+    if (a !== e) {
>+        throw new Error("Bad!");
>+    }
>+}
>+
>+function logicalAnd(a, b) {
>+    return a && b;
>+}
>+noInline(logicalAnd);
>+
>+for (let i = 0; i < 100000; i++) {
>+    assert(logicalAnd(1n, 10n), 10n);
>+    assert(logicalAnd(1n, 1n), 1n);
>+    assert(logicalAnd(1n, 0n), 0n);
>+    assert(logicalAnd(1n, -1n), -1n);
>+}
>+
>diff --git a/JSTests/stress/big-int-logical-not.js b/JSTests/stress/big-int-logical-not.js
>new file mode 100644
>index 0000000000000000000000000000000000000000..af28a72b1c3548af6044c1a82e48c4d7788595f4
>--- /dev/null
>+++ b/JSTests/stress/big-int-logical-not.js
>@@ -0,0 +1,20 @@
>+//@ runBigIntEnabled
>+
>+function assert(a, e) {
>+    if (a !== e) {
>+        throw new Error("Bad!");
>+    }
>+}
>+
>+function logicalNot(a) {
>+    return !a;
>+}
>+noInline(logicalNot);
>+
>+for (let i = 0; i < 100000; i++) {
>+    assert(logicalNot(10n), false);
>+    assert(logicalNot(1n), false);
>+    assert(logicalNot(0n), true);
>+    assert(logicalNot(-1n), false);
>+}
>+
>diff --git a/JSTests/stress/big-int-logical-or.js b/JSTests/stress/big-int-logical-or.js
>new file mode 100644
>index 0000000000000000000000000000000000000000..36a2d344562933a03ef2e04ce40c67da67da4139
>--- /dev/null
>+++ b/JSTests/stress/big-int-logical-or.js
>@@ -0,0 +1,20 @@
>+//@ runBigIntEnabled
>+
>+function assert(a, e) {
>+    if (a !== e) {
>+        throw new Error("Bad!");
>+    }
>+}
>+
>+function logicalOr(a, b) {
>+    return a || b;
>+}
>+noInline(logicalOr);
>+
>+for (let i = 0; i < 100000; i++) {
>+    assert(logicalOr(10n, "abc"), 10n);
>+    assert(logicalOr(1n, "abc"), 1n);
>+    assert(logicalOr(0n, "abc"), "abc");
>+    assert(logicalOr(-1n, "abc"), -1n);
>+}
>+

Actions: View | Formatted Diff | Diff

Attachments on bug 179903: 352488 | 352527 | 353670 | 354236 | 354240 | 354714 | 354812 | 355244 | 355634 | 355865 | 356431 | 356482 | 356490