[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