[PATCH] D103220: Support stripping indirectly referenced DILocations from !llvm.loop metadata in stripDebugInfo()

Shafik Yaghmour via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu May 27 09:31:07 PDT 2021


shafik added inline comments.


================
Comment at: llvm/lib/IR/DebugInfo.cpp:392
+    return true;
+  if (!Visited.insert(N).second)
+    return false;
----------------
This is why I dislike `std::pair` what does `second` mean? A comment like `if it was already inserted exit now` would be helpful.


================
Comment at: llvm/lib/IR/DebugInfo.cpp:411
   // if there is no debug location, we do not have to rewrite this MDNode.
-  if (std::none_of(N->op_begin() + 1, N->op_end(), [](const MDOperand &Op) {
-        return isa<DILocation>(Op.get());
+  if (!std::count_if(N->op_begin() + 1, N->op_end(), [&](const MDOperand &Op) {
+        return isa<DILocation>(Op.get()) ||
----------------
I see in the other files the lambda explicitly list their captures instead of just using `&` to capture all by reference, I think that is a better style to be explicit.


================
Comment at: llvm/lib/Transforms/Utils/InlineFunction.cpp:1523
+      auto updateLoopInfoLoc = [&Ctx, &InlinedAtNode,
+                                &IANodes](Metadata *MD) -> Metadata * {
+        if (auto *Loc = dyn_cast_or_null<DILocation>(MD))
----------------
I see that previously `Loc` was `const` can `MD` also be `const`?


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

https://reviews.llvm.org/D103220



More information about the llvm-commits mailing list