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

Matthias Braun via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 6 16:34:57 PDT 2022


MatzeB added inline comments.


================
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);
----------------
MatzeB wrote:
> 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?
Oh I guess this a worklist like algorithm to traverse MDNodes recursively. So you can ignore most of that my comment. Though the `Visited` use in line 36 still seems unnecessary.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132077



More information about the llvm-commits mailing list