[PATCH] D141451: [clang] report inlining decisions with -Wattribute-{warning|error}

[Nick Desaulniers via Phabricator via llvm-commits]

nickdesaulniers added a comment.

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 (git at 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

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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141451