[PATCH] D132077: [llvm-reduce] Add pass that reduces DebugInfo metadata

[Matthias Braun via Phabricator via llvm-commits]

MatzeB added inline comments.


================
Comment at: llvm/tools/llvm-reduce/DeltaManager.cpp:70
     DELTA_PASS("metadata", reduceMetadataDeltaPass)                            \
+    DELTA_PASS("dimetadata", reduceDIMetadataDeltaPass)                        \
     DELTA_PASS("arguments", reduceArgumentsDeltaPass)                          \
----------------
ormris wrote:
> aeubanks wrote:
> > ormris wrote:
> > > aeubanks wrote:
> > > > I wonder if we should move the metadata passes later where we should already have removed as many potential metadata havers, e.g. after `instructions` or `ir-passes`
> > > Good point. I've run some tests locally and it seems like moving it after the instructions pass saves a few seconds. I'll change the patch.
> > > 
> > > | Position | Time (s) |
> > > | Current | 85.3 |
> > > | Last | 81.8 |
> > > | After instructions | 80.96 |
> > running `dimetadata` before `metadata` seems better since `dimetadata` can make `metadata` do less work, but not vice versa
> Fixed
> running `dimetadata` before `metadata` seems better since `dimetadata` can make `metadata` do less work, but not vice versa

Is that true? I think I typically see `metadata` drop references to `DICompileUnit` nodes and with it all the debug info disappears, reducing the work for `dimetadata`...


================
Comment at: llvm/tools/llvm-reduce/deltas/ReduceDIMetadata.cpp:35-37
+  for (const auto &NMD : MDs)
+    if (NMD && !Visited.count(NMD))
+      ToLook.push_back(NMD);
----------------
Isn't `Visited` empty in this loop and `!Visited()` will just always be true? So this loop in effect just copies from `MDs` to `ToLook` but filtering out `nullptr`. The next loop then seems to use `Visited` to ensure every node is only checked once even if it appears multiple times in the `ToLook` list.

Unless I miss something here, wouldn't it be a lot easier to:
1) Turn `MDs` into a `std::set<MDNode*>` or similar so duplicates get eliminated immediately?
2) Don't add `nullptr`s in the first place (or alternative just skip the 1 unique `nullptr` in the later loop)
3) No need for `ToLook` or `pop_back` anymore, just do `for (MDNode *MD : MDs)` for the loop in line 39?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132077