[PATCH] D110018: [lld-macho] Speed up markLive()

Vincent Lee via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Sep 20 00:11:20 PDT 2021


thevinster added inline comments.


================
Comment at: lld/MachO/InputSection.h:101
 public:
+  ConcatInputSection(ConcatInputSection *isec)
+      : ConcatInputSection(isec->shared->segname, isec->shared->name,
----------------
Why do we need to define our own copy constructor? Isn't the compiler doing that for us already?


================
Comment at: lld/MachO/InputSection.h:118
   // ConcatInputSections are entirely live or dead, so the offset is irrelevant.
-  bool isLive(uint64_t off) const override { return live; }
-  void markLive(uint64_t off) override { live = true; }
+  bool isLive(uint64_t off) override { return live; }
+  void markLive(uint64_t off) override {
----------------
Shouldn't this method also have the mutex as well? 


================
Comment at: lld/MachO/MarkLive.cpp:48
       assert(!s->isCoalescedWeak());
+      const std::lock_guard<std::mutex> l(listMutex);
       worklist.push_back(s);
----------------
I think we might have a race condition here. If two different symbols pointing to the same `isec` and `off` happen to enter this function, we could end up writing the same value twice to `worklist`. It feels like the lock should be at the `enqueue` scope instead of the list here. 

Doing so, I believe we can also remove all the locks on the `isLive` and `markLive` methods. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110018



More information about the llvm-commits mailing list