Simplify DFG::DesiredIdentifiers and make it possible to turn a UniquedStringImpl...
authorfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 23 Jul 2015 04:58:34 +0000 (04:58 +0000)
committerfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 23 Jul 2015 04:58:34 +0000 (04:58 +0000)
https://bugs.webkit.org/show_bug.cgi?id=147218

Reviewed by Sam Weinig.

I want to be able to take a UniquedStringImpl* and turn it into an identifierNumber at
various points in my work on https://bugs.webkit.org/show_bug.cgi?id=146929. Currently,
most Nodes that deal with identifiers use identifierNumbers and you can only create an
identifierNumber in BytecodeGenerator. DFG::ByteCodeParser does sort of have the
ability to create new identifierNumbers when inlining - it takes the inlined code's
identifiers and either gives them new numbers or reuses numbers from the enclosing
code.

This patch takes that basic functionality and puts it in
DFG::DesiredIdentifiers::ensure(). Anyone can call this at any time to turn a
UniquedStringImpl* into an identifierNumber. This data structure is already used by
Plan to properly install any newly created identifier table entries into the CodeBlock.

* dfg/DFGByteCodeParser.cpp:
(JSC::DFG::ByteCodeParser::ByteCodeParser):
(JSC::DFG::ByteCodeParser::noticeArgumentsUse):
(JSC::DFG::ByteCodeParser::linkBlocks):
(JSC::DFG::ByteCodeParser::InlineStackEntry::InlineStackEntry):
(JSC::DFG::ByteCodeParser::buildOperandMapsIfNecessary): Deleted.
* dfg/DFGDesiredIdentifiers.cpp:
(JSC::DFG::DesiredIdentifiers::DesiredIdentifiers):
(JSC::DFG::DesiredIdentifiers::numberOfIdentifiers):
(JSC::DFG::DesiredIdentifiers::ensure):
(JSC::DFG::DesiredIdentifiers::at):
(JSC::DFG::DesiredIdentifiers::addLazily): Deleted.
* dfg/DFGDesiredIdentifiers.h:

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

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
Source/JavaScriptCore/dfg/DFGDesiredIdentifiers.cpp
Source/JavaScriptCore/dfg/DFGDesiredIdentifiers.h

index 6510d57..27ab119 100644 (file)
@@ -1,5 +1,39 @@
 2015-07-22  Filip Pizlo  <fpizlo@apple.com>
 
+        Simplify DFG::DesiredIdentifiers and make it possible to turn a UniquedStringImpl* into an identifierNumber at any time
+        https://bugs.webkit.org/show_bug.cgi?id=147218
+
+        Reviewed by Sam Weinig.
+        
+        I want to be able to take a UniquedStringImpl* and turn it into an identifierNumber at
+        various points in my work on https://bugs.webkit.org/show_bug.cgi?id=146929. Currently,
+        most Nodes that deal with identifiers use identifierNumbers and you can only create an
+        identifierNumber in BytecodeGenerator. DFG::ByteCodeParser does sort of have the
+        ability to create new identifierNumbers when inlining - it takes the inlined code's
+        identifiers and either gives them new numbers or reuses numbers from the enclosing
+        code.
+        
+        This patch takes that basic functionality and puts it in
+        DFG::DesiredIdentifiers::ensure(). Anyone can call this at any time to turn a
+        UniquedStringImpl* into an identifierNumber. This data structure is already used by
+        Plan to properly install any newly created identifier table entries into the CodeBlock.
+
+        * dfg/DFGByteCodeParser.cpp:
+        (JSC::DFG::ByteCodeParser::ByteCodeParser):
+        (JSC::DFG::ByteCodeParser::noticeArgumentsUse):
+        (JSC::DFG::ByteCodeParser::linkBlocks):
+        (JSC::DFG::ByteCodeParser::InlineStackEntry::InlineStackEntry):
+        (JSC::DFG::ByteCodeParser::buildOperandMapsIfNecessary): Deleted.
+        * dfg/DFGDesiredIdentifiers.cpp:
+        (JSC::DFG::DesiredIdentifiers::DesiredIdentifiers):
+        (JSC::DFG::DesiredIdentifiers::numberOfIdentifiers):
+        (JSC::DFG::DesiredIdentifiers::ensure):
+        (JSC::DFG::DesiredIdentifiers::at):
+        (JSC::DFG::DesiredIdentifiers::addLazily): Deleted.
+        * dfg/DFGDesiredIdentifiers.h:
+
+2015-07-22  Filip Pizlo  <fpizlo@apple.com>
+
         Simplify things like CompareEq(@x,@x)
         https://bugs.webkit.org/show_bug.cgi?id=145850
 
index 99a34b9..e5ed615 100644 (file)
@@ -145,7 +145,6 @@ public:
         , m_parameterSlots(0)
         , m_numPassedVarArgs(0)
         , m_inlineStackTop(0)
-        , m_haveBuiltOperandMaps(false)
         , m_currentInstruction(0)
         , m_hasDebuggerEnabled(graph.hasDebuggerEnabled())
     {
@@ -838,8 +837,6 @@ private:
             argument->mergeShouldNeverUnbox(true);
     }
     
-    void buildOperandMapsIfNecessary();
-    
     VM* m_vm;
     CodeBlock* m_codeBlock;
     CodeBlock* m_profiledBlock;
@@ -989,12 +986,6 @@ private:
     
     Vector<DelayedSetLocal, 2> m_setLocalQueue;
 
-    // Have we built operand maps? We initialize them lazily, and only when doing
-    // inlining.
-    bool m_haveBuiltOperandMaps;
-    // Mapping between identifier names and numbers.
-    BorrowedIdentifierMap m_identifierMap;
-    
     CodeBlock* m_dfgCodeBlock;
     CallLinkStatus::ContextMap m_callContextMap;
     StubInfoMap m_dfgStubInfos;
@@ -3961,17 +3952,6 @@ void ByteCodeParser::linkBlocks(Vector<UnlinkedBlock>& unlinkedBlocks, Vector<Ba
     }
 }
 
-void ByteCodeParser::buildOperandMapsIfNecessary()
-{
-    if (m_haveBuiltOperandMaps)
-        return;
-    
-    for (size_t i = 0; i < m_codeBlock->numberOfIdentifiers(); ++i)
-        m_identifierMap.add(m_codeBlock->identifier(i).impl(), i);
-    
-    m_haveBuiltOperandMaps = true;
-}
-
 ByteCodeParser::InlineStackEntry::InlineStackEntry(
     ByteCodeParser* byteCodeParser,
     CodeBlock* codeBlock,
@@ -4033,18 +4013,14 @@ ByteCodeParser::InlineStackEntry::InlineStackEntry(
         m_inlineCallFrame->arguments.resizeToFit(argumentCountIncludingThis); // Set the number of arguments including this, but don't configure the value recoveries, yet.
         m_inlineCallFrame->kind = kind;
         
-        byteCodeParser->buildOperandMapsIfNecessary();
-        
         m_identifierRemap.resize(codeBlock->numberOfIdentifiers());
         m_constantBufferRemap.resize(codeBlock->numberOfConstantBuffers());
         m_switchRemap.resize(codeBlock->numberOfSwitchJumpTables());
 
         for (size_t i = 0; i < codeBlock->numberOfIdentifiers(); ++i) {
             UniquedStringImpl* rep = codeBlock->identifier(i).impl();
-            BorrowedIdentifierMap::AddResult result = byteCodeParser->m_identifierMap.add(rep, byteCodeParser->m_graph.identifiers().numberOfIdentifiers());
-            if (result.isNewEntry)
-                byteCodeParser->m_graph.identifiers().addLazily(rep);
-            m_identifierRemap[i] = result.iterator->value;
+            unsigned index = byteCodeParser->m_graph.identifiers().ensure(rep);
+            m_identifierRemap[i] = index;
         }
         for (unsigned i = 0; i < codeBlock->numberOfConstantBuffers(); ++i) {
             // If we inline the same code block multiple times, we don't want to needlessly
index 74e2468..d3e8eac 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2013, 2014 Apple Inc. All rights reserved.
+ * Copyright (C) 2013-2015 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -35,11 +35,13 @@ namespace JSC { namespace DFG {
 
 DesiredIdentifiers::DesiredIdentifiers()
     : m_codeBlock(nullptr)
+    , m_didProcessIdentifiers(false)
 {
 }
 
 DesiredIdentifiers::DesiredIdentifiers(CodeBlock* codeBlock)
     : m_codeBlock(codeBlock)
+    , m_didProcessIdentifiers(false)
 {
 }
 
@@ -52,9 +54,23 @@ unsigned DesiredIdentifiers::numberOfIdentifiers()
     return m_codeBlock->numberOfIdentifiers() + m_addedIdentifiers.size();
 }
 
-void DesiredIdentifiers::addLazily(UniquedStringImpl* rep)
+unsigned DesiredIdentifiers::ensure(UniquedStringImpl* rep)
 {
-    m_addedIdentifiers.append(rep);
+    if (!m_didProcessIdentifiers) {
+        // Do this now instead of the constructor so that we don't pay the price on the main
+        // thread. Also, not all compilations need to call ensure().
+        for (unsigned index = m_codeBlock->numberOfIdentifiers(); index--;)
+            m_identifierNumberForName.add(m_codeBlock->identifier(index).impl(), index);
+        m_didProcessIdentifiers = true;
+    }
+
+    auto addResult = m_identifierNumberForName.add(rep, numberOfIdentifiers());
+    unsigned result = addResult.iterator->value;
+    if (addResult.isNewEntry) {
+        m_addedIdentifiers.append(rep);
+        ASSERT(at(result) == rep);
+    }
+    return result;
 }
 
 UniquedStringImpl* DesiredIdentifiers::at(unsigned index) const
index 3a2a348..d8587dc 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2013, 2014 Apple Inc. All rights reserved.
+ * Copyright (C) 2013-2015 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -45,7 +45,7 @@ public:
     ~DesiredIdentifiers();
     
     unsigned numberOfIdentifiers();
-    void addLazily(UniquedStringImpl*);
+    unsigned ensure(UniquedStringImpl*);
     
     UniquedStringImpl* at(unsigned index) const;
     
@@ -56,6 +56,8 @@ public:
 private:
     CodeBlock* m_codeBlock;
     Vector<UniquedStringImpl*> m_addedIdentifiers;
+    HashMap<UniquedStringImpl*, unsigned> m_identifierNumberForName;
+    bool m_didProcessIdentifiers;
 };
 
 } } // namespace JSC::DFG