[PATCH] D76977: [lld-macho] Implement basic export trie

[Jez Ng via Phabricator via llvm-commits]

int3 marked 6 inline comments as done.
int3 added inline comments.


================
Comment at: lld/MachO/ExportTrie.cpp:40-42
+template <>
+struct llvm::ilist_alloc_traits<TrieEdge>
+    : llvm::ilist_noalloc_traits<TrieEdge> {};
----------------
may want to consider making this a BumpPtrList


================
Comment at: lld/MachO/ExportTrie.cpp:90
+        StringRef bNodeStr = edge.child->cumulativeString;
+        bNodeStr = bNodeStr.drop_back(edgeStr.size() - n).copy(bAlloc);
+        auto *bNode = make<TrieNode>(bNodeStr);
----------------
The original implementation used a unique BumpPtrAllocator instance for the export trie construction, which gets deallocated once trie serialization is complete. Here I am using the global `bAlloc` that doesn't deallocate till the end of the program. Not if it's worth using a local allocator instance to reduce memory consumption


================
Comment at: lld/MachO/ExportTrie.cpp:192-195
+  std::vector<TrieNode *> orderedNodes;
+  orderedNodes.reserve(allNodes.size());
+  for (const Symbol *sym : exported)
+    rootNode->orderNodes(*sym, orderedNodes);
----------------
This is basically a topological sort. It's unclear to me though why this approach (of finding every element starting from the root) was chosen vs doing a single preorder traversal of the trie...


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76977/new/

https://reviews.llvm.org/D76977