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

[Nick Desaulniers via Phabricator via llvm-commits]

nickdesaulniers added a comment.

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. I guess I need to check for `-Wno-attribute-warning`.  If `-Wno-attribute-warning` is //not// set and `-g` (or friends) is not set, then I should set `emissionKind` to `NoDebug` (I think).

>> ---
>>
>> In D141451#4045214 <https://reviews.llvm.org/D141451#4045214>, @dblaikie wrote:
>>
>>> It'd be nice not to invent a new way of tracking inlining separate from the way debug info does this - duplicate systems with separate opportunities for bugs, etc. Any chance we can reuse the debug info inlining descriptions for this?
>>
>> So it looks like we have:
>>
>> `!28 = !DILocation(line: 14, column: 3, scope: !8, inlinedAt: !29)`
>>
>> Let me see if I can create DILocation without line or column values.
>
> Not sure I follow - why would you want to drop line/column info? Isn't that relevant to the inlining stack - you might have multiple calls in the same function & it'd be good to know which one the diagnostic is referring to.
>
>>   The DISubprogram and DILocation should form a similar chain, even if significantly more complicated to "unwind."

Not dropping it, more like without `-g` (or friends) it never existed in the first place.  If you look at my change to `llvm/lib/Transforms/Utils/InlineFunction.cpp` (in this patch), I'm basically synthesizing metadata during inlining.  If we don't have Debug Info related metadata because the program wasn't compiled with `-g` (or w/e), then the DILocation for the callsites was never produced by the frontend in the first place.  This patch doesn't intentionally drop anything, it's more like the anything might never have existed in the first place.

So this warning could "be improved" if you recompiled with `-g`; I don't think there's really precedent for that and expect it's perhaps controversial.  Hence my custom metadate nodes rather than the existing DILocation.



================
Comment at: llvm/lib/Transforms/Utils/InlineFunction.cpp:2467-2468
+
+        const Function *Callee = CI->getCalledFunction();
+        if (Callee && (Callee->hasFnAttribute("dontcall-error") ||
+                       Callee->hasFnAttribute("dontcall-warn"))) {
----------------
nickdesaulniers wrote:
> nickdesaulniers wrote:
> > arsenm wrote:
> > > Misses constexpr casts and aliases
> > The base feature doesn't work with aliases (or ConstantExpr), in GCC or Clang.  I should perhaps fix that first...
> Perhaps I should use Call.getCalledOperand()->stripPointerCasts() for constantexpr case.
I've added support for ConstantExpr to the base feature in D142058.  I should still fix this in this PR and add a test case, so not yet marking this thread as done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141451