Implement a faster Interpreter::getOpcodeID().

https://bugs.webkit.org/show_bug.cgi?id=172669

Reviewed by Saam Barati.

Source/JavaScriptCore:

We can implement Interpreter::getOpcodeID() without a hash table lookup by always
embedding the OpcodeID in the 32-bit word just before the start of the LLInt
handler code that executes each opcode.  getOpcodeID() can therefore just read
the 32-bits before the opcode address to get its OpcodeID.

This is currently only enabled for CPU(X86), CPU(X86_64), CPU(ARM64),
CPU(ARM_THUMB2), and only for OS(DARWIN).  It'll probably just work for linux as
well, but I'll let the Linux folks turn that on after they have verified that it
works on linux too.

I'll also take this opportunity to clean up how we initialize the opcodeIDTable:
1. we only need to initialize it once per process, not once per VM / interpreter
   instance.
2. we can initialize it in the Interpreter constructor instead of requiring a
   separate call to an initialize() function.

On debug builds, the Interpreter constructor will also verify that getOpcodeID()
is working correctly for each opcode when USE(LLINT_EMBEDDED_OPCODE_ID).

* bytecode/BytecodeList.json:
* generate-bytecode-files:
* interpreter/Interpreter.cpp:
(JSC::Interpreter::Interpreter):
(JSC::Interpreter::opcodeIDTable):
(JSC::Interpreter::initialize): Deleted.
* interpreter/Interpreter.h:
(JSC::Interpreter::getOpcode):
(JSC::Interpreter::getOpcodeID):
* llint/LowLevelInterpreter.cpp:
* runtime/VM.cpp:
(JSC::VM::VM):

Source/WTF:

Added the USE(LLINT_EMBEDDED_OPCODE_ID) configuration.

* wtf/Platform.h:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@217526 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
index fc273e1..8f86c9b 100644 (file)
--- a/Source/JavaScriptCore/ChangeLog
+++ b/Source/JavaScriptCore/ChangeLog
@@ -1,3 +1,42 @@
+2017-05-28  Mark Lam  <mark.lam@apple.com>
+
+        Implement a faster Interpreter::getOpcodeID().
+        https://bugs.webkit.org/show_bug.cgi?id=172669
+
+        Reviewed by Saam Barati.
+
+        We can implement Interpreter::getOpcodeID() without a hash table lookup by always
+        embedding the OpcodeID in the 32-bit word just before the start of the LLInt
+        handler code that executes each opcode.  getOpcodeID() can therefore just read
+        the 32-bits before the opcode address to get its OpcodeID.
+
+        This is currently only enabled for CPU(X86), CPU(X86_64), CPU(ARM64),
+        CPU(ARM_THUMB2), and only for OS(DARWIN).  It'll probably just work for linux as
+        well, but I'll let the Linux folks turn that on after they have verified that it
+        works on linux too.
+
+        I'll also take this opportunity to clean up how we initialize the opcodeIDTable:
+        1. we only need to initialize it once per process, not once per VM / interpreter
+           instance.
+        2. we can initialize it in the Interpreter constructor instead of requiring a
+           separate call to an initialize() function.
+
+        On debug builds, the Interpreter constructor will also verify that getOpcodeID()
+        is working correctly for each opcode when USE(LLINT_EMBEDDED_OPCODE_ID).
+
+        * bytecode/BytecodeList.json:
+        * generate-bytecode-files:
+        * interpreter/Interpreter.cpp:
+        (JSC::Interpreter::Interpreter):
+        (JSC::Interpreter::opcodeIDTable):
+        (JSC::Interpreter::initialize): Deleted.
+        * interpreter/Interpreter.h:
+        (JSC::Interpreter::getOpcode):
+        (JSC::Interpreter::getOpcodeID):
+        * llint/LowLevelInterpreter.cpp:
+        * runtime/VM.cpp:
+        (JSC::VM::VM):
+
 2017-05-27  Yusuke Suzuki  <utatane.tea@gmail.com>
 
         [JSC] Map and Set constructors should have fast path for cloning

diff --git a/Source/JavaScriptCore/bytecode/BytecodeList.json b/Source/JavaScriptCore/bytecode/BytecodeList.json
index 11b9a65..d7d624f 100644 (file)
--- a/Source/JavaScriptCore/bytecode/BytecodeList.json
+++ b/Source/JavaScriptCore/bytecode/BytecodeList.json
@@ -1,6 +1,6 @@
 [
     {
-        "section" : "Bytecodes", "emitInHFile" : true, "emitInASMFile" : true, 
+        "section" : "Bytecodes", "emitInHFile" : true, "emitInASMFile" : true, "emitOpcodeIDStringValuesInHFile" : true,
         "macroNameComponent" : "BYTECODE", "asmPrefix" : "llint_", 
         "bytecodes" : [
             { "name" : "op_enter", "length" : 1 },
@@ -158,7 +158,7 @@
         ]
     },
     {
-        "section" : "CLoopHelpers", "emitInHFile" : true, "emitInASMFile" : false, "defaultLength" : 1,
+        "section" : "CLoopHelpers", "emitInHFile" : true, "emitInASMFile" : false, "emitOpcodeIDStringValuesInHFile" : false, "defaultLength" : 1,
         "macroNameComponent" : "CLOOP_BYTECODE_HELPER",
         "bytecodes" : [
             { "name" : "llint_entry" },
@@ -181,7 +181,7 @@
         ]
     },
     {
-        "section" : "NativeHelpers", "emitInHFile" : true, "emitInASMFile" : true, "defaultLength" : 1,
+        "section" : "NativeHelpers", "emitInHFile" : true, "emitInASMFile" : true, "emitOpcodeIDStringValuesInHFile" : false, "defaultLength" : 1,
         "macroNameComponent" : "BYTECODE_HELPER",
         "bytecodes" : [
             { "name" : "llint_program_prologue" },

diff --git a/Source/JavaScriptCore/generate-bytecode-files b/Source/JavaScriptCore/generate-bytecode-files
index 5b2fce8..71b4f80 100644 (file)
--- a/Source/JavaScriptCore/generate-bytecode-files
+++ b/Source/JavaScriptCore/generate-bytecode-files
@@ -1,6 +1,6 @@
 #! /usr/bin/python
 
-# Copyright (C) 2014 Apple Inc. All rights reserved.
+# Copyright (C) 2014-2017 Apple Inc. All rights reserved.
 #
 # Redistribution and use in source and binary forms, with or without
 # modification, are permitted provided that the following conditions
@@ -201,6 +201,15 @@ if __name__ == "__main__":
             bytecodeHFile.write("\n\n")
             bytecodeHFile.write("#define NUMBER_OF_{0}_IDS {1}\n\n".format(section["macroNameComponent"], bytecodeNum))
 
+        if bytecodeHFilename and section['emitOpcodeIDStringValuesInHFile']:
+            bytecodeNum = 0
+            for bytecode in section["bytecodes"]:
+                bytecodeHFile.write("#define {0}_value_string \"{1}\"\n".format(bytecode["name"], bytecodeNum))
+                firstMacro = False
+                bytecodeNum = bytecodeNum + 1
+
+            bytecodeHFile.write("\n")            
+
         if initASMFileName and section['emitInASMFile']:
             prefix = ""
             if "asmPrefix" in section:

diff --git a/Source/JavaScriptCore/interpreter/Interpreter.cpp b/Source/JavaScriptCore/interpreter/Interpreter.cpp
index cb4bbba..b46c55c 100644 (file)
--- a/Source/JavaScriptCore/interpreter/Interpreter.cpp
+++ b/Source/JavaScriptCore/interpreter/Interpreter.cpp
@@ -31,6 +31,7 @@
 #include "Interpreter.h"
 
 #include "BatchedTransitionOptimizer.h"
+#include "Bytecodes.h"
 #include "CallFrameClosure.h"
 #include "CodeBlock.h"
 #include "DirectArguments.h"
@@ -75,6 +76,7 @@
 
 #include <limits.h>
 #include <stdio.h>
+#include <wtf/NeverDestroyed.h>
 #include <wtf/StackStats.h>
 #include <wtf/StdLibExtras.h>
 #include <wtf/StringPrintStream.h>
@@ -315,28 +317,43 @@ Interpreter::Interpreter(VM& vm)
 #if !ENABLE(JIT)
     , m_cloopStack(vm)
 #endif
-#if !ASSERT_DISABLED
-    , m_initialized(false)
+#if ENABLE(COMPUTED_GOTO_OPCODES)
+    , m_opcodeTable { LLInt::opcodeMap() }
+#if !USE(LLINT_EMBEDDED_OPCODE_ID) || !ASSERT_DISABLED
+    , m_opcodeIDTable { opcodeIDTable() }
+#endif
 #endif
 {
+#if !ASSERT_DISABLED
+    static std::once_flag assertOnceKey;
+    std::call_once(assertOnceKey, [this] {
+        for (unsigned i = 0; i < NUMBER_OF_BYTECODE_IDS; ++i)
+            RELEASE_ASSERT(getOpcodeID(m_opcodeTable[i]) == static_cast<OpcodeID>(i));
+    });
+#endif // USE(LLINT_EMBEDDED_OPCODE_ID)
 }
 
 Interpreter::~Interpreter()
 {
 }
 
-void Interpreter::initialize()
-{
 #if ENABLE(COMPUTED_GOTO_OPCODES)
-    m_opcodeTable = LLInt::opcodeMap();
-    for (int i = 0; i < numOpcodeIDs; ++i)
-        m_opcodeIDTable.add(m_opcodeTable[i], static_cast<OpcodeID>(i));
-#endif
+#if !USE(LLINT_EMBEDDED_OPCODE_ID) || !ASSERT_DISABLED
+HashMap<Opcode, OpcodeID>& Interpreter::opcodeIDTable()
+{
+    static NeverDestroyed<HashMap<Opcode, OpcodeID>> opcodeIDTable;
 
-#if !ASSERT_DISABLED
-    m_initialized = true;
-#endif
+    static std::once_flag initializeKey;
+    std::call_once(initializeKey, [&] {
+        const Opcode* opcodeTable = LLInt::opcodeMap();
+        for (unsigned i = 0; i < NUMBER_OF_BYTECODE_IDS; ++i)
+            opcodeIDTable.get().add(opcodeTable[i], static_cast<OpcodeID>(i));
+    });
+
+    return opcodeIDTable;
 }
+#endif // !USE(LLINT_EMBEDDED_OPCODE_ID) || !ASSERT_DISABLED
+#endif // ENABLE(COMPUTED_GOTO_OPCODES)
 
 #ifdef NDEBUG
 
@@ -449,6 +466,7 @@ void Interpreter::dumpRegisters(CallFrame* callFrame)
 
 #endif
 
+#if !ASSERT_DISABLED
 bool Interpreter::isOpcode(Opcode opcode)
 {
 #if ENABLE(COMPUTED_GOTO_OPCODES)
@@ -459,6 +477,7 @@ bool Interpreter::isOpcode(Opcode opcode)
     return opcode >= 0 && opcode <= op_end;
 #endif
 }
+#endif // !ASSERT_DISABLED
 
 class GetStackTraceFunctor {
 public:

diff --git a/Source/JavaScriptCore/interpreter/Interpreter.h b/Source/JavaScriptCore/interpreter/Interpreter.h
index d5e7032..bbb5018 100644 (file)
--- a/Source/JavaScriptCore/interpreter/Interpreter.h
+++ b/Source/JavaScriptCore/interpreter/Interpreter.h
@@ -97,15 +97,12 @@ namespace JSC {
         Interpreter(VM &);
         ~Interpreter();
         
-        void initialize();
-
 #if !ENABLE(JIT)
         CLoopStack& cloopStack() { return m_cloopStack; }
 #endif
         
         Opcode getOpcode(OpcodeID id)
         {
-            ASSERT(m_initialized);
 #if ENABLE(COMPUTED_GOTO_OPCODES)
             return m_opcodeTable[id];
 #else
@@ -115,11 +112,22 @@ namespace JSC {
 
         OpcodeID getOpcodeID(Opcode opcode)
         {
-            ASSERT(m_initialized);
 #if ENABLE(COMPUTED_GOTO_OPCODES)
             ASSERT(isOpcode(opcode));
-            return m_opcodeIDTable.get(opcode);
+#if USE(LLINT_EMBEDDED_OPCODE_ID)
+            // The OpcodeID is embedded in the int32_t word preceding the location of
+            // the LLInt code for the opcode (see the EMBED_OPCODE_ID_IF_NEEDED macro
+            // in LowLevelInterpreter.cpp).
+            MacroAssemblerCodePtr codePtr(reinterpret_cast<void*>(opcode));
+            int32_t* opcodeIDAddress = reinterpret_cast<int32_t*>(codePtr.dataLocation()) - 1;
+            OpcodeID opcodeID = static_cast<OpcodeID>(*opcodeIDAddress);
+            ASSERT(opcodeID < NUMBER_OF_BYTECODE_IDS);
+            return opcodeID;
 #else
+            return m_opcodeIDTable.get(opcode);
+#endif // USE(LLINT_EMBEDDED_OPCODE_ID)
+
+#else // not ENABLE(COMPUTED_GOTO_OPCODES)
             return opcode;
 #endif
         }
@@ -127,7 +135,9 @@ namespace JSC {
         OpcodeID getOpcodeID(const Instruction&);
         OpcodeID getOpcodeID(const UnlinkedInstruction&);
 
+#if !ASSERT_DISABLED
         bool isOpcode(Opcode);
+#endif
 
         JSValue executeProgram(const SourceCode&, CallFrame*, JSObject* thisObj);
         JSValue executeModuleProgram(ModuleProgramExecutable*, CallFrame*, JSModuleEnvironment*);
@@ -170,13 +180,14 @@ namespace JSC {
 #endif
         
 #if ENABLE(COMPUTED_GOTO_OPCODES)
-        Opcode* m_opcodeTable; // Maps OpcodeID => Opcode for compiling
-        HashMap<Opcode, OpcodeID> m_opcodeIDTable; // Maps Opcode => OpcodeID for decompiling
-#endif
+        const Opcode* m_opcodeTable; // Maps OpcodeID => Opcode for compiling
 
-#if !ASSERT_DISABLED
-        bool m_initialized;
-#endif
+#if !USE(LLINT_EMBEDDED_OPCODE_ID) || !ASSERT_DISABLED
+        HashMap<Opcode, OpcodeID>& m_opcodeIDTable; // Maps Opcode => OpcodeID for decompiling
+
+        static HashMap<Opcode, OpcodeID>& opcodeIDTable();
+#endif // !USE(LLINT_EMBEDDED_OPCODE_ID) || !ASSERT_DISABLED
+#endif // ENABLE(COMPUTED_GOTO_OPCODES)
     };
 
     JSValue eval(CallFrame*);

diff --git a/Source/JavaScriptCore/llint/LowLevelInterpreter.cpp b/Source/JavaScriptCore/llint/LowLevelInterpreter.cpp
index 59a250f..a0f09d4 100644 (file)
--- a/Source/JavaScriptCore/llint/LowLevelInterpreter.cpp
+++ b/Source/JavaScriptCore/llint/LowLevelInterpreter.cpp
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2012, 2014, 2016 Apple Inc. All rights reserved.
+ * Copyright (C) 2012-2017 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -497,7 +497,16 @@ JSValue CLoop::execute(OpcodeID entryOpcodeID, void* executableAddress, VM* vm,
 #define OFFLINE_ASM_BEGIN   asm (
 #define OFFLINE_ASM_END     );
 
-#define OFFLINE_ASM_OPCODE_LABEL(__opcode) OFFLINE_ASM_LOCAL_LABEL(llint_##__opcode)
+#if USE(LLINT_EMBEDDED_OPCODE_ID)
+#define EMBED_OPCODE_ID_IF_NEEDED(__opcode) ".int " __opcode##_value_string "\n"
+#else
+#define EMBED_OPCODE_ID_IF_NEEDED(__opcode)
+#endif
+
+#define OFFLINE_ASM_OPCODE_LABEL(__opcode) \
+    EMBED_OPCODE_ID_IF_NEEDED(__opcode) \
+    OFFLINE_ASM_LOCAL_LABEL(llint_##__opcode)
+
 #define OFFLINE_ASM_GLUE_LABEL(__opcode)   OFFLINE_ASM_LOCAL_LABEL(__opcode)
 
 #if CPU(ARM_THUMB2)

diff --git a/Source/JavaScriptCore/runtime/VM.cpp b/Source/JavaScriptCore/runtime/VM.cpp
index 1a29899..e50ba84 100644 (file)
--- a/Source/JavaScriptCore/runtime/VM.cpp
+++ b/Source/JavaScriptCore/runtime/VM.cpp
@@ -288,8 +288,6 @@ VM::VM(VMType vmType, HeapType heapType)
     ftlThunks = std::make_unique<FTL::Thunks>();
 #endif // ENABLE(FTL_JIT)
     
-    interpreter->initialize();
-    
 #if ENABLE(JIT)
     initializeHostCallReturnValue(); // This is needed to convince the linker not to drop host call return support.
 #endif

diff --git a/Source/WTF/ChangeLog b/Source/WTF/ChangeLog
index 8bf6391..ed30807 100644 (file)
--- a/Source/WTF/ChangeLog
+++ b/Source/WTF/ChangeLog
@@ -1,3 +1,14 @@
+2017-05-28  Mark Lam  <mark.lam@apple.com>
+
+        Implement a faster Interpreter::getOpcodeID().
+        https://bugs.webkit.org/show_bug.cgi?id=172669
+
+        Reviewed by Saam Barati.
+
+        Added the USE(LLINT_EMBEDDED_OPCODE_ID) configuration.
+
+        * wtf/Platform.h:
+
 2017-05-26  Brent Fulgham  <bfulgham@apple.com>
 
         [WK2] Address thread safety issues with ResourceLoadStatistics

diff --git a/Source/WTF/wtf/Platform.h b/Source/WTF/wtf/Platform.h
index ae1b9d8..928d3aa 100644 (file)
--- a/Source/WTF/wtf/Platform.h
+++ b/Source/WTF/wtf/Platform.h
@@ -916,6 +916,15 @@
 #define ENABLE_COMPUTED_GOTO_OPCODES 1
 #endif
 
+#if ENABLE(JIT) && !COMPILER(MSVC) && \
+    (CPU(X86) || CPU(X86_64) || CPU(ARM64) || CPU(ARM_THUMB2)) && OS(DARWIN)
+/* This feature works by embedding the OpcodeID in the 32 bit just before the generated LLint code
+   that executes each opcode. It cannot be supported by the CLoop since there's no way to embed the
+   OpcodeID word in the CLoop's switch statement cases. It is also currently not implemented for MSVC.
+*/
+#define USE_LLINT_EMBEDDED_OPCODE_ID 1
+#endif
+
 /* Regular Expression Tracing - Set to 1 to trace RegExp's in jsc.  Results dumped at exit */
 #define ENABLE_REGEXP_TRACING 0