Attachment 341752 Details for Bug 185708 – Patch

[patch] Patch

bug-185708-20180601213301.patch (text/plain), 27.41 KB, created by Yusuke Suzuki on 2018-06-01 05:33:02 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Yusuke Suzuki

Created: 2018-06-01 05:33:02 PDT

Size: 27.41 KB

patch

obsolete

>Subversion Revision: 232391
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index 526c31e0643e24ea94ca005e3c77e9d62f17acef..921c6b6d6aeb91df9b75466f2ae5d8076044bd59 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,128 @@
>+2018-06-01  Yusuke Suzuki  <utatane.tea@gmail.com>
>+
>+        Baseline op_jtrue emits an insane amount of code
>+        https://bugs.webkit.org/show_bug.cgi?id=185708
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        op_jtrue / op_jfalse bloats massive amount of code. This patch attempts to reduce the size of this code by,
>+
>+        1. op_jtrue / op_jfalse immediately jumps if the condition met. We add AssemblyHelpers::branchIf{Truthy,Falsey}
>+           to jump directly. This tightens the code.
>+
>+        2. Align our emitConvertValueToBoolean implementation to FTL's boolify function. It emits less code.
>+
>+        This reduces the code size of op_jtrue in x64 from 220 bytes to 164 bytes.
>+
>+        [  12] jtrue             arg1, 6(->18)
>+              0x7f233170162c: mov 0x30(%rbp), %rax
>+              0x7f2331701630: mov %rax, %rsi
>+              0x7f2331701633: xor $0x6, %rsi
>+              0x7f2331701637: test $0xfffffffffffffffe, %rsi
>+              0x7f233170163e: jnz 0x7f2331701654
>+              0x7f2331701644: cmp $0x7, %eax
>+              0x7f2331701647: setz %sil
>+              0x7f233170164b: movzx %sil, %esi
>+              0x7f233170164f: jmp 0x7f2331701705
>+              0x7f2331701654: test %rax, %r14
>+              0x7f2331701657: jz 0x7f233170169c
>+              0x7f233170165d: cmp %r14, %rax
>+              0x7f2331701660: jb 0x7f2331701675
>+              0x7f2331701666: test %eax, %eax
>+              0x7f2331701668: setnz %sil
>+              0x7f233170166c: movzx %sil, %esi
>+              0x7f2331701670: jmp 0x7f2331701705
>+              0x7f2331701675: lea (%r14,%rax), %rsi
>+              0x7f2331701679: movq %rsi, %xmm0
>+              0x7f233170167e: xorps %xmm1, %xmm1
>+              0x7f2331701681: ucomisd %xmm1, %xmm0
>+              0x7f2331701685: jz 0x7f2331701695
>+              0x7f233170168b: mov $0x1, %esi
>+              0x7f2331701690: jmp 0x7f2331701705
>+              0x7f2331701695: xor %esi, %esi
>+              0x7f2331701697: jmp 0x7f2331701705
>+              0x7f233170169c: test %rax, %r15
>+              0x7f233170169f: jnz 0x7f2331701703
>+              0x7f23317016a5: cmp $0x1, 0x5(%rax)
>+              0x7f23317016a9: jnz 0x7f23317016c1
>+              0x7f23317016af: mov 0x8(%rax), %esi
>+              0x7f23317016b2: test %esi, %esi
>+              0x7f23317016b4: setnz %sil
>+              0x7f23317016b8: movzx %sil, %esi
>+              0x7f23317016bc: jmp 0x7f2331701705
>+              0x7f23317016c1: test $0x1, 0x6(%rax)
>+              0x7f23317016c5: jz 0x7f23317016f9
>+              0x7f23317016cb: mov (%rax), %esi
>+              0x7f23317016cd: mov $0x7f23315000c8, %rdx
>+              0x7f23317016d7: mov (%rdx), %rdx
>+              0x7f23317016da: mov (%rdx,%rsi,8), %rsi
>+              0x7f23317016de: mov $0x7f2330de0000, %rdx
>+              0x7f23317016e8: cmp %rdx, 0x18(%rsi)
>+              0x7f23317016ec: jnz 0x7f23317016f9
>+              0x7f23317016f2: xor %esi, %esi
>+              0x7f23317016f4: jmp 0x7f2331701705
>+              0x7f23317016f9: mov $0x1, %esi
>+              0x7f23317016fe: jmp 0x7f2331701705
>+              0x7f2331701703: xor %esi, %esi
>+              0x7f2331701705: test %esi, %esi
>+              0x7f2331701707: jnz 0x7f233170171b
>+
>+        [  12] jtrue             arg1, 6(->18)
>+              0x7f6c8710156c: mov 0x30(%rbp), %rax
>+              0x7f6c87101570: test %rax, %r15
>+              0x7f6c87101573: jnz 0x7f6c871015c8
>+              0x7f6c87101579: cmp $0x1, 0x5(%rax)
>+              0x7f6c8710157d: jnz 0x7f6c87101592
>+              0x7f6c87101583: cmp $0x0, 0x8(%rax)
>+              0x7f6c87101587: jnz 0x7f6c87101623
>+              0x7f6c8710158d: jmp 0x7f6c87101615
>+              0x7f6c87101592: test $0x1, 0x6(%rax)
>+              0x7f6c87101596: jz 0x7f6c87101623
>+              0x7f6c8710159c: mov (%rax), %esi
>+              0x7f6c8710159e: mov $0x7f6c86f000e0, %rdx
>+              0x7f6c871015a8: mov (%rdx), %rdx
>+              0x7f6c871015ab: mov (%rdx,%rsi,8), %rsi
>+              0x7f6c871015af: mov $0x7f6c867e0000, %rdx
>+              0x7f6c871015b9: cmp %rdx, 0x18(%rsi)
>+              0x7f6c871015bd: jnz 0x7f6c87101623
>+              0x7f6c871015c3: jmp 0x7f6c87101615
>+              0x7f6c871015c8: cmp %r14, %rax
>+              0x7f6c871015cb: jb 0x7f6c871015de
>+              0x7f6c871015d1: test %eax, %eax
>+              0x7f6c871015d3: jnz 0x7f6c87101623
>+              0x7f6c871015d9: jmp 0x7f6c87101615
>+              0x7f6c871015de: test %rax, %r14
>+              0x7f6c871015e1: jz 0x7f6c87101602
>+              0x7f6c871015e7: lea (%r14,%rax), %rsi
>+              0x7f6c871015eb: movq %rsi, %xmm0
>+              0x7f6c871015f0: xorps %xmm1, %xmm1
>+              0x7f6c871015f3: ucomisd %xmm1, %xmm0
>+              0x7f6c871015f7: jz 0x7f6c87101615
>+              0x7f6c871015fd: jmp 0x7f6c87101623
>+              0x7f6c87101602: mov $0x7, %r11
>+              0x7f6c8710160c: cmp %r11, %rax
>+              0x7f6c8710160f: jz 0x7f6c87101623
>+
>+        * dfg/DFGSpeculativeJIT32_64.cpp:
>+        (JSC::DFG::SpeculativeJIT::emitBranch):
>+        * dfg/DFGSpeculativeJIT64.cpp:
>+        (JSC::DFG::SpeculativeJIT::emitBranch):
>+        * jit/AssemblyHelpers.cpp:
>+        (JSC::AssemblyHelpers::emitConvertValueToBoolean):
>+        (JSC::AssemblyHelpers::branchIfValue):
>+        * jit/AssemblyHelpers.h:
>+        (JSC::AssemblyHelpers::branchIfTruthy):
>+        (JSC::AssemblyHelpers::branchIfFalsey):
>+        * jit/JIT.h:
>+        * jit/JITInlines.h:
>+        (JSC::JIT::addJump):
>+        * jit/JITOpcodes.cpp:
>+        (JSC::JIT::emit_op_jfalse):
>+        (JSC::JIT::emit_op_jtrue):
>+        * jit/JITOpcodes32_64.cpp:
>+        (JSC::JIT::emit_op_jfalse):
>+        (JSC::JIT::emit_op_jtrue):
>+
> 2018-05-31  Yusuke Suzuki  <utatane.tea@gmail.com>
> 
>         [Baseline] Store constant directly in emit_op_mov
>diff --git a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp
>index ac73b7bb45b6694554662cada849b0a1d87e44d9..6949161d26452d45417597849144aec15bc7d74e 100644
>--- a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp
>+++ b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp
>@@ -1713,8 +1713,8 @@ void SpeculativeJIT::emitBranch(Node* node)
> 
>         bool shouldCheckMasqueradesAsUndefined = !masqueradesAsUndefinedWatchpointIsStillValid();
>         JSGlobalObject* globalObject = m_jit.graph().globalObjectFor(node->origin.semantic);
>-        m_jit.emitConvertValueToBoolean(*m_jit.vm(), valueRegs, resultGPR, temp.gpr(), valueFPR.fpr(), tempFPR.fpr(), shouldCheckMasqueradesAsUndefined, globalObject);
>-        branchTest32(JITCompiler::Zero, resultGPR, notTaken);
>+        auto falsey = m_jit.branchIfFalsey(*m_jit.vm(), valueRegs, resultGPR, temp.gpr(), valueFPR.fpr(), tempFPR.fpr(), shouldCheckMasqueradesAsUndefined, globalObject);
>+        addBranch(falsey, notTaken);
>         jump(taken, ForceJump);
> 
>         noResult(node, UseChildrenCalledExplicitly);
>diff --git a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp
>index db193475f5a4d98d68655989bf4e591888983697..435ea6687170cad89d34f0f3afdebf40a33d3c5d 100644
>--- a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp
>+++ b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp
>@@ -1888,9 +1888,8 @@ void SpeculativeJIT::emitBranch(Node* node)
>             value.use();
> 
>             JSGlobalObject* globalObject = m_jit.graph().globalObjectFor(node->origin.semantic);
>-            m_jit.emitConvertValueToBoolean(*m_jit.vm(), JSValueRegs(valueGPR), resultGPR, scratchGPR, valueFPR, tempFPR, shouldCheckMasqueradesAsUndefined, globalObject);
>-    
>-            branchTest32(MacroAssembler::NonZero, resultGPR, taken);
>+            auto truthy = m_jit.branchIfTruthy(*m_jit.vm(), JSValueRegs(valueGPR), resultGPR, scratchGPR, valueFPR, tempFPR, shouldCheckMasqueradesAsUndefined, globalObject);
>+            addBranch(truthy, taken);
>             jump(notTaken);
>         }
>         
>diff --git a/Source/JavaScriptCore/jit/AssemblyHelpers.cpp b/Source/JavaScriptCore/jit/AssemblyHelpers.cpp
>index 6cca65010fbba659d1a81afb8406bc35a429b980..8cc878f55c68c32669decb3d0c8901d93d91ba94 100644
>--- a/Source/JavaScriptCore/jit/AssemblyHelpers.cpp
>+++ b/Source/JavaScriptCore/jit/AssemblyHelpers.cpp
>@@ -754,92 +754,166 @@ void AssemblyHelpers::wangsInt64Hash(GPRReg inputAndResult, GPRReg scratch)
> }
> #endif // USE(JSVALUE64)
> 
>-void AssemblyHelpers::emitConvertValueToBoolean(VM& vm, JSValueRegs value, GPRReg result, GPRReg scratch, FPRReg valueAsFPR, FPRReg tempFPR, bool shouldCheckMasqueradesAsUndefined, JSGlobalObject* globalObject, bool negateResult)
>+void AssemblyHelpers::emitConvertValueToBoolean(VM& vm, JSValueRegs value, GPRReg result, GPRReg scratchIfShouldCheckMasqueradesAsUndefined, FPRReg valueAsFPR, FPRReg tempFPR, bool shouldCheckMasqueradesAsUndefined, JSGlobalObject* globalObject, bool invert)
> {
>     // Implements the following control flow structure:
>-    // if (value is boolean) {
>-    //     result = value === true
>-    // } else if (value is integer) {
>-    //     result = value !== 0
>-    // } else if (value is double) {
>-    //     result = value !== 0.0 && !isNaN(value);
>-    // } else if (value is cell) {
>-    //     if (value is string) {
>-    //          result = value.length() !== 0;
>-    //     } else {
>-    //          do crazy things for masquerades as undefined
>+    // if (value is cell) {
>+    //     if (value is string)
>+    //         result = !!value->length
>+    //     else {
>+    //         do evil things for masquerades-as-undefined
>+    //         result = true
>     //     }
>+    // } else if (value is int32) {
>+    //     result = !!unboxInt32(value)
>+    // } else if (value is number) {
>+    //     result = !!unboxDouble(value)
>     // } else {
>-    //     result = false;
>+    //     result = value == jsTrue
>     // }
>-    //
>-    // if (negateResult)
>-    //     result = !result;
> 
>     JumpList done;
>-    auto notBoolean = branchIfNotBoolean(value, result);
>-#if USE(JSVALUE64)
>-    compare32(negateResult ? NotEqual : Equal, value.gpr(), TrustedImm32(ValueTrue), result);
>-#else
>-    compare32(negateResult ? Equal : NotEqual, value.payloadGPR(), TrustedImm32(0), result);
>-#endif
>+
>+    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());
> 
>-    notBoolean.link(this);
>-#if USE(JSVALUE64)
>-    auto isNotNumber = branchIfNotNumber(value.gpr());
>-#else
>-    ASSERT(scratch != InvalidGPRReg);
>-    auto isNotNumber = branchIfNotNumber(value, scratch);
>-#endif
>-    auto isDouble = branchIfNotInt32(value);
>+    isCellButNotString.link(this);
>+    if (shouldCheckMasqueradesAsUndefined) {
>+        ASSERT(scratchIfShouldCheckMasqueradesAsUndefined != InvalidGPRReg);
>+        JumpList isNotMasqueradesAsUndefined;
>+        isNotMasqueradesAsUndefined.append(branchTest8(Zero, Address(value.payloadGPR(), JSCell::typeInfoFlagsOffset()), TrustedImm32(MasqueradesAsUndefined)));
>+        emitLoadStructure(vm, value.payloadGPR(), result, scratchIfShouldCheckMasqueradesAsUndefined);
>+        move(TrustedImmPtr(globalObject), scratchIfShouldCheckMasqueradesAsUndefined);
>+        isNotMasqueradesAsUndefined.append(branchPtr(NotEqual, Address(result, Structure::globalObjectOffset()), scratchIfShouldCheckMasqueradesAsUndefined));
> 
>-    // It's an int32.
>-    compare32(negateResult ? Equal : NotEqual, value.payloadGPR(), TrustedImm32(0), result);
>+        // We act like we are "undefined" here.
>+        move(invert ? TrustedImm32(1) : TrustedImm32(0), result);
>+        done.append(jump());
>+        isNotMasqueradesAsUndefined.link(this);
>+    }
>+    move(invert ? TrustedImm32(0) : TrustedImm32(1), result);
>+    done.append(jump());
>+
>+    notCell.link(this);
>+    auto notInt32 = branchIfNotInt32(value);
>+    compare32(invert ? Equal : NotEqual, value.payloadGPR(), TrustedImm32(0), result);
>     done.append(jump());
> 
>-    isDouble.link(this);
>+    notInt32.link(this);
>+    auto notDouble = branchIfNotDoubleKnownNotInt32(value);
> #if USE(JSVALUE64)
>     unboxDouble(value.gpr(), result, valueAsFPR);
> #else
>     unboxDouble(value, valueAsFPR, tempFPR);
> #endif
>-    auto isZeroOrNaN = branchDoubleZeroOrNaN(valueAsFPR, tempFPR);
>-    move(negateResult ? TrustedImm32(0) : TrustedImm32(1), result);
>-    done.append(jump());
>-    isZeroOrNaN.link(this);
>-    move(negateResult ? TrustedImm32(1) : TrustedImm32(0), result);
>+    move(invert ? TrustedImm32(1) : TrustedImm32(0), result);
>+    done.append(branchDoubleZeroOrNaN(valueAsFPR, tempFPR));
>+    move(invert ? TrustedImm32(0) : TrustedImm32(1), result);
>     done.append(jump());
> 
>-    isNotNumber.link(this);
>-    auto isNotCellAndIsNotNumberAndIsNotBoolean = branchIfNotCell(value);
>-    auto isCellButNotString = branch8(NotEqual,
>-        Address(value.payloadGPR(), JSCell::typeInfoTypeOffset()), TrustedImm32(StringType));
>-    load32(Address(value.payloadGPR(), JSString::offsetOfLength()), result);
>-    compare32(negateResult ? Equal : NotEqual, result, TrustedImm32(0), result);
>+    notDouble.link(this);
>+#if USE(JSVALUE64)
>+    static_assert(static_cast<int32_t>(ValueTrue) == ValueTrue, "");
>+    compare64(invert ? NotEqual : Equal, value.gpr(), TrustedImm32(ValueTrue), result);
>+#else
>+    move(invert ? TrustedImm32(1) : TrustedImm32(0), result);
>+    done.append(branchIfNotBoolean(value, InvalidGPRReg));
>+    compare32(invert ? Equal : NotEqual, value.payloadGPR(), TrustedImm32(0), result);
>+#endif
>+
>+    done.link(this);
>+}
>+
>+AssemblyHelpers::JumpList AssemblyHelpers::branchIfValue(VM& vm, JSValueRegs value, GPRReg scratch, GPRReg scratchIfShouldCheckMasqueradesAsUndefined, FPRReg valueAsFPR, FPRReg tempFPR, bool shouldCheckMasqueradesAsUndefined, JSGlobalObject* globalObject, bool invert)
>+{
>+    // Implements the following control flow structure:
>+    // if (value is cell) {
>+    //     if (value is string)
>+    //         result = !!value->length
>+    //     else {
>+    //         do evil things for masquerades-as-undefined
>+    //         result = true
>+    //     }
>+    // } else if (value is int32) {
>+    //     result = !!unboxInt32(value)
>+    // } else if (value is number) {
>+    //     result = !!unboxDouble(value)
>+    // } else {
>+    //     result = value == jsTrue
>+    // }
>+
>+    JumpList done;
>+    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);
>     if (shouldCheckMasqueradesAsUndefined) {
>-        ASSERT(scratch != InvalidGPRReg);
>+        isCellButNotString.link(this);
>+        ASSERT(scratchIfShouldCheckMasqueradesAsUndefined != InvalidGPRReg);
>         JumpList isNotMasqueradesAsUndefined;
>         isNotMasqueradesAsUndefined.append(branchTest8(Zero, Address(value.payloadGPR(), JSCell::typeInfoFlagsOffset()), TrustedImm32(MasqueradesAsUndefined)));
>-        emitLoadStructure(vm, value.payloadGPR(), result, scratch);
>-        move(TrustedImmPtr(globalObject), scratch);
>-        isNotMasqueradesAsUndefined.append(branchPtr(NotEqual, Address(result, Structure::globalObjectOffset()), scratch));
>+        emitLoadStructure(vm, value.payloadGPR(), scratch, scratchIfShouldCheckMasqueradesAsUndefined);
>+        move(TrustedImmPtr(globalObject), scratchIfShouldCheckMasqueradesAsUndefined);
>+        isNotMasqueradesAsUndefined.append(branchPtr(NotEqual, Address(scratch, Structure::globalObjectOffset()), scratchIfShouldCheckMasqueradesAsUndefined));
>+
>         // We act like we are "undefined" here.
>-        move(negateResult ? TrustedImm32(1) : TrustedImm32(0), result);
>-        done.append(jump());
>-        isNotMasqueradesAsUndefined.link(this);
>+        if (invert)
>+            truthy.append(jump());
>+        else
>+            done.append(jump());
>+
>+        if (invert)
>+            done.append(isNotMasqueradesAsUndefined);
>+        else
>+            truthy.append(isNotMasqueradesAsUndefined);
>+    } else {
>+        if (invert)
>+            done.append(isCellButNotString);
>+        else
>+            truthy.append(isCellButNotString);
>     }
>-    move(negateResult ? TrustedImm32(0) : TrustedImm32(1), result);
>+
>+    notCell.link(this);
>+    auto notInt32 = branchIfNotInt32(value);
>+    truthy.append(branchTest32(invert ? Zero : NonZero, value.payloadGPR()));
>     done.append(jump());
> 
>-    // null or undefined.
>-    isNotCellAndIsNotNumberAndIsNotBoolean.link(this); 
>-    move(negateResult ? TrustedImm32(1) : TrustedImm32(0), result);
>+    notInt32.link(this);
>+    auto notDouble = branchIfNotDoubleKnownNotInt32(value);
>+#if USE(JSVALUE64)
>+    unboxDouble(value.gpr(), scratch, valueAsFPR);
>+#else
>+    unboxDouble(value, valueAsFPR, tempFPR);
>+#endif
>+    if (invert) {
>+        truthy.append(branchDoubleZeroOrNaN(valueAsFPR, tempFPR));
>+        done.append(jump());
>+    } else {
>+        done.append(branchDoubleZeroOrNaN(valueAsFPR, tempFPR));
>+        truthy.append(jump());
>+    }
>+
>+    notDouble.link(this);
>+#if USE(JSVALUE64)
>+    truthy.append(branch64(invert ? NotEqual : Equal, value.gpr(), TrustedImm64(JSValue::encode(jsBoolean(true)))));
>+#else
>+    auto notBoolean = branchIfNotBoolean(value, InvalidGPRReg);
>+    if (invert)
>+        truthy.append(notBoolean);
>+    else
>+        done.append(notBoolean);
>+    truthy.append(branch32(invert ? Equal : NotEqual, value.payloadGPR(), TrustedImm32(0)));
>+#endif
> 
>     done.link(this);
>+
>+    return truthy;
> }
> 
> #if ENABLE(WEBASSEMBLY)
>diff --git a/Source/JavaScriptCore/jit/AssemblyHelpers.h b/Source/JavaScriptCore/jit/AssemblyHelpers.h
>index 1e387f688fcf4904e5fae3ca3c2619970f94642b..205ffefc86abd71a6ec31bee91f8c187ac40d6e9 100644
>--- a/Source/JavaScriptCore/jit/AssemblyHelpers.h
>+++ b/Source/JavaScriptCore/jit/AssemblyHelpers.h
>@@ -1758,6 +1758,15 @@ class AssemblyHelpers : public MacroAssembler {
>         storePtr(TrustedImmPtr(nullptr), Address(resultGPR, JSObject::butterflyOffset()));
>     }
> 
>+    JumpList branchIfValue(VM&, JSValueRegs value, GPRReg scratch, GPRReg scratchIfShouldCheckMasqueradesAsUndefined, FPRReg, FPRReg, bool shouldCheckMasqueradesAsUndefined, JSGlobalObject*, bool negateResult);
>+    JumpList branchIfTruthy(VM& vm, JSValueRegs value, GPRReg scratch, GPRReg scratchIfShouldCheckMasqueradesAsUndefined, FPRReg scratchFPR0, FPRReg scratchFPR1, bool shouldCheckMasqueradesAsUndefined, JSGlobalObject* globalObject)
>+    {
>+        return branchIfValue(vm, value, scratch, scratchIfShouldCheckMasqueradesAsUndefined, scratchFPR0, scratchFPR1, shouldCheckMasqueradesAsUndefined, globalObject, false);
>+    }
>+    JumpList branchIfFalsey(VM& vm, JSValueRegs value, GPRReg scratch, GPRReg scratchIfShouldCheckMasqueradesAsUndefined, FPRReg scratchFPR0, FPRReg scratchFPR1, bool shouldCheckMasqueradesAsUndefined, JSGlobalObject* globalObject)
>+    {
>+        return branchIfValue(vm, value, scratch, scratchIfShouldCheckMasqueradesAsUndefined, scratchFPR0, scratchFPR1, shouldCheckMasqueradesAsUndefined, globalObject, true);
>+    }
>     void emitConvertValueToBoolean(VM&, JSValueRegs value, GPRReg result, GPRReg scratchIfShouldCheckMasqueradesAsUndefined, FPRReg, FPRReg, bool shouldCheckMasqueradesAsUndefined, JSGlobalObject*, bool negateResult = false);
>     
>     template<typename ClassType>
>diff --git a/Source/JavaScriptCore/jit/JIT.h b/Source/JavaScriptCore/jit/JIT.h
>index cf203c6db4bcaee57e3decb8418445c54e30ce8d..593d5bf38be7f86a2ee275cac2a1846a974526c7 100644
>--- a/Source/JavaScriptCore/jit/JIT.h
>+++ b/Source/JavaScriptCore/jit/JIT.h
>@@ -305,6 +305,7 @@ namespace JSC {
>         void addSlowCase(const JumpList&);
>         void addSlowCase();
>         void addJump(Jump, int);
>+        void addJump(const JumpList&, int);
>         void emitJumpSlowToHot(Jump, int);
> 
>         void compileOpCall(OpcodeID, Instruction*, unsigned callLinkInfoIndex);
>diff --git a/Source/JavaScriptCore/jit/JITInlines.h b/Source/JavaScriptCore/jit/JITInlines.h
>index 723b73ed35d06c5699a221ec7ab4d558402b33ff..2bb67f13bede04d754b92c576af8a9fa0397c384 100644
>--- a/Source/JavaScriptCore/jit/JITInlines.h
>+++ b/Source/JavaScriptCore/jit/JITInlines.h
>@@ -235,6 +235,14 @@ ALWAYS_INLINE void JIT::addJump(Jump jump, int relativeOffset)
>     m_jmpTable.append(JumpTable(jump, m_bytecodeOffset + relativeOffset));
> }
> 
>+ALWAYS_INLINE void JIT::addJump(const JumpList& jumpList, int relativeOffset)
>+{
>+    ASSERT(m_bytecodeOffset != std::numeric_limits<unsigned>::max()); // This method should only be called during hot/cold path generation, so that m_bytecodeOffset is set.
>+
>+    for (auto& jump : jumpList.jumps())
>+        addJump(jump, relativeOffset);
>+}
>+
> ALWAYS_INLINE void JIT::emitJumpSlowToHot(Jump jump, int relativeOffset)
> {
>     ASSERT(m_bytecodeOffset != std::numeric_limits<unsigned>::max()); // This method should only be called during hot/cold path generation, so that m_bytecodeOffset is set.
>diff --git a/Source/JavaScriptCore/jit/JITOpcodes.cpp b/Source/JavaScriptCore/jit/JITOpcodes.cpp
>index 459d1a2db5a1dc1ad0c1b12203ee71fda2bca276..146dacf695a264d26b19b3adffa075232dafdc63 100644
>--- a/Source/JavaScriptCore/jit/JITOpcodes.cpp
>+++ b/Source/JavaScriptCore/jit/JITOpcodes.cpp
>@@ -351,14 +351,12 @@ void JIT::emit_op_jfalse(Instruction* currentInstruction)
>     unsigned target = currentInstruction[2].u.operand;
> 
>     GPRReg value = regT0;
>-    GPRReg result = regT1;
>-    GPRReg scratch = regT2;
>+    GPRReg scratch1 = regT1;
>+    GPRReg scratch2 = regT2;
>     bool shouldCheckMasqueradesAsUndefined = true;
> 
>     emitGetVirtualRegister(currentInstruction[1].u.operand, value);
>-    emitConvertValueToBoolean(*vm(), JSValueRegs(value), result, scratch, fpRegT0, fpRegT1, shouldCheckMasqueradesAsUndefined, m_codeBlock->globalObject());
>-
>-    addJump(branchTest32(Zero, result), target);
>+    addJump(branchIfFalsey(*vm(), JSValueRegs(value), scratch1, scratch2, fpRegT0, fpRegT1, shouldCheckMasqueradesAsUndefined, m_codeBlock->globalObject()), target);
> }
> 
> void JIT::emit_op_jeq_null(Instruction* currentInstruction)
>@@ -442,12 +440,11 @@ void JIT::emit_op_jtrue(Instruction* currentInstruction)
>     unsigned target = currentInstruction[2].u.operand;
> 
>     GPRReg value = regT0;
>-    GPRReg result = regT1;
>-    GPRReg scratch = regT2;
>+    GPRReg scratch1 = regT1;
>+    GPRReg scratch2 = regT2;
>     bool shouldCheckMasqueradesAsUndefined = true;
>     emitGetVirtualRegister(currentInstruction[1].u.operand, value);
>-    emitConvertValueToBoolean(*vm(), JSValueRegs(value), result, scratch, fpRegT0, fpRegT1, shouldCheckMasqueradesAsUndefined, m_codeBlock->globalObject());
>-    addJump(branchTest32(NonZero, result), target);
>+    addJump(branchIfTruthy(*vm(), JSValueRegs(value), scratch1, scratch2, fpRegT0, fpRegT1, shouldCheckMasqueradesAsUndefined, m_codeBlock->globalObject()), target);
> }
> 
> void JIT::emit_op_neq(Instruction* currentInstruction)
>diff --git a/Source/JavaScriptCore/jit/JITOpcodes32_64.cpp b/Source/JavaScriptCore/jit/JITOpcodes32_64.cpp
>index eee9d28465272315f38d97226ed7225e4bd2d80b..f4c016bbd10edc683fa02cfff1ef1f6ceacdcb79 100644
>--- a/Source/JavaScriptCore/jit/JITOpcodes32_64.cpp
>+++ b/Source/JavaScriptCore/jit/JITOpcodes32_64.cpp
>@@ -347,12 +347,10 @@ void JIT::emit_op_jfalse(Instruction* currentInstruction)
>     emitLoad(cond, regT1, regT0);
> 
>     JSValueRegs value(regT1, regT0);
>-    GPRReg scratch = regT2;
>-    GPRReg result = regT3;
>+    GPRReg scratch1 = regT2;
>+    GPRReg scratch2 = regT3;
>     bool shouldCheckMasqueradesAsUndefined = true;
>-    emitConvertValueToBoolean(*vm(), value, result, scratch, fpRegT0, fpRegT1, shouldCheckMasqueradesAsUndefined, m_codeBlock->globalObject());
>-
>-    addJump(branchTest32(Zero, result), target);
>+    addJump(branchIfFalsey(*vm(), value, scratch1, scratch2, fpRegT0, fpRegT1, shouldCheckMasqueradesAsUndefined, m_codeBlock->globalObject()), target);
> }
> 
> void JIT::emit_op_jtrue(Instruction* currentInstruction)
>@@ -363,11 +361,9 @@ void JIT::emit_op_jtrue(Instruction* currentInstruction)
>     emitLoad(cond, regT1, regT0);
>     bool shouldCheckMasqueradesAsUndefined = true;
>     JSValueRegs value(regT1, regT0);
>-    GPRReg scratch = regT2;
>-    GPRReg result = regT3;
>-    emitConvertValueToBoolean(*vm(), value, result, scratch, fpRegT0, fpRegT1, shouldCheckMasqueradesAsUndefined, m_codeBlock->globalObject());
>-
>-    addJump(branchTest32(NonZero, result), target);
>+    GPRReg scratch1 = regT2;
>+    GPRReg scratch2 = regT3;
>+    addJump(branchIfTruthy(*vm(), value, scratch1, scratch2, fpRegT0, fpRegT1, shouldCheckMasqueradesAsUndefined, m_codeBlock->globalObject()), target);
> }
> 
> void JIT::emit_op_jeq_null(Instruction* currentInstruction)
>diff --git a/JSTests/ChangeLog b/JSTests/ChangeLog
>index c6f6f12687bb2ecdf6995f63fb48c8b7403540d1..8b597b0ec8dbdb9c294d54488e24e972cd5d250e 100644
>--- a/JSTests/ChangeLog
>+++ b/JSTests/ChangeLog
>@@ -1,3 +1,17 @@
>+2018-06-01  Yusuke Suzuki  <utatane.tea@gmail.com>
>+
>+        Baseline op_jtrue emits an insane amount of code
>+        https://bugs.webkit.org/show_bug.cgi?id=185708
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * stress/logical-not-masquerades-as-undefined.js: Added.
>+        (shouldBe):
>+        (test):
>+        * stress/logical-not.js: Added.
>+        (shouldBe):
>+        (test):
>+
> 2018-05-31  Caio Lima  <ticaiolima@gmail.com>
> 
>         [ESNext][BigInt] Implement support for "=<" and ">=" relational operation
>diff --git a/JSTests/stress/logical-not-masquerades-as-undefined.js b/JSTests/stress/logical-not-masquerades-as-undefined.js
>new file mode 100644
>index 0000000000000000000000000000000000000000..3db7d1a94b120ac17c6399ad8b2bf40899e3df61
>--- /dev/null
>+++ b/JSTests/stress/logical-not-masquerades-as-undefined.js
>@@ -0,0 +1,35 @@
>+function shouldBe(actual, expected) {
>+    if (actual !== expected)
>+        throw new Error('bad value: ' + actual);
>+}
>+
>+function test(value)
>+{
>+    return !value;
>+}
>+noInline(test);
>+
>+var data = [
>+    [ {}, true ],
>+    [ true, true ],
>+    [ false, false ],
>+    [ -0, false ],
>+    [ 1, true ],
>+    [ 4.2, true ],
>+    [ NaN, false ],
>+    [ Infinity, true ],
>+    [ [], true ],
>+    [ new Date(), true ],
>+    [ "", false ],
>+    [ "" + "" + "", false ],
>+    [ "Cocoa", true ],
>+    [ undefined, false ],
>+    [ null, false ],
>+    [ Symbol(), true ],
>+    [ makeMasquerader() , false]
>+];
>+
>+for (var i = 0; i < 1e4; ++i) {
>+    for (let [ value, result ] of data)
>+        shouldBe(!test(value), result);
>+}
>diff --git a/JSTests/stress/logical-not.js b/JSTests/stress/logical-not.js
>new file mode 100644
>index 0000000000000000000000000000000000000000..aa90dcba5c70c6c6ca66e3bb5736f02892847843
>--- /dev/null
>+++ b/JSTests/stress/logical-not.js
>@@ -0,0 +1,34 @@
>+function shouldBe(actual, expected) {
>+    if (actual !== expected)
>+        throw new Error('bad value: ' + actual);
>+}
>+
>+function test(value)
>+{
>+    return !value;
>+}
>+noInline(test);
>+
>+var data = [
>+    [ {}, true ],
>+    [ true, true ],
>+    [ false, false ],
>+    [ -0, false ],
>+    [ 1, true ],
>+    [ 4.2, true ],
>+    [ NaN, false ],
>+    [ Infinity, true ],
>+    [ [], true ],
>+    [ new Date(), true ],
>+    [ "", false ],
>+    [ "" + "" + "", false ],
>+    [ "Cocoa", true ],
>+    [ undefined, false ],
>+    [ null, false ],
>+    [ Symbol(), true ],
>+];
>+
>+for (var i = 0; i < 1e4; ++i) {
>+    for (let [ value, result ] of data)
>+        shouldBe(!test(value), result);
>+}

Flags:

fpizlo: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 185708: 341752