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

[Djordje Todorovic via Phabricator via llvm-commits]

djtodoro added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/Debugify.cpp:355
+
+      auto InstrIt = DILocsBefore.find(Instr);
+      if (InstrIt == DILocsBefore.end()) {
----------------
vsk wrote:
> djtodoro wrote:
> > vsk wrote:
> > > Is `Instr` an `llvm::Instruction *`? I wonder whether it is sound to assume that pointer equality for `llvm::Instruction` implies actual equality, if we're comparing pointers before/after a pass runs. It's possible for a pass to delete and then create instructions: that could result in pointer reuse/recycling.
> > It is an `llvm::Instruction *`. I see... any idea how to address that?
> > (I was trying to avoid having to make a temp object of the instr...)
> I dug into this a bit and unfortunately it looks like a thorny issue. It looks like Instructions are (de)allocated via `User::operator new` and `User::operator delete`, and these essentially call into the system allocator. Relying on pointer equality to identify a unique instruction before/after a pass modifies the IR might necessitate e.g. changing the `delete` implementation to never free memory. That might not be practical -- it's certainly a big hammer.
> 
> It's possible that there's some alternative approach, but I'm not sure whether they'd be effective. One idea might be to use metadata to uniquely identify instructions (not !dbg metadata, of course :), and to teach the `dropUnknownNonDebugMetadata` utility to preserve that "inst ID" metadata. (I'd be curious to get some other opinions on whether that's workable.)
Thanks! I have also taken a look, and I agree that the gymnastic with the `new/delete` operators won't be practical choice.

Using some special kind of metadata can help us for this purpose. The "inst ID" metadata sounds good to me, since we are going to use it for this purpose **only**. We can call it `!debug-instr-number`, similar to the LiveDebugValues/InstrRef (D85741) work.

Let me try to implement a prototype for it, so we can see what are the pros && cons.


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

https://reviews.llvm.org/D82545