[llvm] r296488 - Strip debug info when inlining into a nodebug function.

David Blaikie via llvm-commits llvm-commits at lists.llvm.org
Tue Mar 7 08:34:12 PST 2017


On Mon, Mar 6, 2017 at 7:27 PM Adrian Prantl <aprantl at apple.com> wrote:

>
> > On Mar 6, 2017, at 2:42 PM, David Blaikie <dblaikie at gmail.com> wrote:
> >
> > Might need to revert and discuss this further.
> >
> > When I implemented the infamous assert many years ago I hit cases like
> this and specifically crafted the assert to work around/allow situations
> like this because they came up in reality:
> >
> >   void f1();
> >   inline __attribute__((alwaysinline)) void f2() {
> >     f1();
> >   }
> >   inline __attribute__((alwaysinline)) __attribute__((nodebug)) void
> f3() {
> >     f2();
> >   }
> >   void f4() {
> >     f3();
> >   }
> >
> > Even in this case, there's value in keeping the deubg info in f3 after
> f2 is inlined into it so that when examining f4 there's inline info about
> f2. It ends up appearing as though f3 were never there - but f2 and f4 were
> totally there/meaningful/etc.
>
> I did not consider the possibility of the nodebug function being inlined
> into a debuggable function, and I'm surprised that this works — but after
> studying the code it looks like this might actually work.
>

Yep, didn't occur to me at first until I was testing all the cases for that
assert a few years ago - and verified that that situation works/seems
intentional/etc.


> I can't immediately revert this because this will now violate the
> verifier, (r296543), but I can instead relax the check there to allow for
> this situation and then revert.
>
> I'll look into this tomorrow.
>

Thanks!


>
> -- adrian
> >
> > Another reason/way to look at this is consistency: If the nodebug
> function were inlined first, then f2 was inlined - it would produce debug
> info for f2 and f4. It would be weird if the order of inlining produced
> substantially different debug info like this.
> >
> > If I'm understanding all this correctly...
> >
> > On Tue, Feb 28, 2017 at 9:09 AM Adrian Prantl via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
> > Author: adrian
> > Date: Tue Feb 28 10:58:13 2017
> > New Revision: 296488
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=296488&view=rev
> > Log:
> > Strip debug info when inlining into a nodebug function.
> >
> > The LLVM backend cannot produce any debug info for an llvm::Function
> > without a DISubprogram attachment. When inlining a debug-info-carrying
> > function into a nodebug function, there is therefore no reason to keep
> > any debug info intrinsic calls or debug locations on the instructions.
> >
> > This fixes a problem discovered in PR32042.
> >
> > rdar://problem/30679307
> >
> > Added:
> >     llvm/trunk/test/Transforms/Inline/nodebug.ll
> > Modified:
> >     llvm/trunk/lib/Transforms/Utils/InlineFunction.cpp
> >
>  llvm/trunk/test/Transforms/Inline/local-as-metadata-undominated-use.ll
> >
> > Modified: llvm/trunk/lib/Transforms/Utils/InlineFunction.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/InlineFunction.cpp?rev=296488&r1=296487&r2=296488&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/lib/Transforms/Utils/InlineFunction.cpp (original)
> > +++ llvm/trunk/lib/Transforms/Utils/InlineFunction.cpp Tue Feb 28
> 10:58:13 2017
> > @@ -1343,22 +1343,26 @@ static bool allocaWouldBeStaticInEntry(c
> >    return isa<Constant>(AI->getArraySize()) && !AI->isUsedWithInAlloca();
> >  }
> >
> > -/// Update inlined instructions' line numbers to
> > -/// to encode location where these instructions are inlined.
> > -static void fixupLineNumbers(Function *Fn, Function::iterator FI,
> > -                             Instruction *TheCall, bool
> CalleeHasDebugInfo) {
> > +/// Update inlined instructions' line numbers to to encode location
> where these
> > +/// instructions are inlined.  Also strip all debug intrinsics that
> were inlined
> > +/// into a nodebug function; there is no debug info the backend could
> produce
> > +/// for a function without a DISubprogram attachment.
> > +static void fixupDebugInfo(Function *Fn, Function::iterator FI,
> > +                           Instruction *TheCall, bool
> CalleeHasDebugInfo) {
> > +  bool CallerHasDebugInfo = Fn->getSubprogram();
> > +  bool StripDebugInfo = !CallerHasDebugInfo && CalleeHasDebugInfo;
> > +  SmallVector<DbgInfoIntrinsic *, 8> IntrinsicsToErase;
> >    const DebugLoc &TheCallDL = TheCall->getDebugLoc();
> > -  if (!TheCallDL)
> > -    return;
> >
> >    auto &Ctx = Fn->getContext();
> > -  DILocation *InlinedAtNode = TheCallDL;
> > +  DILocation *InlinedAtNode = nullptr;
> >
> >    // Create a unique call site, not to be confused with any other call
> from the
> >    // same location.
> > -  InlinedAtNode = DILocation::getDistinct(
> > -      Ctx, InlinedAtNode->getLine(), InlinedAtNode->getColumn(),
> > -      InlinedAtNode->getScope(), InlinedAtNode->getInlinedAt());
> > +  if (TheCallDL)
> > +    InlinedAtNode = DILocation::getDistinct(
> > +        Ctx, TheCallDL->getLine(), TheCallDL->getColumn(),
> > +        TheCallDL->getScope(), TheCallDL->getInlinedAt());
> >
> >    // Cache the inlined-at nodes as they're built so they are reused,
> without
> >    // this every instruction's inlined-at chain would become distinct
> from each
> > @@ -1368,6 +1372,17 @@ static void fixupLineNumbers(Function *F
> >    for (; FI != Fn->end(); ++FI) {
> >      for (BasicBlock::iterator BI = FI->begin(), BE = FI->end();
> >           BI != BE; ++BI) {
> > +      if (StripDebugInfo) {
> > +        // Inlining into a nodebug function.
> > +        if (auto *DI = dyn_cast<DbgInfoIntrinsic>(BI))
> > +          // Mark dead debug intrinsics for deletion.
> > +          IntrinsicsToErase.push_back(DI);
> > +        else
> > +          // Remove the dangling debug location.
> > +          BI->setDebugLoc(DebugLoc());
> > +        continue;
> > +      }
> > +
> >        if (DebugLoc DL = BI->getDebugLoc()) {
> >          BI->setDebugLoc(
> >              updateInlinedAtInfo(DL, InlinedAtNode, BI->getContext(),
> IANodes));
> > @@ -1390,6 +1405,9 @@ static void fixupLineNumbers(Function *F
> >        BI->setDebugLoc(TheCallDL);
> >      }
> >    }
> > +
> > +  for (auto *DI : IntrinsicsToErase)
> > +    DI->eraseFromParent();
> >  }
> >  /// Update the block frequencies of the caller after a callee has been
> inlined.
> >  ///
> > @@ -1710,8 +1728,8 @@ bool llvm::InlineFunction(CallSite CS, I
> >      // For 'nodebug' functions, the associated DISubprogram is always
> null.
> >      // Conservatively avoid propagating the callsite debug location to
> >      // instructions inlined from a function whose DISubprogram is not
> null.
> > -    fixupLineNumbers(Caller, FirstNewBlock, TheCall,
> > -                     CalledFunc->getSubprogram() != nullptr);
> > +    fixupDebugInfo(Caller, FirstNewBlock, TheCall,
> > +                   CalledFunc->getSubprogram() != nullptr);
> >
> >      // Clone existing noalias metadata if necessary.
> >      CloneAliasScopeMetadata(CS, VMap);
> >
> > Modified:
> llvm/trunk/test/Transforms/Inline/local-as-metadata-undominated-use.ll
> > URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/Inline/local-as-metadata-undominated-use.ll?rev=296488&r1=296487&r2=296488&view=diff
> >
> ==============================================================================
> > ---
> llvm/trunk/test/Transforms/Inline/local-as-metadata-undominated-use.ll
> (original)
> > +++
> llvm/trunk/test/Transforms/Inline/local-as-metadata-undominated-use.ll Tue
> Feb 28 10:58:13 2017
> > @@ -16,14 +16,14 @@ entry:
> >  }
> >
> >  ; CHECK-LABEL: define i32 @caller(
> > -define i32 @caller(i32 %i) {
> > +define i32 @caller(i32 %i) !dbg !3 {
> >  ; CHECK-NEXT: entry:
> >  entry:
> >  ; Although the inliner shouldn't crash, it can't be expected to get the
> >  ; "correct" SSA value since its assumptions have been violated.
> >  ; CHECK-NEXT:   tail call void @llvm.dbg.value(metadata
> ![[EMPTY:[0-9]+]],
> >  ; CHECK-NEXT:   %{{.*}} = add nsw
> > -  %call = tail call i32 @foo(i32 %i)
> > +  %call = tail call i32 @foo(i32 %i), !dbg !14
> >    ret i32 %call
> >  }
> >
> > @@ -34,9 +34,9 @@ declare void @llvm.dbg.value(metadata, i
> >
> >  !0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer:
> "clang version 3.9.0 (trunk 265634) (llvm/trunk 265637)", isOptimized:
> true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2)
> >  !1 = !DIFile(filename: "t.c", directory: "/path/to/tests")
> > -
> >  ; CHECK: ![[EMPTY]] = !{}
> >  !2 = !{}
> > +!3 = distinct !DISubprogram(name: "caller", scope: !1, file: !1, line:
> 3, type: !5, isLocal: false, isDefinition: true, scopeLine: 3, flags:
> DIFlagPrototyped, isOptimized: true, unit: !0)
> >  !4 = distinct !DISubprogram(name: "foo", scope: !1, file: !1, line: 2,
> type: !5, isLocal: false, isDefinition: true, scopeLine: 2, flags:
> DIFlagPrototyped, isOptimized: true, unit: !0)
> >  !5 = !DISubroutineType(types: !6)
> >  !6 = !{!7, !7}
> > @@ -47,3 +47,4 @@ declare void @llvm.dbg.value(metadata, i
> >  !11 = !DILocation(line: 2, column: 13, scope: !4)
> >  !12 = !DILocation(line: 2, column: 27, scope: !4)
> >  !13 = !DILocation(line: 2, column: 18, scope: !4)
> > +!14 = !DILocation(line: 3, scope: !3)
> >
> > Added: llvm/trunk/test/Transforms/Inline/nodebug.ll
> > URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/Inline/nodebug.ll?rev=296488&view=auto
> >
> ==============================================================================
> > --- llvm/trunk/test/Transforms/Inline/nodebug.ll (added)
> > +++ llvm/trunk/test/Transforms/Inline/nodebug.ll Tue Feb 28 10:58:13 2017
> > @@ -0,0 +1,36 @@
> > +; RUN: opt -inline -S -o - < %s | FileCheck %s
> > +; Check that debug info is stripped when inlining into a nodebug
> function.
> > +
> > +declare void @llvm.dbg.declare(metadata, metadata, metadata)
> > +declare void @llvm.dbg.value(metadata, i64, metadata, metadata)
> > +
> > +define void @foo() !dbg !2 {
> > +entry:
> > +  %a = alloca i32
> > +  call void @llvm.dbg.value(metadata i32 0, i64 0, metadata !3,
> metadata !DIExpression()), !dbg !6
> > +  store i32 0, i32* %a, !dbg !6
> > +  ret void, !dbg !6
> > +}
> > +
> > +; CHECK: define void @bar()
> > +define void @bar() {
> > +; CHECK-NEXT: entry
> > +entry:
> > +; CHECK-NEXT: alloca i32
> > +; CHECK-NOT: dbg
> > +; CHECK: ret void
> > +  call void @foo()
> > +  ret void
> > +}
> > +
> > +!llvm.dbg.cu = !{!0}
> > +!llvm.module.flags = !{!7, !8}
> > +
> > +!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer:
> "clang", emissionKind: FullDebug)
> > +!1 = !DIFile(filename: "x.c", directory: "/")
> > +!2 = distinct !DISubprogram(name: "foo", scope: !0, isDefinition: true,
> unit: !0)
> > +!3 = !DILocalVariable(name: "a", arg: 1, scope: !2, file: !1, line: 1,
> type: !5)
> > +!5 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
> > +!6 = !DILocation(line: 1, scope: !2)
> > +!7 = !{i32 2, !"Dwarf Version", i32 4}
> > +!8 = !{i32 1, !"Debug Info Version", i32 3}
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at lists.llvm.org
> > http://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/20170307/0cd43c5e/attachment.html>


More information about the llvm-commits mailing list