Page MenuHomePhabricator

[DebugIR] Revive the Debug IR pass. [Added llvm-commits]
Needs RevisionPublic

Authored by bollu on Dec 3 2017, 1:46 PM.

Details

Summary

Re-submitting this patch since I had not added llvm-commits previously. Old patch: D40207

This is a revival of the old DebugIR pass.

This pass strips away old debug info, and inserts new debug info that provides LLVM IR leveldebugging in a debugger.

I do not fully understand the debug infrastructure of LLVM, so reviews will be much appreciated!

Diff Detail

Event Timeline

bollu created this revision.Dec 3 2017, 1:46 PM
EwanCrawford added inline comments.
lib/Transforms/Instrumentation/DebugIR.cpp
669

rewrite this comment

EwanCrawford added inline comments.Dec 4 2017, 4:33 AM
lib/Transforms/Instrumentation/DebugIR.cpp
422

this comment is inappropriate

bollu updated this revision to Diff 125316.Dec 4 2017, 5:47 AM

Removed expletives from patch that arose due to frustration with debug metadata. Run clang-format on file.

bollu marked 2 inline comments as done.Dec 4 2017, 5:48 AM
vsk requested changes to this revision.Dec 4 2017, 12:54 PM
vsk added a subscriber: Ralith.

Thanks for resubmitting this @bollu. I discussed this idea with @Ralith, @rkruppe, @aprantl and others. I have a better notion now of how debugIR could be helpful for optimization and frontend authors.

A few high-level technical comments:

  • Should debugIR be a pass? The main use seems to be: there is an existing *.ll file on disk, and it needs to be debugged. Doesn't it make more sense to add a cl::opt to the LL reader to enable debugIR, so that the existing file can be debugged in-place?
  • Is there a reason to hand-roll DebugMetadataRemover when there's an existing StripDebugInfo utility?
This revision now requires changes to proceed.Dec 4 2017, 12:54 PM
Ralith added a comment.Dec 4 2017, 1:14 PM
In D40778#944100, @vsk wrote:
  • Should debugIR be a pass? The main use seems to be: there is an existing *.ll file on disk, and it needs to be debugged. Doesn't it make more sense to add a cl::opt to the LL reader to enable debugIR, so that the existing file can be debugged in-place?

Personally, I'm usually debugging JITd code, so there is no natural *.ll file on disk, and at no point is IR parsed from text. It's my understanding that debuggers do generally expect a source file to exist, but it would still be more natural for me to generate the source file on the side, rather than writing it out and then immediately using it to reconstruct the module.

vsk added a comment.Dec 4 2017, 1:25 PM
In D40778#944100, @vsk wrote:
  • Should debugIR be a pass? The main use seems to be: there is an existing *.ll file on disk, and it needs to be debugged. Doesn't it make more sense to add a cl::opt to the LL reader to enable debugIR, so that the existing file can be debugged in-place?

Personally, I'm usually debugging JITd code, so there is no natural *.ll file on disk, and at no point is IR parsed from text. It's my understanding that debuggers do generally expect a source file to exist, but it would still be more natural for me to generate the source file on the side, rather than writing it out and then immediately using it to reconstruct the module.

I don't think this use case would be supported unless -debugir would always be a part of the JIT pipeline. If debugging needs to be enabled by a special flag, it would be simple enough to implement it by invoking llvm-as on M.dump(). Implementing -debugir in the LL reader has other advantages, e.g it allows you to mark up the LL file with comments or edit it, then re-compile & debug again.

Ralith added a comment.Dec 4 2017, 1:45 PM
In D40778#944145, @vsk wrote:

Personally, I'm usually debugging JITd code, so there is no natural *.ll file on disk, and at no point is IR parsed from text. It's my understanding that debuggers do generally expect a source file to exist, but it would still be more natural for me to generate the source file on the side, rather than writing it out and then immediately using it to reconstruct the module.

I don't think this use case would be supported unless -debugir would always be a part of the JIT pipeline. If debugging needs to be enabled by a special flag, it would be simple enough to implement it by invoking llvm-as on M.dump(). Implementing -debugir in the LL reader has other advantages, e.g it allows you to mark up the LL file with comments or edit it, then re-compile & debug again.

To be clear, I'm not using the off-the-shelf JIT pipeline, but rather generating code and wrangling RuntimeDyld and friends myself. Hence, a pass would work well for me. While your proposed approach could be made to work for my case, it would be significantly less convenient, and would not let me take advantage of any opportunity to add comments to IR for debugging convenience (although I have never felt any need to do so).

vsk added a comment.Dec 4 2017, 2:01 PM
In D40778#944145, @vsk wrote:

Personally, I'm usually debugging JITd code, so there is no natural *.ll file on disk, and at no point is IR parsed from text. It's my understanding that debuggers do generally expect a source file to exist, but it would still be more natural for me to generate the source file on the side, rather than writing it out and then immediately using it to reconstruct the module.

I don't think this use case would be supported unless -debugir would always be a part of the JIT pipeline. If debugging needs to be enabled by a special flag, it would be simple enough to implement it by invoking llvm-as on M.dump(). Implementing -debugir in the LL reader has other advantages, e.g it allows you to mark up the LL file with comments or edit it, then re-compile & debug again.

To be clear, I'm not using the off-the-shelf JIT pipeline, but rather generating code and wrangling RuntimeDyld and friends myself. Hence, a pass would work well for me. While your proposed approach could be made to work for my case, it would be significantly less convenient, and would not let me take advantage of any opportunity to add comments to IR for debugging convenience (although I have never felt any need to do so).

Calling 'parseIR(..., EnableDebugIR=true)->print(OS)' before the running the program seems reasonable, though I admit I don't know how messy this might be in your use case. The alternative is to *not* support -debugIR on marked-up *.ll files, which seems worse.

bollu added a comment.Dec 5 2017, 1:50 PM

Regarding StripDebugInfo, I did not know it existed. I should refactor the code to use it, I assume? I'll check this out.

About being a pass or not - I don't understand what this sentence means, @vsk :

Implementing -debugir in the LL reader has other advantages, e.g it allows you to mark up the LL file with comments or edit it, then re-compile & debug again

Why would this not work if it were a pass as well? Because the comments would be stripped away in the LLVM module?

vsk added a comment.Dec 5 2017, 5:34 PM

Regarding StripDebugInfo, I did not know it existed. I should refactor the code to use it, I assume? I'll check this out.

Sounds good.

About being a pass or not - I don't understand what this sentence means, @vsk :

Implementing -debugir in the LL reader has other advantages, e.g it allows you to mark up the LL file with comments or edit it, then re-compile & debug again

Why would this not work if it were a pass as well? Because the comments would be stripped away in the LLVM module?

Right, unless the line to instruction mapping were somehow recorded into the module. The easiest way to do that is to attach debug info to the IR when it’s parsed.

davide added a subscriber: davide.Mar 20 2018, 5:41 PM

@bollu Are you still interested in working on this? I'm interested in getting this pass back into upstream LLVM. If you don't have time I'm willing to take over this if you like.

bollu added a comment.Mar 21 2018, 7:47 AM

I'm sorry. I don't have the bandwidth for this right now. Please do take it up, it'd be great to see this upstreamed! If we can get the base version upstreamed, I don't mind quickly iterating on smaller features / tests / things like that.

@bollu Are you still interested in working on this? I'm interested in getting this pass back into upstream LLVM. If you don't have time I'm willing to take over this if you like.

Are you working on this now? If so, what needs do be done and could you use some (inexperienced) help?

As mentioned, I don't have the bandwidth :( If you could fix the issues that were raised during the review, it would be awesome.

eush added a subscriber: eush.Oct 26 2018, 11:19 PM