[llvm] r289468 - Recommit r288212: Emit 'no line' information for interesting 'orphan' instructions.

Robinson, Paul via llvm-commits llvm-commits at lists.llvm.org
Fri Dec 16 14:29:39 PST 2016


Sure.  I see that the very first debug location is explicitly setting line 0.  Is that really what you want?
--paulr

From: aprantl at apple.com [mailto:aprantl at apple.com]
Sent: Friday, December 16, 2016 2:00 PM
To: Robinson, Paul
Cc: llvm-commits; Saleem Abdulrasool
Subject: Re: [llvm] r289468 - Recommit r288212: Emit 'no line' information for interesting 'orphan' instructions.

Hi Paul,

we were investigating a test suite regression in the Swift compiler (just in case you are curious: https://bugs.swift.org/browse/SR-3421) that I could bisect down to this commit.

When compiling the attached IR (which I reduced manually from the full testcase) llc now runs into a situation where it never sets the prologue_end flag for the function main().

$llc t.ll -o - | grep '\.loc'
        .loc    1 0 0
        .loc    1 14 13

I assume that this is something we never want to happen. Would you mind taking a look?

-- adrian


> On Dec 12, 2016, at 12:49 PM, Paul Robinson via llvm-commits <llvm-commits at lists.llvm.org<mailto:llvm-commits at lists.llvm.org>> wrote:
>
> Author: probinson
> Date: Mon Dec 12 14:49:11 2016
> New Revision: 289468
>
> URL: http://llvm.org/viewvc/llvm-project?rev=289468&view=rev
> Log:
> Recommit r288212: Emit 'no line' information for interesting 'orphan' instructions.
>
> DWARF specifies that "line 0" really means "no appropriate source
> location" in the line table.  By default, use this for branch targets
> and some other cases that have no specified source location, to
> prevent inheriting unfortunate line numbers from physically preceding
> instructions (which might be from completely unrelated source).
>
> Updated patch allows enabling or suppressing this behavior for all
> unspecified source locations.
>
> Differential Revision: http://reviews.llvm.org/D24180
>
> Added:
>    llvm/trunk/test/DebugInfo/X86/dwarf-no-source-loc.ll
> Modified:
>    llvm/trunk/lib/CodeGen/AsmPrinter/DebugHandlerBase.cpp
>    llvm/trunk/lib/CodeGen/AsmPrinter/DebugHandlerBase.h
>    llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
>    llvm/trunk/test/CodeGen/X86/stack-protector.ll
>    llvm/trunk/test/CodeGen/X86/unknown-location.ll
>    llvm/trunk/test/DebugInfo/AArch64/line-header.ll
>    llvm/trunk/test/DebugInfo/MIR/X86/no-cfi-loc.mir
>    llvm/trunk/test/DebugInfo/X86/tail-merge.ll
>
> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DebugHandlerBase.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DebugHandlerBase.cpp?rev=289468&r1=289467&r2=289468&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/AsmPrinter/DebugHandlerBase.cpp (original)
> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DebugHandlerBase.cpp Mon Dec 12 14:49:11 2016
> @@ -200,8 +200,10 @@ void DebugHandlerBase::endInstruction()
>   assert(CurMI != nullptr);
>   // Don't create a new label after DBG_VALUE instructions.
>   // They don't generate code.
> -  if (!CurMI->isDebugValue())
> +  if (!CurMI->isDebugValue()) {
>     PrevLabel = nullptr;
> +    PrevInstBB = CurMI->getParent();
> +  }
>
>   DenseMap<const MachineInstr *, MCSymbol *>::iterator I =
>       LabelsAfterInsn.find(CurMI);
>
> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DebugHandlerBase.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DebugHandlerBase.h?rev=289468&r1=289467&r2=289468&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/AsmPrinter/DebugHandlerBase.h (original)
> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DebugHandlerBase.h Mon Dec 12 14:49:11 2016
> @@ -38,10 +38,12 @@ protected:
>   MachineModuleInfo *MMI;
>
>   /// Previous instruction's location information. This is used to
> -  /// determine label location to indicate scope boundries in dwarf
> -  /// debug info.
> +  /// determine label location to indicate scope boundaries in debug info.
> +  /// We track the previous instruction's source location (if not line 0),
> +  /// whether it was a label, and its parent BB.
>   DebugLoc PrevInstLoc;
>   MCSymbol *PrevLabel = nullptr;
> +  const MachineBasicBlock *PrevInstBB = nullptr;
>
>   /// This location indicates end of function prologue and beginning of
>   /// function body.
>
> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp?rev=289468&r1=289467&r2=289468&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp (original)
> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp Mon Dec 12 14:49:11 2016
> @@ -62,11 +62,6 @@ static cl::opt<bool>
> DisableDebugInfoPrinting("disable-debug-info-print", cl::Hidden,
>                          cl::desc("Disable debug info printing"));
>
> -static cl::opt<bool> UnknownLocations(
> -    "use-unknown-locations", cl::Hidden,
> -    cl::desc("Make an absence of debug location information explicit."),
> -    cl::init(false));
> -
> static cl::opt<bool>
> GenerateGnuPubSections("generate-gnu-dwarf-pub-sections", cl::Hidden,
>                        cl::desc("Generate GNU-style pubnames and pubtypes"),
> @@ -81,6 +76,13 @@ namespace {
> enum DefaultOnOff { Default, Enable, Disable };
> }
>
> +static cl::opt<DefaultOnOff> UnknownLocations(
> +    "use-unknown-locations", cl::Hidden,
> +    cl::desc("Make an absence of debug location information explicit."),
> +    cl::values(clEnumVal(Default, "At top of block or after label"),
> +               clEnumVal(Enable, "In all cases"), clEnumVal(Disable, "Never")),
> +    cl::init(Default));
> +
> static cl::opt<DefaultOnOff>
> DwarfAccelTables("dwarf-accel-tables", cl::Hidden,
>                  cl::desc("Output prototype dwarf accelerator tables."),
> @@ -1010,31 +1012,73 @@ void DwarfDebug::beginInstruction(const
>   if (MI->isDebugValue() || MI->isCFIInstruction())
>     return;
>   const DebugLoc &DL = MI->getDebugLoc();
> -  if (DL == PrevInstLoc)
> +  // When we emit a line-0 record, we don't update PrevInstLoc; so look at
> +  // the last line number actually emitted, to see if it was line 0.
> +  unsigned LastAsmLine =
> +      Asm->OutStreamer->getContext().getCurrentDwarfLoc().getLine();
> +
> +  if (DL == PrevInstLoc) {
> +    // If we have an ongoing unspecified location, nothing to do here.
> +    if (!DL)
> +      return;
> +    // We have an explicit location, same as the previous location.
> +    // But we might be coming back to it after a line 0 record.
> +    if (LastAsmLine == 0 && DL.getLine() != 0) {
> +      // Reinstate the source location but not marked as a statement.
> +      const MDNode *Scope = DL.getScope();
> +      recordSourceLine(DL.getLine(), DL.getCol(), Scope, /*Flags=*/0);
> +    }
>     return;
> +  }
>
>   if (!DL) {
>     // We have an unspecified location, which might want to be line 0.
> -    if (UnknownLocations) {
> -      PrevInstLoc = DL;
> -      recordSourceLine(0, 0, nullptr, 0);
> +    // If we have already emitted a line-0 record, don't repeat it.
> +    if (LastAsmLine == 0)
> +      return;
> +    // If user said Don't Do That, don't do that.
> +    if (UnknownLocations == Disable)
> +      return;
> +    // See if we have a reason to emit a line-0 record now.
> +    // Reasons to emit a line-0 record include:
> +    // - User asked for it (UnknownLocations).
> +    // - Instruction has a label, so it's referenced from somewhere else,
> +    //   possibly debug information; we want it to have a source location.
> +    // - Instruction is at the top of a block; we don't want to inherit the
> +    //   location from the physically previous (maybe unrelated) block.
> +    if (UnknownLocations == Enable || PrevLabel ||
> +        (PrevInstBB && PrevInstBB != MI->getParent())) {
> +      // Preserve the file number, if we can, to save space in the line table.
> +      // Do not update PrevInstLoc, it remembers the last non-0 line.
> +      // FIXME: Also preserve the column number, to save more space?
> +      const MDNode *Scope = PrevInstLoc ? PrevInstLoc.getScope() : nullptr;
> +      recordSourceLine(0, 0, Scope, 0);
>     }
>     return;
>   }
>
> -  // We have a new, explicit location.
> +  // We have an explicit location, different from the previous location.
> +  // Don't repeat a line-0 record, but otherwise emit the new location.
> +  // (The new location might be an explicit line 0, which we do emit.)
> +  if (DL.getLine() == 0 && LastAsmLine == 0)
> +    return;
>   unsigned Flags = 0;
> -  PrevInstLoc = DL;
>   if (DL == PrologEndLoc) {
>     Flags |= DWARF2_FLAG_PROLOGUE_END | DWARF2_FLAG_IS_STMT;
>     PrologEndLoc = DebugLoc();
>   }
> -  if (DL.getLine() !=
> -      Asm->OutStreamer->getContext().getCurrentDwarfLoc().getLine())
> +  // If the line changed, we call that a new statement; unless we went to
> +  // line 0 and came back, in which case it is not a new statement.
> +  unsigned OldLine = PrevInstLoc ? PrevInstLoc.getLine() : LastAsmLine;
> +  if (DL.getLine() && DL.getLine() != OldLine)
>     Flags |= DWARF2_FLAG_IS_STMT;
>
>   const MDNode *Scope = DL.getScope();
>   recordSourceLine(DL.getLine(), DL.getCol(), Scope, Flags);
> +
> +  // If we're not at line 0, remember this location.
> +  if (DL.getLine())
> +    PrevInstLoc = DL;
> }
>
> static DebugLoc findPrologueEndLoc(const MachineFunction *MF) {
>
> Modified: llvm/trunk/test/CodeGen/X86/stack-protector.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/stack-protector.ll?rev=289468&r1=289467&r2=289468&view=diff
> ==============================================================================
> --- llvm/trunk/test/CodeGen/X86/stack-protector.ll (original)
> +++ llvm/trunk/test/CodeGen/X86/stack-protector.ll Mon Dec 12 14:49:11 2016
> @@ -3888,23 +3888,27 @@ entry:
> define void @test32() #1 !dbg !7 {
> entry:
> ; LINUX-I386-LABEL: test32:
> -; LINUX-I386:       .loc 1 0 0 prologue_end
> +; LINUX-I386:       .loc 1 4 2 prologue_end
> +; LINUX-I386:       .loc 1 0 0
> ; LINUX-I386-NEXT:  calll __stack_chk_fail
>
> ; LINUX-X64-LABEL: test32:
> -; LINUX-X64:       .loc 1 0 0 prologue_end
> +; LINUX-X64:       .loc 1 4 2 prologue_end
> +; LINUX-X64:       .loc 1 0 0
> ; LINUX-X64-NEXT:  callq __stack_chk_fail
>
> ; LINUX-KERNEL-X64-LABEL: test32:
> -; LINUX-KERNEL-X64:       .loc 1 0 0 prologue_end
> +; LINUX-KERNEL-X64:       .loc 1 4 2 prologue_end
> +; LINUX-KERNEL-X64:       .loc 1 0 0
> ; LINUX-KERNEL-X64-NEXT:  callq __stack_chk_fail
>
> ; OPENBSD-AMD64-LABEL: test32:
> -; OPENBSD-AMD64:       .loc 1 0 0 prologue_end
> +; OPENBSD-AMD64:       .loc 1 4 2 prologue_end
> +; OPENBSD-AMD64:       .loc 1 0 0
> ; OPENBSD-AMD64-NEXT:  movl
> ; OPENBSD-AMD64-NEXT:  callq __stack_smash_handler
>   %0 = alloca [5 x i8], align 1
> -  ret void
> +  ret void, !dbg !9
> }
>
> declare double @testi_aux()
> @@ -3940,3 +3944,4 @@ attributes #5 = { ssp "stack-protector-b
> !6 = distinct !DISubprogram(name: "__stack_chk_fail", scope: !1, type: !8, unit: !0)
> !7 = distinct !DISubprogram(name: "test32", scope: !1, type: !8, unit: !0)
> !8 = !DISubroutineType(types: !2)
> +!9 = !DILocation(line: 4, column: 2, scope: !7)
>
> Modified: llvm/trunk/test/CodeGen/X86/unknown-location.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/unknown-location.ll?rev=289468&r1=289467&r2=289468&view=diff
> ==============================================================================
> --- llvm/trunk/test/CodeGen/X86/unknown-location.ll (original)
> +++ llvm/trunk/test/CodeGen/X86/unknown-location.ll Mon Dec 12 14:49:11 2016
> @@ -1,4 +1,4 @@
> -; RUN: llc < %s -asm-verbose=false -mtriple=x86_64-apple-darwin10 -use-unknown-locations | FileCheck %s
> +; RUN: llc < %s -asm-verbose=false -mtriple=x86_64-apple-darwin10 -use-unknown-locations=Enable | FileCheck %s
>
> ; The divide instruction does not have a debug location. CodeGen should
> ; represent this in the debug information. This is done by setting line
>
> Modified: llvm/trunk/test/DebugInfo/AArch64/line-header.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/DebugInfo/AArch64/line-header.ll?rev=289468&r1=289467&r2=289468&view=diff
> ==============================================================================
> --- llvm/trunk/test/DebugInfo/AArch64/line-header.ll (original)
> +++ llvm/trunk/test/DebugInfo/AArch64/line-header.ll Mon Dec 12 14:49:11 2016
> @@ -3,4 +3,4 @@
>
> ; check line table length is correctly calculated for both big and little endian
> CHECK-LABEL: .debug_line contents:
> -CHECK: total_length: 0x0000003c
> +CHECK: total_length: 0x0000003e
>
> Modified: llvm/trunk/test/DebugInfo/MIR/X86/no-cfi-loc.mir
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/DebugInfo/MIR/X86/no-cfi-loc.mir?rev=289468&r1=289467&r2=289468&view=diff
> ==============================================================================
> --- llvm/trunk/test/DebugInfo/MIR/X86/no-cfi-loc.mir (original)
> +++ llvm/trunk/test/DebugInfo/MIR/X86/no-cfi-loc.mir Mon Dec 12 14:49:11 2016
> @@ -1,6 +1,6 @@
> # Verify that a CFI instruction with no debug location
> # does not result in a line-0 location in the assembler.
> -# RUN: %llc_dwarf -start-after=prologepilog -march=x86-64 -use-unknown-locations %s -o - | FileCheck %s
> +# RUN: %llc_dwarf -start-after=prologepilog -march=x86-64 -use-unknown-locations=Enable %s -o - | FileCheck %s
> #
> # CHECK-NOT: .loc 1 0
> # CHECK:     .cfi_def_cfa_offset
>
> Added: llvm/trunk/test/DebugInfo/X86/dwarf-no-source-loc.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/DebugInfo/X86/dwarf-no-source-loc.ll?rev=289468&view=auto
> ==============================================================================
> --- llvm/trunk/test/DebugInfo/X86/dwarf-no-source-loc.ll (added)
> +++ llvm/trunk/test/DebugInfo/X86/dwarf-no-source-loc.ll Mon Dec 12 14:49:11 2016
> @@ -0,0 +1,77 @@
> +; Verify "no source location" directives appear in appropriate places,
> +; but don't appear when we explicitly suppress them.
> +; RUN: llc < %s -o - | FileCheck %s
> +; RUN: llc < %s -o - -use-unknown-locations=Disable | FileCheck %s --check-prefix=DISABLE
> +
> +; Generated from this .cpp targeting linux using -g
> +; and then removed function attributes as clutter.
> +;
> +; void bar(int *);
> +; void baz(int *);
> +; # 5 "no-source-loc.cpp"
> +; void foo(int x) {
> +;   int z;
> +;   if (x)
> +; # 20 "include.h"
> +;     bar(&z);
> +; # 10 "no-source-loc.cpp"
> +;   baz(&z);
> +; }
> +
> +target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
> +target triple = "x86_64-unknown-linux-gnu"
> +
> +; Function Attrs: uwtable
> +define void @_Z3fooi(i32 %x) !dbg !6 {
> +entry:
> +  %x.addr = alloca i32, align 4
> +  %z = alloca i32, align 4
> +  store i32 %x, i32* %x.addr, align 4
> +  %0 = load i32, i32* %x.addr, align 4, !dbg !8
> +  %tobool = icmp ne i32 %0, 0, !dbg !8
> +  br i1 %tobool, label %if.then, label %if.end, !dbg !8
> +
> +if.then:                                          ; preds = %entry
> +  call void @_Z3barPi(i32* %z), !dbg !9
> +  br label %if.end, !dbg !9
> +
> +if.end:                                           ; preds = %if.then, %entry
> +  call void @_Z3bazPi(i32* %z), !dbg !12
> +  ret void, !dbg !14
> +}
> +
> +; CHECK:      .loc 1 7 7
> +; CHECK-NOT:  .loc
> +; CHECK:      .loc 1 0 0 is_stmt 0
> +; CHECK-NOT:  .loc
> +; CHECK:      .loc 2 20 5 is_stmt 1
> +; CHECK:      .LBB0_2:
> +; CHECK-NEXT: .loc 2 0 0 is_stmt 0
> +; CHECK-NOT:  .loc
> +; CHECK:      .loc 1 10 3 is_stmt 1
> +;
> +; DISABLE-NOT: .loc 1 0
> +
> +declare void @_Z3barPi(i32*)
> +
> +declare void @_Z3bazPi(i32*)
> +
> +!llvm.dbg.cu = !{!0}
> +!llvm.module.flags = !{!3, !4}
> +!llvm.ident = !{!5}
> +
> +!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !1, producer: "clang version 4.0.0 (trunk 278782)", isOptimized: false, runtimeVersion: 0, emissionKind: LineTablesOnly, enums: !2)
> +!1 = !DIFile(filename: "no-source-loc.cpp", directory: "/tests")
> +!2 = !{}
> +!3 = !{i32 2, !"Dwarf Version", i32 4}
> +!4 = !{i32 2, !"Debug Info Version", i32 3}
> +!5 = !{!"clang version 4.0.0 (trunk 278782)"}
> +!6 = distinct !DISubprogram(name: "foo", scope: !1, file: !1, line: 5, type: !7, isLocal: false, isDefinition: true, scopeLine: 5, flags: DIFlagPrototyped, isOptimized: false, unit: !0, variables: !2)
> +!7 = !DISubroutineType(types: !2)
> +!8 = !DILocation(line: 7, column: 7, scope: !6)
> +!9 = !DILocation(line: 20, column: 5, scope: !10)
> +!10 = !DILexicalBlockFile(scope: !6, file: !11, discriminator: 0)
> +!11 = !DIFile(filename: "include.h", directory: "/tests")
> +!12 = !DILocation(line: 10, column: 3, scope: !13)
> +!13 = !DILexicalBlockFile(scope: !6, file: !1, discriminator: 0)
> +!14 = !DILocation(line: 11, column: 1, scope: !13)
>
> Modified: llvm/trunk/test/DebugInfo/X86/tail-merge.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/DebugInfo/X86/tail-merge.ll?rev=289468&r1=289467&r2=289468&view=diff
> ==============================================================================
> --- llvm/trunk/test/DebugInfo/X86/tail-merge.ll (original)
> +++ llvm/trunk/test/DebugInfo/X86/tail-merge.ll Mon Dec 12 14:49:11 2016
> @@ -1,4 +1,4 @@
> -; RUN: llc %s -mtriple=x86_64-unknown-unknown -use-unknown-locations=true -o - | FileCheck %s
> +; RUN: llc %s -mtriple=x86_64-unknown-unknown -use-unknown-locations=Enable -o - | FileCheck %s
>
> ; Generated with "clang -gline-tables-only -c -emit-llvm -o - | opt -sroa -S"
> ; from source:
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org<mailto: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/20161216/06d432ff/attachment.html>


More information about the llvm-commits mailing list