[PATCH] D82545: [Debugify] Make the debugify aware of the original (-g) Debug Info

[Adrian Prantl via Phabricator via llvm-commits]

aprantl added inline comments.


================
Comment at: llvm/docs/HowToUpdateDebugInfo.rst:221
 
 The ``debugify`` utility
 ^^^^^^^^^^^^^^^^^^^^^^^^
----------------
Can we call this
```
The ``debugify`` utility pass
```
?

I kind of expect a utility to be a standalone executable.


================
Comment at: llvm/docs/HowToUpdateDebugInfo.rst:230
+
+* `original` - collects and checks the preservation of real debug info
+  metadata
----------------
I think that this is a useful feature, but from the user's perspective the naming is confusing.

The name `debugify` implies that that the tool is taking input and then adds something debug-related to it. That clashes with preserving original information.

My suggestion is to either rename debugify to something more generic that encompasses both use-cases, or to share the implementation in one class, but give it two separate user-facing names. Maybe separating out the debugify action from the check action would also work (`--enable-debugify --enable-debuginfo-(preservation-)verification`).


================
Comment at: llvm/include/llvm/Transforms/Utils/Debugify.h:27
+using DebugFnMap = std::map<llvm::StringRef, const llvm::DISubprogram *>;
+using DebugInstMap = std::map<const llvm::Instruction *, bool>;
+
----------------
Why is this a std::map and not a DenseMap? Do we need the map's sorted-ness?


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

https://reviews.llvm.org/D82545