<div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Wed, Aug 22, 2018 at 8:29 PM Hsiangkai Wang via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: hsiangkai<br>
Date: Wed Aug 22 20:28:24 2018<br>
New Revision: 340508<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=340508&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=340508&view=rev</a><br>
Log:<br>
[DebugInfo] Fix bug in LiveDebugVariables.<br>
<br>
In lib/CodeGen/LiveDebugVariables.cpp, it uses std::prev(MBBI) to<br>
get DebugValue's SlotIndex. However, the previous instruction may be<br>
also a debug instruction. It could not use a debug instruction to query<br>
SlotIndex in mi2iMap.<br>
<br>
Scan all debug instructions and use the first debug instruction to query<br>
SlotIndex for following debug instructions. Only handle DBG_VALUE in<br>
handleDebugValue().<br>
<br>
Differential Revision: <a href="https://reviews.llvm.org/D50621" rel="noreferrer" target="_blank">https://reviews.llvm.org/D50621</a><br>
<br>
Added:<br>
    llvm/trunk/test/DebugInfo/Generic/debug-var-slot.ll<br>
Modified:<br>
    llvm/trunk/include/llvm/CodeGen/SlotIndexes.h<br>
    llvm/trunk/lib/CodeGen/LiveDebugVariables.cpp<br>
<br>
Modified: llvm/trunk/include/llvm/CodeGen/SlotIndexes.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/SlotIndexes.h?rev=340508&r1=340507&r2=340508&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/SlotIndexes.h?rev=340508&r1=340507&r2=340508&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/include/llvm/CodeGen/SlotIndexes.h (original)<br>
+++ llvm/trunk/include/llvm/CodeGen/SlotIndexes.h Wed Aug 22 20:28:24 2018<br>
@@ -414,6 +414,8 @@ class raw_ostream;<br>
     SlotIndex getInstructionIndex(const MachineInstr &MI) const {<br>
       // Instructions inside a bundle have the same number as the bundle itself.<br>
       const MachineInstr &BundleStart = *getBundleStart(MI.getIterator());<br>
+      assert(!BundleStart.isDebugInstr() &&<br>
+             "Could not use a debug instruction to query mi2iMap.");<br>
       Mi2IndexMap::const_iterator itr = mi2iMap.find(&BundleStart);<br>
       assert(itr != mi2iMap.end() && "Instruction not found in maps.");<br>
       return itr->second;<br>
<br>
Modified: llvm/trunk/lib/CodeGen/LiveDebugVariables.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/LiveDebugVariables.cpp?rev=340508&r1=340507&r2=340508&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/LiveDebugVariables.cpp?rev=340508&r1=340507&r2=340508&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/CodeGen/LiveDebugVariables.cpp (original)<br>
+++ llvm/trunk/lib/CodeGen/LiveDebugVariables.cpp Wed Aug 22 20:28:24 2018<br>
@@ -578,23 +578,28 @@ bool LDVImpl::collectDebugValues(Machine<br>
     MachineBasicBlock *MBB = &*MFI;<br>
     for (MachineBasicBlock::iterator MBBI = MBB->begin(), MBBE = MBB->end();<br>
          MBBI != MBBE;) {<br>
-      if (!MBBI->isDebugValue()) {<br>
+      // Use the first debug instruction in the sequence to get a SlotIndex<br>
+      // for following consecutive debug instructions.<br>
+      if (!MBBI->isDebugInstr()) {<br>
         ++MBBI;<br>
         continue;<br>
       }<br>
-      // DBG_VALUE has no slot index, use the previous instruction instead.<br>
+      // Debug instructions has no slot index. Use the previous<br>
+      // non-debug instruction's SlotIndex as its SlotIndex.<br>
       SlotIndex Idx =<br>
           MBBI == MBB->begin()<br>
               ? LIS->getMBBStartIdx(MBB)<br>
               : LIS->getInstructionIndex(*std::prev(MBBI)).getRegSlot();<br>
-      // Handle consecutive DBG_VALUE instructions with the same slot index.<br>
+      // Handle consecutive debug instructions with the same slot index.<br>
       do {<br>
-        if (handleDebugValue(*MBBI, Idx)) {<br>
+        // Only handle DBG_VALUE in handleDebugValue(). Skip all other<br>
+        // kinds of debug instructions.<br>
+        if (MBBI->isDebugValue() && handleDebugValue(*MBBI, Idx)) {<br>
           MBBI = MBB->erase(MBBI);<br>
           Changed = true;<br>
         } else<br>
           ++MBBI;<br>
-      } while (MBBI != MBBE && MBBI->isDebugValue());<br>
+      } while (MBBI != MBBE && MBBI->isDebugInstr());<br>
     }<br>
   }<br>
   return Changed;<br>
<br>
Added: llvm/trunk/test/DebugInfo/Generic/debug-var-slot.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/DebugInfo/Generic/debug-var-slot.ll?rev=340508&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/DebugInfo/Generic/debug-var-slot.ll?rev=340508&view=auto</a><br>
==============================================================================<br>
--- llvm/trunk/test/DebugInfo/Generic/debug-var-slot.ll (added)<br>
+++ llvm/trunk/test/DebugInfo/Generic/debug-var-slot.ll Wed Aug 22 20:28:24 2018<br>
@@ -0,0 +1,59 @@<br>
+; After adding new debug instruction for labels, it is possible to have<br>
+; debug instructions before DBG_VALUE. When querying DBG_VALUE's slot<br>
+; index using previous instruction and the previous instruction is debug<br>
+; instruction, it will trigger an assertion as using debug instruction<br>
+; to get slot index. This test is to emulate the case when DBG_VALUE's<br>
+; previous instruction is DBG_LABEL in LiveDebugVariables pass.<br>
+;<br>
+; RUN: llc < %s -stop-after=livedebugvars -debug 2>&1 | FileCheck %s<br></blockquote><div><br></div><div>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.</div><div><br></div><div>I'm reverting for now as its failing on several bots:</div><div><a href="http://lab.llvm.org:8011/builders/clang-with-thin-lto-ubuntu/builds/12232/steps/test-stage1-compiler/logs/stdio">http://lab.llvm.org:8011/builders/clang-with-thin-lto-ubuntu/builds/12232/steps/test-stage1-compiler/logs/stdio</a></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+;<br>
+; CHECK: COMPUTING LIVE DEBUG VARIABLES: foo<br>
+; CHECK: DEBUG VARIABLES<br>
+; CHECK-NEXT: "local_var,7"<br>
+<br>
+source_filename = "debug-var-slot.c"<br>
+<br>
+define dso_local i32 @foo(i32 %a, i32 %b) !dbg !6 {<br>
+entry:<br>
+  %a.addr = alloca i32, align 4<br>
+  %b.addr = alloca i32, align 4<br>
+  %sum = alloca i32, align 4<br>
+  store i32 %a, i32* %a.addr, align 4<br>
+  store i32 %b, i32* %b.addr, align 4<br>
+  br label %top<br>
+<br>
+top:<br>
+  call void @llvm.dbg.label(metadata !10), !dbg !13<br>
+  call void @llvm.dbg.value(metadata i32 %0, metadata !12, metadata !DIExpression()), !dbg !14<br>
+  %0 = load i32, i32* %a.addr, align 4<br>
+  %1 = load i32, i32* %a.addr, align 4<br>
+  %2 = load i32, i32* %b.addr, align 4<br>
+  %add = add nsw i32 %1, %2<br>
+  store i32 %add, i32* %sum, align 4<br>
+  br label %done<br>
+<br>
+done:<br>
+  %3 = load i32, i32* %sum, align 4<br>
+  ret i32 %3, !dbg !15<br>
+}<br>
+<br>
+declare void @llvm.dbg.label(metadata)<br>
+declare void @llvm.dbg.value(metadata, metadata, metadata)<br>
+<br>
+!<a href="http://llvm.dbg.cu" rel="noreferrer" target="_blank">llvm.dbg.cu</a> = !{!0}<br>
+!llvm.module.flags = !{!4}<br>
+<br>
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, isOptimized: true, emissionKind: FullDebug, enums: !2)<br>
+!1 = !DIFile(filename: "debug-var-slot.c", directory: "./")<br>
+!2 = !{}<br>
+!4 = !{i32 2, !"Debug Info Version", i32 3}<br>
+!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)<br>
+!7 = !DISubroutineType(types: !8)<br>
+!8 = !{!9, !9, !9}<br>
+!9 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)<br>
+!10 = !DILabel(scope: !6, name: "top", file: !1, line: 4)<br>
+!11 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)<br>
+!12 = !DILocalVariable(name: "local_var", scope: !6, file: !1, line: 7, type: !11)<br>
+!13 = !DILocation(line: 4, column: 1, scope: !6)<br>
+!14 = !DILocation(line: 7, column: 1, scope: !6)<br>
+!15 = !DILocation(line: 8, column: 3, scope: !6)<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div></div>