[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