[llvm] r340508 - [DebugInfo] Fix bug in LiveDebugVariables.

Wed Aug 22 22:24:03 PDT 2018

On Wed, Aug 22, 2018 at 8:29 PM Hsiangkai Wang via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> Author: hsiangkai
> Date: Wed Aug 22 20:28:24 2018
> New Revision: 340508
>
> URL: http://llvm.org/viewvc/llvm-project?rev=340508&view=rev
> Log:
> [DebugInfo] Fix bug in LiveDebugVariables.
>
> In lib/CodeGen/LiveDebugVariables.cpp, it uses std::prev(MBBI) to
> get DebugValue's SlotIndex. However, the previous instruction may be
> also a debug instruction. It could not use a debug instruction to query
> SlotIndex in mi2iMap.
>
> Scan all debug instructions and use the first debug instruction to query
> SlotIndex for following debug instructions. Only handle DBG_VALUE in
> handleDebugValue().
>
> Differential Revision: https://reviews.llvm.org/D50621
>
> Added:
>     llvm/trunk/test/DebugInfo/Generic/debug-var-slot.ll
> Modified:
>     llvm/trunk/include/llvm/CodeGen/SlotIndexes.h
>     llvm/trunk/lib/CodeGen/LiveDebugVariables.cpp
>
> Modified: llvm/trunk/include/llvm/CodeGen/SlotIndexes.h
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/SlotIndexes.h?rev=340508&r1=340507&r2=340508&view=diff
>
> ==============================================================================
> --- llvm/trunk/include/llvm/CodeGen/SlotIndexes.h (original)
> +++ llvm/trunk/include/llvm/CodeGen/SlotIndexes.h Wed Aug 22 20:28:24 2018
> @@ -414,6 +414,8 @@ class raw_ostream;
>      SlotIndex getInstructionIndex(const MachineInstr &MI) const {
>        // Instructions inside a bundle have the same number as the bundle
> itself.
>        const MachineInstr &BundleStart = *getBundleStart(MI.getIterator());
> +      assert(!BundleStart.isDebugInstr() &&
> +             "Could not use a debug instruction to query mi2iMap.");
>        Mi2IndexMap::const_iterator itr = mi2iMap.find(&BundleStart);
>        assert(itr != mi2iMap.end() && "Instruction not found in maps.");
>        return itr->second;
>
> Modified: llvm/trunk/lib/CodeGen/LiveDebugVariables.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/LiveDebugVariables.cpp?rev=340508&r1=340507&r2=340508&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/LiveDebugVariables.cpp (original)
> +++ llvm/trunk/lib/CodeGen/LiveDebugVariables.cpp Wed Aug 22 20:28:24 2018
> @@ -578,23 +578,28 @@ bool LDVImpl::collectDebugValues(Machine
>      MachineBasicBlock *MBB = &*MFI;
>      for (MachineBasicBlock::iterator MBBI = MBB->begin(), MBBE =
> MBB->end();
>           MBBI != MBBE;) {
> -      if (!MBBI->isDebugValue()) {
> +      // Use the first debug instruction in the sequence to get a
> SlotIndex
> +      // for following consecutive debug instructions.
> +      if (!MBBI->isDebugInstr()) {
>          ++MBBI;
>          continue;
>        }
> -      // DBG_VALUE has no slot index, use the previous instruction
> instead.
> +      // Debug instructions has no slot index. Use the previous
> +      // non-debug instruction's SlotIndex as its SlotIndex.
>        SlotIndex Idx =
>            MBBI == MBB->begin()
>                ? LIS->getMBBStartIdx(MBB)
>                : LIS->getInstructionIndex(*std::prev(MBBI)).getRegSlot();
> -      // Handle consecutive DBG_VALUE instructions with the same slot
> index.
> +      // Handle consecutive debug instructions with the same slot index.
>        do {
> -        if (handleDebugValue(*MBBI, Idx)) {
> +        // Only handle DBG_VALUE in handleDebugValue(). Skip all other
> +        // kinds of debug instructions.
> +        if (MBBI->isDebugValue() && handleDebugValue(*MBBI, Idx)) {
>            MBBI = MBB->erase(MBBI);
>            Changed = true;
>          } else
>            ++MBBI;
> -      } while (MBBI != MBBE && MBBI->isDebugValue());
> +      } while (MBBI != MBBE && MBBI->isDebugInstr());
>      }
>    }
>    return Changed;
>
> Added: llvm/trunk/test/DebugInfo/Generic/debug-var-slot.ll
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/DebugInfo/Generic/debug-var-slot.ll?rev=340508&view=auto
>
> ==============================================================================
> --- llvm/trunk/test/DebugInfo/Generic/debug-var-slot.ll (added)
> +++ llvm/trunk/test/DebugInfo/Generic/debug-var-slot.ll Wed Aug 22
> 20:28:24 2018
> @@ -0,0 +1,59 @@
> +; After adding new debug instruction for labels, it is possible to have
> +; debug instructions before DBG_VALUE. When querying DBG_VALUE's slot
> +; index using previous instruction and the previous instruction is debug
> +; instruction, it will trigger an assertion as using debug instruction
> +; to get slot index. This test is to emulate the case when DBG_VALUE's
> +; previous instruction is DBG_LABEL in LiveDebugVariables pass.
> +;
> +; RUN: llc < %s -stop-after=livedebugvars -debug 2>&1 | FileCheck %s
>

You shouldn't use debug output as the primary way of testing. Notably, it
doesn't work when asserts are disabled. See my more detailed comments on
the review.

I'm reverting for now as its failing on several bots:
http://lab.llvm.org:8011/builders/clang-with-thin-lto-ubuntu/builds/12232/steps/test-stage1-compiler/logs/stdio

> +;
> +; CHECK: COMPUTING LIVE DEBUG VARIABLES: foo
> +; CHECK: DEBUG VARIABLES
> +; CHECK-NEXT: "local_var,7"
> +
> +source_filename = "debug-var-slot.c"
> +
> +define dso_local i32 @foo(i32 %a, i32 %b) !dbg !6 {
> +entry:
> +  %a.addr = alloca i32, align 4
> +  %b.addr = alloca i32, align 4
> +  %sum = alloca i32, align 4
> +  store i32 %a, i32* %a.addr, align 4
> +  store i32 %b, i32* %b.addr, align 4
> +  br label %top
> +
> +top:
> +  call void @llvm.dbg.label(metadata !10), !dbg !13
> +  call void @llvm.dbg.value(metadata i32 %0, metadata !12, metadata
> !DIExpression()), !dbg !14
> +  %0 = load i32, i32* %a.addr, align 4
> +  %1 = load i32, i32* %a.addr, align 4
> +  %2 = load i32, i32* %b.addr, align 4
> +  %add = add nsw i32 %1, %2
> +  store i32 %add, i32* %sum, align 4
> +  br label %done
> +
> +done:
> +  %3 = load i32, i32* %sum, align 4
> +  ret i32 %3, !dbg !15
> +}
> +
> +declare void @llvm.dbg.label(metadata)
> +declare void @llvm.dbg.value(metadata, metadata, metadata)
> +
> +!llvm.dbg.cu = !{!0}
> +!llvm.module.flags = !{!4}
> +
> +!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1,
> isOptimized: true, emissionKind: FullDebug, enums: !2)
> +!1 = !DIFile(filename: "debug-var-slot.c", directory: "./")
> +!2 = !{}
> +!4 = !{i32 2, !"Debug Info Version", i32 3}
> +!6 = distinct !DISubprogram(name: "foo", scope: !1, file: !1, line: 1,
> type: !7, isLocal: false, isDefinition: true, scopeLine: 2, isOptimized:
> true, unit: !0, retainedNodes: !2)
> +!7 = !DISubroutineType(types: !8)
> +!8 = !{!9, !9, !9}
> +!9 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
> +!10 = !DILabel(scope: !6, name: "top", file: !1, line: 4)
> +!11 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
> +!12 = !DILocalVariable(name: "local_var", scope: !6, file: !1, line: 7,
> type: !11)
> +!13 = !DILocation(line: 4, column: 1, scope: !6)
> +!14 = !DILocation(line: 7, column: 1, scope: !6)
> +!15 = !DILocation(line: 8, column: 3, scope: !6)
>
>
> _______________________________________________
> 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/20180822/090a2d0c/attachment.html>