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

[Arthur Eubanks via Phabricator via llvm-commits]

aeubanks added a comment.

some nits, but thanks for going this direction, looks much better



================
Comment at: llvm/test/tools/llvm-reduce/remove-dimetadata.ll:22
+!7 = !{!13}
+; CHECK: !{}
+!8 = distinct !DICompileUnit(language: DW_LANG_C, file: !1, producer: "clang", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !9)
----------------
can we be more explicit about this CHECK and the one below?
e.g.
```
; CHECK: DISubroutineType(types: [[CHILD:![0-9]+]])
; CHECK: [[CHILD]] = !{}
```
(I might have the exact syntax wrong)


================
Comment at: llvm/tools/llvm-reduce/deltas/ReduceDIMetadata.cpp:36
+  for (const auto &NMD : MDs)
+    if (NMD && !Visited.count(NMD))
+      ToLook.push_back(NMD);
----------------
when would this be false/null?


================
Comment at: llvm/tools/llvm-reduce/deltas/ReduceDIMetadata.cpp:45
+    if (DINode *DIM = dyn_cast_or_null<DINode>(MD)) {
+        // Scan operands and record attached tuples
+        for (size_t I = 0; I < DIM->getNumOperands(); ++I)
----------------
some indentation weirdness, `git clang-format`?


================
Comment at: llvm/tools/llvm-reduce/deltas/ReduceDIMetadata.cpp:48
+          if (MDTuple *MDT = dyn_cast_or_null<MDTuple>(DIM->getOperand(I)))
+            if (MDT && !Visited.count(MDT) && MDT->getNumOperands())
+              Tuples.insert({DIM, I, MDT});
----------------
the outer if just checked that this isn't false/null


================
Comment at: llvm/tools/llvm-reduce/deltas/ReduceDIMetadata.cpp:55
+      if (MDNode *OMD = dyn_cast_or_null<MDNode>(Op))
+        if (OMD && !Visited.count(OMD))
+          ToLook.push_back(OMD);
----------------
ditto


================
Comment at: llvm/tools/llvm-reduce/deltas/ReduceDIMetadata.cpp:62
+  for (auto &T : Tuples) {
+    auto [MO, J, MT] = T;
+    // Remove the operands of the tuple that are not in the desired chunks.
----------------
could you give these more descriptive names?


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

https://reviews.llvm.org/D132077