Attachment 369670 Details for Bug 197822 – Patch

[patch] Patch

bug-197822-20190512041303.patch (text/plain), 18.20 KB, created by Yusuke Suzuki on 2019-05-12 04:13:04 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Yusuke Suzuki

Created: 2019-05-12 04:13:04 PDT

Size: 18.20 KB

patch

obsolete

>Subversion Revision: 245212
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index b3ecb66cabd844cd19a132cc938e16ed48a62234..a8f356cd2fc94e0095ebda5480326e2d89c8f253 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,41 @@
>+2019-05-12  Yusuke Suzuki  <ysuzuki@apple.com>
>+
>+        [JSC] Compact generator code's bytecode size
>+        https://bugs.webkit.org/show_bug.cgi?id=197822
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        op_put_to_scope's symbolTableOrScopeDepth is represented as int. This was OK for the old bytecode format since
>+        VirtualRegister / scope depth can be represented by int anyway. But it is problematic now since only int8_t range
>+        will be represented in narrow bytecode. When this field is used for symbol table constant index, it is always
>+        larger than FirstConstantRegisterIndex. So it always exceeds the range of int8_t, and results in wide bytecode.
>+        It makes all generator's op_put_to_scope wide bytecode.
>+
>+        In this patch, we introduce a new union type SymbolTableOrScopeDepth. It holds unsigned value, and we store the
>+        SymbolTableConstantIndex - FirstConstantRegisterIndex in this unsigned value to make op_put_to_scope narrow bytecode.
>+
>+        * bytecode/BytecodeGeneratorification.cpp:
>+        (JSC::BytecodeGeneratorification::run):
>+        * bytecode/BytecodeList.rb:
>+        * bytecode/CodeBlock.cpp:
>+        (JSC::CodeBlock::finishCreation):
>+        * bytecode/Fits.h:
>+        * bytecompiler/BytecodeGenerator.cpp:
>+        (JSC::BytecodeGenerator::BytecodeGenerator):
>+        (JSC::BytecodeGenerator::emitProfileType):
>+        (JSC::BytecodeGenerator::emitPutToScope):
>+        (JSC::BytecodeGenerator::localScopeDepth const):
>+        * bytecompiler/BytecodeGenerator.h:
>+        * runtime/SymbolTableOrScopeDepth.h: Added.
>+        (JSC::SymbolTableOrScopeDepth::symbolTable):
>+        (JSC::SymbolTableOrScopeDepth::scopeDepth):
>+        (JSC::SymbolTableOrScopeDepth::raw):
>+        (JSC::SymbolTableOrScopeDepth::symbolTable const):
>+        (JSC::SymbolTableOrScopeDepth::scopeDepth const):
>+        (JSC::SymbolTableOrScopeDepth::raw const):
>+        (JSC::SymbolTableOrScopeDepth::dump const):
>+        (JSC::SymbolTableOrScopeDepth::SymbolTableOrScopeDepth):
>+
> 2019-05-10  Saam barati  <sbarati@apple.com>
> 
>         Call to JSToWasmICCallee::createStructure passes in wrong prototype value
>diff --git a/Source/JavaScriptCore/bytecode/BytecodeGeneratorification.cpp b/Source/JavaScriptCore/bytecode/BytecodeGeneratorification.cpp
>index 2e6dcc828cc84382b653e9f0f49d8d9ca39cfbfe..24125b75a4c822e3bb38724bcbf3d8bdb86f490f 100644
>--- a/Source/JavaScriptCore/bytecode/BytecodeGeneratorification.cpp
>+++ b/Source/JavaScriptCore/bytecode/BytecodeGeneratorification.cpp
>@@ -247,12 +247,13 @@ void BytecodeGeneratorification::run()
>                 VirtualRegister operand = virtualRegisterForLocal(index);
>                 Storage storage = storageForGeneratorLocal(vm, index);
> 
>+                dataLogLn(m_generatorFrameSymbolTableIndex, " ", FirstConstantRegisterIndex);
>                 fragment.appendInstruction<OpPutToScope>(
>                     scope, // scope
>                     storage.identifierIndex, // identifier
>                     operand, // value
>                     GetPutInfo(DoNotThrowIfNotFound, LocalClosureVar, InitializationMode::NotInitialization), // info
>-                    m_generatorFrameSymbolTableIndex, // symbol table constant index
>+                    SymbolTableOrScopeDepth::symbolTable(VirtualRegister { m_generatorFrameSymbolTableIndex }), // symbol table constant index
>                     storage.scopeOffset.offset() // scope offset
>                 );
>             });
>diff --git a/Source/JavaScriptCore/bytecode/BytecodeList.rb b/Source/JavaScriptCore/bytecode/BytecodeList.rb
>index 8ca0da9c30f52c2e6b190f399ef5147c395707db..0695a2507b62397649a87989616755c0939377ab 100644
>--- a/Source/JavaScriptCore/bytecode/BytecodeList.rb
>+++ b/Source/JavaScriptCore/bytecode/BytecodeList.rb
>@@ -50,6 +50,7 @@
>     :StructureID,
>     :StructureChain,
>     :SymbolTable,
>+    :SymbolTableOrScopeDepth,
>     :ToThisStatus,
>     :TypeLocation,
>     :WatchpointSet,
>@@ -881,7 +882,7 @@
>         value: VirtualRegister, # offset 3
>         # $begin: :private,
>         getPutInfo: GetPutInfo,
>-        symbolTableOrScopeDepth: int,
>+        symbolTableOrScopeDepth: SymbolTableOrScopeDepth,
>         offset: unsigned,
>     },
>     metadata: {
>@@ -977,7 +978,7 @@
> op :profile_type,
>     args: {
>         targetVirtualRegister: VirtualRegister,
>-        symbolTableOrScopeDepth: int,
>+        symbolTableOrScopeDepth: SymbolTableOrScopeDepth,
>         flag: ProfileTypeBytecodeFlag,
>         identifier?: unsigned,
>         resolveType: ResolveType,
>diff --git a/Source/JavaScriptCore/bytecode/CodeBlock.cpp b/Source/JavaScriptCore/bytecode/CodeBlock.cpp
>index db28f67e2d17b30395500484e86945a7ce1e9d8a..ecc1a95095f7beff6169eea83c267d447b2bcabc 100644
>--- a/Source/JavaScriptCore/bytecode/CodeBlock.cpp
>+++ b/Source/JavaScriptCore/bytecode/CodeBlock.cpp
>@@ -643,7 +643,7 @@ bool CodeBlock::finishCreation(VM& vm, ScriptExecutable* ownerExecutable, Unlink
>             if (bytecode.m_getPutInfo.resolveType() == LocalClosureVar) {
>                 // Only do watching if the property we're putting to is not anonymous.
>                 if (bytecode.m_var != UINT_MAX) {
>-                    SymbolTable* symbolTable = jsCast<SymbolTable*>(getConstant(bytecode.m_symbolTableOrScopeDepth));
>+                    SymbolTable* symbolTable = jsCast<SymbolTable*>(getConstant(bytecode.m_symbolTableOrScopeDepth.symbolTable().offset()));
>                     const Identifier& ident = identifier(bytecode.m_var);
>                     ConcurrentJSLocker locker(symbolTable->m_lock);
>                     auto iter = symbolTable->find(locker, ident.impl());
>@@ -657,7 +657,7 @@ bool CodeBlock::finishCreation(VM& vm, ScriptExecutable* ownerExecutable, Unlink
> 
>             const Identifier& ident = identifier(bytecode.m_var);
>             metadata.m_watchpointSet = nullptr;
>-            ResolveOp op = JSScope::abstractResolve(m_globalObject->globalExec(), bytecode.m_symbolTableOrScopeDepth, scope, ident, Put, bytecode.m_getPutInfo.resolveType(), bytecode.m_getPutInfo.initializationMode());
>+            ResolveOp op = JSScope::abstractResolve(m_globalObject->globalExec(), bytecode.m_symbolTableOrScopeDepth.scopeDepth(), scope, ident, Put, bytecode.m_getPutInfo.resolveType(), bytecode.m_getPutInfo.initializationMode());
>             RETURN_IF_EXCEPTION(throwScope, false);
> 
>             metadata.m_getPutInfo = GetPutInfo(bytecode.m_getPutInfo.resolveMode(), op.type, bytecode.m_getPutInfo.initializationMode());
>@@ -687,7 +687,7 @@ bool CodeBlock::finishCreation(VM& vm, ScriptExecutable* ownerExecutable, Unlink
>             switch (bytecode.m_flag) {
>             case ProfileTypeBytecodeClosureVar: {
>                 const Identifier& ident = identifier(bytecode.m_identifier);
>-                unsigned localScopeDepth = bytecode.m_symbolTableOrScopeDepth;
>+                unsigned localScopeDepth = bytecode.m_symbolTableOrScopeDepth.scopeDepth();
>                 // Even though type profiling may be profiling either a Get or a Put, we can always claim a Get because
>                 // we're abstractly "read"ing from a JSScope.
>                 ResolveOp op = JSScope::abstractResolve(m_globalObject->globalExec(), localScopeDepth, scope, ident, Get, bytecode.m_resolveType, InitializationMode::NotInitialization);
>@@ -711,7 +711,7 @@ bool CodeBlock::finishCreation(VM& vm, ScriptExecutable* ownerExecutable, Unlink
>                 break;
>             }
>             case ProfileTypeBytecodeLocallyResolved: {
>-                int symbolTableIndex = bytecode.m_symbolTableOrScopeDepth;
>+                int symbolTableIndex = bytecode.m_symbolTableOrScopeDepth.symbolTable().offset();
>                 SymbolTable* symbolTable = jsCast<SymbolTable*>(getConstant(symbolTableIndex));
>                 const Identifier& ident = identifier(bytecode.m_identifier);
>                 ConcurrentJSLocker locker(symbolTable->m_lock);
>diff --git a/Source/JavaScriptCore/bytecode/Fits.h b/Source/JavaScriptCore/bytecode/Fits.h
>index 207e8853fa35edaeda768f28d9e240d1e0c510ca..24d7757c979465ef28018ad3b7c91f339c82a1d7 100644
>--- a/Source/JavaScriptCore/bytecode/Fits.h
>+++ b/Source/JavaScriptCore/bytecode/Fits.h
>@@ -33,6 +33,7 @@
> #include "PutByIdFlags.h"
> #include "ResultType.h"
> #include "SpecialPointer.h"
>+#include "SymbolTableOrScopeDepth.h"
> #include "VirtualRegister.h"
> #include <type_traits>
> 
>@@ -132,6 +133,25 @@ struct Fits<VirtualRegister, OpcodeSize::Narrow> {
>     }
> };
> 
>+template<>
>+struct Fits<SymbolTableOrScopeDepth, OpcodeSize::Narrow> {
>+    static bool check(SymbolTableOrScopeDepth u)
>+    {
>+        return u.raw() <= UINT8_MAX;
>+    }
>+
>+    static uint8_t convert(SymbolTableOrScopeDepth u)
>+    {
>+        ASSERT(check(u));
>+        return static_cast<uint8_t>(u.raw());
>+    }
>+
>+    static SymbolTableOrScopeDepth convert(uint8_t u)
>+    {
>+        return SymbolTableOrScopeDepth::raw(u);
>+    }
>+};
>+
> template<>
> struct Fits<Special::Pointer, OpcodeSize::Narrow> : Fits<int, OpcodeSize::Narrow> {
>     using Base = Fits<int, OpcodeSize::Narrow>;
>diff --git a/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp b/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp
>index 661cb49d3f3d34da4d48db203da617ab22a08556..667dac047e187ed7e6db983af30611b4efd7ae39 100644
>--- a/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp
>+++ b/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp
>@@ -551,7 +551,7 @@ BytecodeGenerator::BytecodeGenerator(VM& vm, FunctionNode* functionNode, Unlinke
>                     entry.disableWatching(*m_vm);
>                     functionSymbolTable->set(NoLockingNecessary, name, entry);
>                 }
>-                OpPutToScope::emit(this, m_lexicalEnvironmentRegister, UINT_MAX, virtualRegisterForArgument(1 + i), GetPutInfo(ThrowIfNotFound, LocalClosureVar, InitializationMode::NotInitialization), symbolTableConstantIndex, offset.offset());
>+                OpPutToScope::emit(this, m_lexicalEnvironmentRegister, UINT_MAX, virtualRegisterForArgument(1 + i), GetPutInfo(ThrowIfNotFound, LocalClosureVar, InitializationMode::NotInitialization), SymbolTableOrScopeDepth::symbolTable(VirtualRegister { symbolTableConstantIndex }), offset.offset());
>             }
>             
>             // This creates a scoped arguments object and copies the overflow arguments into the
>@@ -589,7 +589,7 @@ BytecodeGenerator::BytecodeGenerator(VM& vm, FunctionNode* functionNode, Unlinke
>                 static_cast<const BindingNode*>(parameters.at(i).first)->boundProperty();
>             functionSymbolTable->set(NoLockingNecessary, name, SymbolTableEntry(VarOffset(offset)));
>             
>-            OpPutToScope::emit(this, m_lexicalEnvironmentRegister, addConstant(ident), virtualRegisterForArgument(1 + i), GetPutInfo(ThrowIfNotFound, LocalClosureVar, InitializationMode::NotInitialization), symbolTableConstantIndex, offset.offset());
>+            OpPutToScope::emit(this, m_lexicalEnvironmentRegister, addConstant(ident), virtualRegisterForArgument(1 + i), GetPutInfo(ThrowIfNotFound, LocalClosureVar, InitializationMode::NotInitialization), SymbolTableOrScopeDepth::symbolTable(VirtualRegister { symbolTableConstantIndex }), offset.offset());
>         }
>     }
>     
>@@ -1803,7 +1803,7 @@ void BytecodeGenerator::emitProfileType(RegisterID* registerToProfile, ProfileTy
>     if (!registerToProfile)
>         return;
> 
>-    OpProfileType::emit(this, registerToProfile, 0, flag, { }, resolveType());
>+    OpProfileType::emit(this, registerToProfile, { }, flag, { }, resolveType());
> 
>     // Don't emit expression info for this version of profile type. This generally means
>     // we're profiling information for something that isn't in the actual text of a JavaScript
>@@ -1823,7 +1823,7 @@ void BytecodeGenerator::emitProfileType(RegisterID* registerToProfile, ProfileTy
>     if (!registerToProfile)
>         return;
> 
>-    OpProfileType::emit(this, registerToProfile, 0,  flag, { }, resolveType());
>+    OpProfileType::emit(this, registerToProfile, { },  flag, { }, resolveType());
>     emitTypeProfilerExpressionInfo(startDivot, endDivot);
> }
> 
>@@ -1836,14 +1836,14 @@ void BytecodeGenerator::emitProfileType(RegisterID* registerToProfile, const Var
>         return;
> 
>     ProfileTypeBytecodeFlag flag;
>-    int symbolTableOrScopeDepth;
>+    SymbolTableOrScopeDepth symbolTableOrScopeDepth;
>     if (var.local() || var.offset().isScope()) {
>         flag = ProfileTypeBytecodeLocallyResolved;
>         ASSERT(var.symbolTableConstantIndex());
>-        symbolTableOrScopeDepth = var.symbolTableConstantIndex();
>+        symbolTableOrScopeDepth = SymbolTableOrScopeDepth::symbolTable(VirtualRegister { var.symbolTableConstantIndex() });
>     } else {
>         flag = ProfileTypeBytecodeClosureVar;
>-        symbolTableOrScopeDepth = localScopeDepth();
>+        symbolTableOrScopeDepth = SymbolTableOrScopeDepth::scopeDepth(localScopeDepth());
>     }
> 
>     OpProfileType::emit(this, registerToProfile, symbolTableOrScopeDepth, flag, addConstant(var.ident()), resolveType());
>@@ -2512,18 +2512,18 @@ RegisterID* BytecodeGenerator::emitPutToScope(RegisterID* scope, const Variable&
>     case VarKind::Scope:
>     case VarKind::Invalid: {
>         GetPutInfo getPutInfo(0);
>-        int scopeDepth;
>+        SymbolTableOrScopeDepth symbolTableOrScopeDepth;
>         ScopeOffset offset;
>         if (variable.offset().isScope()) {
>             offset = variable.offset().scopeOffset();
>             getPutInfo = GetPutInfo(resolveMode, LocalClosureVar, initializationMode);
>-            scopeDepth = variable.symbolTableConstantIndex();
>+            symbolTableOrScopeDepth = SymbolTableOrScopeDepth::symbolTable(VirtualRegister { variable.symbolTableConstantIndex() });
>         } else {
>             ASSERT(resolveType() != LocalClosureVar);
>             getPutInfo = GetPutInfo(resolveMode, resolveType(), initializationMode);
>-            scopeDepth = localScopeDepth();
>+            symbolTableOrScopeDepth = SymbolTableOrScopeDepth::scopeDepth(localScopeDepth());
>         }
>-        OpPutToScope::emit(this, scope, addConstant(variable.ident()), value, getPutInfo, scopeDepth, !!offset ? offset.offset() : 0);
>+        OpPutToScope::emit(this, scope, addConstant(variable.ident()), value, getPutInfo, symbolTableOrScopeDepth, !!offset ? offset.offset() : 0);
>         m_codeBlock->addPropertyAccessInstruction(m_lastInstruction.offset());
>         return value;
>     } }
>@@ -3743,7 +3743,7 @@ RegisterID* BytecodeGenerator::emitArgumentCount(RegisterID* dst)
>     return dst;
> }
> 
>-int BytecodeGenerator::localScopeDepth() const
>+unsigned BytecodeGenerator::localScopeDepth() const
> {
>     return m_localScopeDepth;
> }
>diff --git a/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h b/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h
>index 95341a9483f5ef5ff0a70d4fea9aa3795a93f515..1c90313c1affb5950ba5dc69afbffac8ce4aa6d8 100644
>--- a/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h
>+++ b/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h
>@@ -1235,10 +1235,10 @@ namespace JSC {
>         SegmentedVector<Label, 32> m_labels;
>         SegmentedVector<LabelScope, 32> m_labelScopes;
>         unsigned m_finallyDepth { 0 };
>-        int m_localScopeDepth { 0 };
>+        unsigned m_localScopeDepth { 0 };
>         const CodeType m_codeType;
> 
>-        int localScopeDepth() const;
>+        unsigned localScopeDepth() const;
>         void pushLocalControlFlowScope();
>         void popLocalControlFlowScope();
> 
>diff --git a/Source/JavaScriptCore/runtime/SymbolTableOrScopeDepth.h b/Source/JavaScriptCore/runtime/SymbolTableOrScopeDepth.h
>new file mode 100644
>index 0000000000000000000000000000000000000000..91f5e8f07f2a1bc14372c09992fdcb663ca38409
>--- /dev/null
>+++ b/Source/JavaScriptCore/runtime/SymbolTableOrScopeDepth.h
>@@ -0,0 +1,78 @@
>+/*
>+ * Copyright (C) 2019 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
>+
>+#include "VirtualRegister.h"
>+#include <wtf/PrintStream.h>
>+
>+namespace JSC {
>+
>+class SymbolTableOrScopeDepth {
>+public:
>+    SymbolTableOrScopeDepth() = default;
>+
>+    static SymbolTableOrScopeDepth symbolTable(VirtualRegister reg)
>+    {
>+        ASSERT(reg.isConstant());
>+        return SymbolTableOrScopeDepth(reg.offset() - FirstConstantRegisterIndex);
>+    }
>+
>+    static SymbolTableOrScopeDepth scopeDepth(unsigned scopeDepth)
>+    {
>+        return SymbolTableOrScopeDepth(scopeDepth);
>+    }
>+
>+    static SymbolTableOrScopeDepth raw(unsigned value)
>+    {
>+        return SymbolTableOrScopeDepth(value);
>+    }
>+
>+    VirtualRegister symbolTable() const
>+    {
>+        return VirtualRegister(m_raw + FirstConstantRegisterIndex);
>+    }
>+
>+    unsigned scopeDepth() const
>+    {
>+        return m_raw;
>+    }
>+
>+    unsigned raw() const { return m_raw; }
>+
>+    void dump(PrintStream& out) const
>+    {
>+        out.print(m_raw);
>+    }
>+
>+private:
>+    SymbolTableOrScopeDepth(unsigned value)
>+        : m_raw(value)
>+    { }
>+
>+    unsigned m_raw { 0 };
>+};
>+
>+} // namespace JSC

Actions: View | Formatted Diff | Diff

Attachments on bug 197822: 369670 | 369671 | 369672 | 369673