This is an archive of the discontinued LLVM Phabricator instance.

Use the NoDebug emission kind to identify compile units that no debug info should be created from.
ClosedPublic

Authored by aprantl on Apr 5 2016, 4:55 PM.

Details

Summary

As indicated in D18612, sample-based profiling and optimization remarks currently remove DICompileUnits from llvm.dbg.cu to suppress the emission of debug info from them. This is somewhat of a hack and only borderline legal IR.

This patch uses the recently introduced NoDebug emission kind in DICompileUnit to achieve the same result without breaking the Verifier. A nice side-effect of this change is that it is now possible to combine NoDebug and regular compile units under LTO.

For simplicity I combined the llvm and cfe patches into one review.

Diff Detail

Event Timeline

aprantl updated this revision to Diff 52745.Apr 5 2016, 4:55 PM
aprantl retitled this revision from to Use the NoDebug emission kind to identify compile units that no debug info should be created from..
aprantl updated this object.
aprantl added reviewers: dblaikie, echristo, dexonsmith.
aprantl set the repository for this revision to rL LLVM.
aprantl added subscribers: davide, llvm-commits, cfe-commits.
dexonsmith edited edge metadata.Apr 5 2016, 9:35 PM
dexonsmith added a subscriber: dexonsmith.

IR changes LGTM, if you add the missing verifier check (I'm surprised
this *ever* passed the verifier...)

Someone else will have to look at DwarfDebug.cpp.

echristo added inline comments.Apr 5 2016, 10:17 PM
lib/CodeGen/AsmPrinter/DwarfDebug.cpp
477–479

Instead of this pattern would it make more sense to have an iterator over the nodes that checks for !NoDebug?

1127–1128

Comment.

aprantl updated this revision to Diff 52842.Apr 6 2016, 1:07 PM
aprantl edited edge metadata.
aprantl removed rL LLVM as the repository for this revision.

This patch adds a DICompileUnit iterator to Module that skips over NoDebug CUs.

Oh. And I also discovered a completely bit-rotted pass called BreakpointPrinter that can impossible function because it looks at llvm.dbg.sp. Looks like it could be easily updated, though.

Duncan: There already exists a verifier check that ensures each DICompileUnit is listed in llvm.dbg.cu.
This didn't break existing users of sample-based profiling, because the Verifier cannot *find* the orphaned DICompileUnits. After the DISubprogram / DICompileUnit pointer reversal, however, this will be possible and the verification will fail.

Okay, weird. So the DICompileUnit wouldn't even get written out to
bitcode, IIUC. LGTM then.

dblaikie accepted this revision.Apr 8 2016, 1:51 PM
dblaikie edited edge metadata.

Looks good, thanks! Sorry for the delays/mthanks for the chat on IRC (summary: no worse than what we have today, more explicit, maybe we can find an alternative and/or more compact representation some other day)

This revision is now accepted and ready to land.Apr 8 2016, 1:51 PM
aprantl closed this revision.Apr 8 2016, 4:25 PM
aprantl marked an inline comment as done.

265860+265861