[PATCH] D115623: [Debugify] Use DebugifyLevel in Debugify original mode

[Orlando Cazalet-Hyams via Phabricator via llvm-commits]

Orlando added a comment.

Hi @djtodoro. The patch SGTM but I do have one concern/question regarding the test (inline comment).



================
Comment at: llvm/lib/Transforms/Utils/Debugify.cpp:323
 
-        // Collect dbg.values and dbg.declares.
-        if (auto *DVI = dyn_cast<DbgVariableIntrinsic>(&I)) {
-          if (!SP)
-            continue;
-          // Skip inlined variables.
-          if (I.getDebugLoc().getInlinedAt())
-            continue;
-          // Skip undef values.
-          if (DVI->isUndef())
+        // Cllect dbg.values and dbg.declare.
+        if (DebugifyLevel > Level::Locations) {
----------------
nit: missing o in Collect and s in dbg.declares.


================
Comment at: llvm/test/Transforms/Util/Debugify/loc-only-original-mode.ll:6
+;; Ensure that we check for DILocation potential issues only.
+; CHECK-NOT: drops dbg.value()/dbg.declare()
+
----------------
This test feels prone to rot because it will pass even if the feature is not working as intended if A) the pass stops dropping intrinsics or B) the warning message is changed. I'm not sure what we could do instead though - do you have any ideas?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115623