[llvm] a5c8ec4 - [CGDebugInfo] Emit subprograms for decls when AT_tail_call is understood

Vedant Kumar via llvm-commits llvm-commits at lists.llvm.org
Thu Nov 7 11:13:27 PST 2019



> On Nov 7, 2019, at 11:12 AM, Adrian Prantl <aprantl at apple.com> wrote:
> 
> 
> 
>> On Nov 7, 2019, at 10:59 AM, Vedant Kumar <vedant_kumar at apple.com <mailto:vedant_kumar at apple.com>> wrote:
>> 
>> + Clément & Eric (not sure why Eric was dropped from the thread, sorry about that)
>> 
>> The verifier failure is due to a combination of
>> 1) The declaration of @memcmp being assigned a DISubprogram, and
>> 2) The MergeICmps pass doesn't assign a debug location to the memcmps it generates
>> 
>> Hence, "inlinable function call" (memcmp) "in a function with debug info must have a !dbg location".
>> 
>> I think this could be addressed by either
>> 1) Not assigning DISubprograms to builtin functions or
> 
> Do we have any kind of useful non-redundant information in the DISubprogram for a builtin? Otherwise this seems fine to me.

I don't think so. Fwiw, prior to this patch we didn't emit the (declaration) DISubprograms for builtins at all.

vedant

> 
> -- adrian
> 
>> 2) Having MergeICmps assigning some location to the memcmps it generates (probably a merged / line 0 location, but maybe the location of the first icmp if that'd be more helpful).
>> 
>> Or both. (1) would shrink debug info size a little, and seems simpler to implement, so I'll start there.
>> 
>> Anyone have thoughts on (2), or other suggestions?
>> 
>> thanks
>> vedant
>> 
>>> On Nov 7, 2019, at 10:27 AM, Vedant Kumar via llvm-commits <llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>> wrote:
>>> 
>>> Thanks for reverting. I'll dig into this today.
>>> 
>>> vedant
>>> 
>>>> On Nov 7, 2019, at 9:09 AM, Eric Christopher via llvm-commits <llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>> wrote:
>>>> 
>>>> Thanks Hans!
>>>> 
>>>> On Thu, Nov 7, 2019, 1:32 AM Hans Wennborg <hans at chromium.org <mailto:hans at chromium.org>> wrote:
>>>> In Chromium it broke the build in a few configurations, so I've
>>>> reverted in 5b9a072c39c.
>>>> 
>>>> See https://bugs.chromium.org/p/chromium/issues/detail?id=1022296#c1 <https://bugs.chromium.org/p/chromium/issues/detail?id=1022296#c1>
>>>> for reproducer.
>>>> 
>>>> On Thu, Nov 7, 2019 at 3:05 AM Eric Christopher via llvm-commits
>>>> <llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>> wrote:
>>>> >
>>>> > As a heads up we're starting to see a problem around this commit with
>>>> > backtraces for inline functions with msan where the line number has
>>>> > gone away (replaced with 0).
>>>> >
>>>> > Dave is looking at it, but I thought I'd send a message as early as I could.
>>>> >
>>>> > -eric
>>>> >
>>>> > On Mon, Nov 4, 2019 at 3:14 PM Vedant Kumar via llvm-commits
>>>> > <llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>> wrote:
>>>> > >
>>>> > >
>>>> > > Author: Vedant Kumar
>>>> > > Date: 2019-11-04T15:14:24-08:00
>>>> > > New Revision: a5c8ec4baa2c00d8dbca36ffd236a40f9e0c07ed
>>>> > >
>>>> > > URL: https://github.com/llvm/llvm-project/commit/a5c8ec4baa2c00d8dbca36ffd236a40f9e0c07ed <https://github.com/llvm/llvm-project/commit/a5c8ec4baa2c00d8dbca36ffd236a40f9e0c07ed>
>>>> > > DIFF: https://github.com/llvm/llvm-project/commit/a5c8ec4baa2c00d8dbca36ffd236a40f9e0c07ed.diff <https://github.com/llvm/llvm-project/commit/a5c8ec4baa2c00d8dbca36ffd236a40f9e0c07ed.diff>
>>>> > >
>>>> > > LOG: [CGDebugInfo] Emit subprograms for decls when AT_tail_call is understood
>>>> > >
>>>> > > Currently, clang emits subprograms for declared functions when the
>>>> > > target debugger or DWARF standard is known to support entry values
>>>> > > (DW_OP_entry_value & the GNU equivalent).
>>>> > >
>>>> > > Treat DW_AT_tail_call the same way to allow debuggers to follow cross-TU
>>>> > > tail calls.
>>>> > >
>>>> > > Pre-patch debug session with a cross-TU tail call:
>>>> > >
>>>> > > ```
>>>> > >   * frame #0: 0x0000000100000fa4 main`target at b.c:4:3 [opt]
>>>> > >     frame #1: 0x0000000100000f99 main`main at a.c:8:10 [opt]
>>>> > > ```
>>>> > >
>>>> > > Post-patch (note that the tail-calling frame, "helper", is visible):
>>>> > >
>>>> > > ```
>>>> > >   * frame #0: 0x0000000100000fa4 main`target at b.c:4:3 [opt]
>>>> > >     frame #1: 0x0000000100000f80 main`helper [opt] [artificial]
>>>> > >     frame #2: 0x0000000100000f99 main`main at a.c:8:10 [opt]
>>>> > > ```
>>>> > >
>>>> > > rdar://46577651 <rdar://46577651>
>>>> > >
>>>> > > Differential Revision: https://reviews.llvm.org/D69743 <https://reviews.llvm.org/D69743>
>>>> > >
>>>> > > Added:
>>>> > >
>>>> > >
>>>> > > Modified:
>>>> > >     clang/lib/CodeGen/CGDebugInfo.cpp
>>>> > >     clang/test/CodeGen/debug-info-extern-call.c
>>>> > >     clang/test/CodeGenCXX/dbg-info-all-calls-described.cpp
>>>> > >     llvm/test/DebugInfo/X86/dwarf-callsite-related-attrs.ll
>>>> > >
>>>> > > Removed:
>>>> > >
>>>> > >
>>>> > >
>>>> > > ################################################################################
>>>> > > diff  --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp
>>>> > > index e0bb3fda7acf..c0c8fd3c8f16 100644
>>>> > > --- a/clang/lib/CodeGen/CGDebugInfo.cpp
>>>> > > +++ b/clang/lib/CodeGen/CGDebugInfo.cpp
>>>> > > @@ -3748,9 +3748,7 @@ void CGDebugInfo::EmitFunctionDecl(GlobalDecl GD, SourceLocation Loc,
>>>> > >  void CGDebugInfo::EmitFuncDeclForCallSite(llvm::CallBase *CallOrInvoke,
>>>> > >                                            QualType CalleeType,
>>>> > >                                            const FunctionDecl *CalleeDecl) {
>>>> > > -  auto &CGOpts = CGM.getCodeGenOpts();
>>>> > > -  if (!CGOpts.EnableDebugEntryValues || !CGM.getLangOpts().Optimize ||
>>>> > > -      !CallOrInvoke)
>>>> > > +  if (!CallOrInvoke || getCallSiteRelatedAttrs() == llvm::DINode::FlagZero)
>>>> > >      return;
>>>> > >
>>>> > >    auto *Func = CallOrInvoke->getCalledFunction();
>>>> > > @@ -4824,10 +4822,10 @@ llvm::DINode::DIFlags CGDebugInfo::getCallSiteRelatedAttrs() const {
>>>> > >    bool SupportsDWARFv4Ext =
>>>> > >        CGM.getCodeGenOpts().DwarfVersion == 4 &&
>>>> > >        (CGM.getCodeGenOpts().getDebuggerTuning() == llvm::DebuggerKind::LLDB ||
>>>> > > -       (CGM.getCodeGenOpts().EnableDebugEntryValues &&
>>>> > > -       CGM.getCodeGenOpts().getDebuggerTuning() == llvm::DebuggerKind::GDB));
>>>> > > +       CGM.getCodeGenOpts().getDebuggerTuning() == llvm::DebuggerKind::GDB);
>>>> > >
>>>> > > -  if (!SupportsDWARFv4Ext && CGM.getCodeGenOpts().DwarfVersion < 5)
>>>> > > +  if (!SupportsDWARFv4Ext && CGM.getCodeGenOpts().DwarfVersion < 5 &&
>>>> > > +      !CGM.getCodeGenOpts().EnableDebugEntryValues)
>>>> > >      return llvm::DINode::FlagZero;
>>>> > >
>>>> > >    return llvm::DINode::FlagAllCallsDescribed;
>>>> > >
>>>> > > diff  --git a/clang/test/CodeGen/debug-info-extern-call.c b/clang/test/CodeGen/debug-info-extern-call.c
>>>> > > index e35669b78f93..7f58fe59a635 100644
>>>> > > --- a/clang/test/CodeGen/debug-info-extern-call.c
>>>> > > +++ b/clang/test/CodeGen/debug-info-extern-call.c
>>>> > > @@ -1,8 +1,21 @@
>>>> > > -// RUN: %clang -Xclang -femit-debug-entry-values -g -O2 -target x86_64-none-linux-gnu -S -emit-llvm %s -o - | FileCheck %s -check-prefix=CHECK-EXT
>>>> > > -// CHECK-EXT: !DISubprogram(name: "fn1"
>>>> > > +// When entry values are emitted, expect a subprogram for extern decls so that
>>>> > > +// the dwarf generator can describe call site parameters at extern call sites.
>>>> > > +//
>>>> > > +// RUN: %clang -Xclang -femit-debug-entry-values -g -O2 -target x86_64-none-linux-gnu -S -emit-llvm %s -o - | FileCheck %s -check-prefix=ENTRY-VAL
>>>> > > +// ENTRY-VAL: !DISubprogram(name: "fn1"
>>>> > >
>>>> > > -// RUN: %clang -g -O2 -target x86_64-none-linux-gnu -S -emit-llvm %s -o - | FileCheck %s
>>>> > > -// CHECK-NOT: !DISubprogram(name: "fn1"
>>>> > > +// Similarly, when the debugger tuning is gdb, expect a subprogram for extern
>>>> > > +// decls so that the dwarf generator can describe information needed for tail
>>>> > > +// call frame reconstrution.
>>>> > > +//
>>>> > > +// RUN: %clang -g -O2 -target x86_64-none-linux-gnu -ggdb -S -emit-llvm %s -o - | FileCheck %s -check-prefix=GDB
>>>> > > +// GDB: !DISubprogram(name: "fn1"
>>>> > > +//
>>>> > > +// Do not emit a subprogram for extern decls when entry values are disabled and
>>>> > > +// the tuning is not set to gdb.
>>>> > > +//
>>>> > > +// RUN: %clang -g -O2 -target x86_64-none-linux-gnu -gsce -S -emit-llvm %s -o - | FileCheck %s -check-prefix=SCE
>>>> > > +// SCE-NOT: !DISubprogram(name: "fn1"
>>>> > >
>>>> > >  extern int fn1(int a, int b);
>>>> > >
>>>> > >
>>>> > > diff  --git a/clang/test/CodeGenCXX/dbg-info-all-calls-described.cpp b/clang/test/CodeGenCXX/dbg-info-all-calls-described.cpp
>>>> > > index 1cb2b6c609f3..667c2469b55e 100644
>>>> > > --- a/clang/test/CodeGenCXX/dbg-info-all-calls-described.cpp
>>>> > > +++ b/clang/test/CodeGenCXX/dbg-info-all-calls-described.cpp
>>>> > > @@ -56,6 +56,7 @@
>>>> > >
>>>> > >  // NO-ATTR-NOT: FlagAllCallsDescribed
>>>> > >
>>>> > > +// HAS-ATTR-DAG: DISubprogram(name: "declaration1", {{.*}}, flags: DIFlagPrototyped
>>>> > >  // HAS-ATTR-DAG: DISubprogram(name: "declaration2", {{.*}}, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition
>>>> > >  // HAS-ATTR-DAG: DISubprogram(name: "struct1", {{.*}}, flags: DIFlagPrototyped, spFlags: DISPFlagOptimized)
>>>> > >  // HAS-ATTR-DAG: DISubprogram(name: "struct1", {{.*}}, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition
>>>> > >
>>>> > > diff  --git a/llvm/test/DebugInfo/X86/dwarf-callsite-related-attrs.ll b/llvm/test/DebugInfo/X86/dwarf-callsite-related-attrs.ll
>>>> > > index c37ce1eb7fbe..33e06faba57b 100644
>>>> > > --- a/llvm/test/DebugInfo/X86/dwarf-callsite-related-attrs.ll
>>>> > > +++ b/llvm/test/DebugInfo/X86/dwarf-callsite-related-attrs.ll
>>>> > > @@ -25,6 +25,14 @@
>>>> > >
>>>> > >  @sink = global i32 0, align 4, !dbg !0
>>>> > >
>>>> > > +define void @__has_no_subprogram() {
>>>> > > +entry:
>>>> > > +  %0 = load volatile i32, i32* @sink, align 4
>>>> > > +  %inc = add nsw i32 %0, 1
>>>> > > +  store volatile i32 %inc, i32* @sink, align 4
>>>> > > +  ret void
>>>> > > +}
>>>> > > +
>>>> > >  ; ASM: DW_TAG_subprogram
>>>> > >  ; ASM:   DW_AT_call_all_calls
>>>> > >  ; OBJ: [[bat_sp:.*]]: DW_TAG_subprogram
>>>> > > @@ -70,6 +78,7 @@ entry:
>>>> > >  ; OBJ:     DW_AT_call_tail_call
>>>> > >  define void @_Z3foov() !dbg !25 {
>>>> > >  entry:
>>>> > > +  tail call void @__has_no_subprogram()
>>>> > >    tail call void @_Z3barv(), !dbg !26
>>>> > >    tail call void @_Z3batv(), !dbg !27
>>>> > >    tail call void @_Z3barv(), !dbg !26
>>>> > >
>>>> > >
>>>> > >
>>>> > > _______________________________________________
>>>> > > llvm-commits mailing list
>>>> > > llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>
>>>> > > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits <https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits>
>>>> > _______________________________________________
>>>> > llvm-commits mailing list
>>>> > llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>
>>>> > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits <https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits>
>>>> _______________________________________________
>>>> llvm-commits mailing list
>>>> llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>
>>>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits <https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits>
>>> 
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>
>>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits <https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits>
>> 
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20191107/e2dd29a1/attachment.html>


More information about the llvm-commits mailing list