[PATCH] D90873: [DWARFLinker] Convert analyzeContextInfo to a work list (NFC)
Frederic Riss via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Nov 6 16:31:45 PST 2020
friss added a comment.
This is really hard to wrap one's head around, but after starring at it for some time the logic seems sound. There's some conditional code depending on whether we built with `-gmodules` or not. Can you make sure to test both clang builds? I left a couple other comments that you might want to consider. Otherwise this LGTM
================
Comment at: llvm/lib/DWARFLinker/DWARFLinker.cpp:256-261
+ ContextWorklistItemType Type;
+ DWARFDie Die;
+ CompileUnit::DIEInfo *OtherInfo = nullptr;
+ DeclContext *Context = nullptr;
+ unsigned ParentIdx = 0;
+ bool InImportedModule;
----------------
Not sure if it matters: the struct has a bunch of padding. You could used a fixed small underlying type for `Type` and put it at the end. It seems like `OtherInfo` and `Context` are mutually exclusive, they could are an anonymous union slot.
================
Comment at: llvm/lib/DWARFLinker/DWARFLinker.cpp:320
+ // LIFO work list.
+ SmallVector<ContextWorklistItem, 4> Worklist;
+ Worklist.emplace_back(DIE, CurrentDeclContext, ParentIdx, InImportedModule);
----------------
Is 4 a reasonable size? It seems like this would overflow most of the time, wouldn't it? Should this just be a `std::vector`?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D90873/new/
https://reviews.llvm.org/D90873
More information about the llvm-commits
mailing list