dblaikie added subscribers: JDevlieghere, aprantl. dblaikie added a comment.
In D141451#4063582 <https://reviews.llvm.org/D141451#4063582>, @nickdesaulniers wrote: > In D141451#4063519 <https://reviews.llvm.org/D141451#4063519>, @dblaikie > wrote: > >> In D141451#4063504 <https://reviews.llvm.org/D141451#4063504>, >> @nickdesaulniers wrote: >> >>> In D141451#4063335 <https://reviews.llvm.org/D141451#4063335>, @dblaikie >>> wrote: >>> >>>> In D141451#4063151 <https://reviews.llvm.org/D141451#4063151>, >>>> @nickdesaulniers wrote: >>>> >>>>> In D141451#4045658 <https://reviews.llvm.org/D141451#4045658>, @efriedma >>>>> wrote: >>>>> >>>>>> clang has a "LocTrackingOnly" setting for debug info, which emits >>>>>> DILocation info into the IR, but emits a marker into the DICompileUnit >>>>>> to skip emitting the .debug_info in the backend. We currently use it >>>>>> for -Rpass. We don't do this by default, I think to save compile time. >>>>> >>>>> Specifically `emissionKind: NoDebug`, example: >>>>> >>>>> `!0 = distinct !DICompileUnit(language: DW_LANG_C11, file: !1, producer: >>>>> "clang version 16.0.0 (g...@github.com:llvm/llvm-project.git >>>>> 7b433e026498cf4176931b2407baece1d5060e16)", isOptimized: true, >>>>> runtimeVersion: 0, emissionKind: NoDebug, splitDebugInlining: false, >>>>> nameTableKind: None)` >>>>> >>>>> Though should the frontend be setting codegen options when parsing? Would >>>>> the idea be to try to re-set `OPT_debug_info_kind_EQ` when clang >>>>> codegen's IR for a function with such an attribute? >>>> >>>> Probably turn on `emissionKind: NoDebug` whenever the warning is enabled? >>> >>> So this warning is default enabled, IIRC. >> >> Then it might be a broader question about whether this extra info is >> acceptable to turn on by default, and if not, maybe an extra flag would be >> needed to say "give me extra info in this diagnostic", basically (or a >> separate warning flag that's off by default) - some perf metrics might help >> indicate whether the extra info is cheap enough. > > The implementation as it stands is zero cost in the sense that if > `__attribute__((warning("")))` or `__attribute__((error("")))` aren't used in > the source code, there is ZERO cost in IR. Additional metadata is only > created when inlining occurs (which might not happen if optimizations weren't > enabled) where callsites to such functions exist (unlikely), and if the call > site is optimized away, the metadata should be deleted (at least, I imagine > that's how metadata works in LLVM. I should actually verify that's the case, > then probably add such verification as a test to this patch, perhaps. > >> I'm still a bit confused/not following about the "Let me see if I can create >> DILocation without line or column values." - why do you want to create that? > > You're the one that asked <https://reviews.llvm.org/D141451#4045214> if if I > can reuse existing metadata nodes, specifically: > >>> It'd be nice not to invent a new way of tracking inlining separate from the >>> way debug info does this Right - I was thinking more, as above, about directly using the existing metadata generation (if it's too expensive to enable by default, then possibly under an off-by-default warning or other flag) that the inliner already knows how to read and write, rather than creating new/different metadata handling. Perhaps there's already logic for enabling this for remarks (CompilerInvocation.cpp:~2019). Though it's possibly a path forward to generate new DILocations in the absence of any - it's not the top of my list reach for here, but that's just my 2c. Perhaps other folks have other ideas. It's possible that this approach might be able to fill a gap we sometimes have in debug info, though - where a call site lacks a DILocation and that makes creating the inline debug info difficult/impossible at the moment (there's a whole long history of this issue, the assertions and verifier checks we've implemented to detect it, and the bugs (frontend missing opportunities to emit useful DILocations) it has found/lead to being fixed, though - so I'm somewhat hesitant to make us more permissive on constructing debug info inlining for call sites without DILocations). (@aprantl @JDevlieghere perhaps you folks have thoughts on this?) > So I could emit DILocation and DISubprogram metadata rather than the custom > metadata nodes as I'm doing in this diff. I just wouldn't be able to > synthesize line or column info for the callsites, since the frontend may not > have ever produced it in the first place. Such an approach would be emitting > `DILocation`+`DISubprogram` from the backend, not the frontend; the backend > doesn't have line+column info unless debugging was enabled in the first place > (which it probably wasn't). > > I don't want to pessimistically emit these always from the frontend with > precise line+column info. That would be so pessimistic for the common case > where you probably never encounter functions with such attributes in a given > TU. It's an awkward situation, to be sure. Looks like GCC does include the line number in its backtrace? So maybe that's a justification for us to do so as well. Again, might be worth knowing what the cost of the debug info metadata loc tracking mode is. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141451/new/ https://reviews.llvm.org/D141451 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
