[llvm] r340679 - [LiveDebugVariables] Avoid faulty addDefsFromCopies in computeIntervals

Sat Aug 25 03:02:03 PDT 2018

Author: bjope
Date: Sat Aug 25 03:02:03 2018
New Revision: 340679

URL: http://llvm.org/viewvc/llvm-project?rev=340679&view=rev
Log:
[LiveDebugVariables] Avoid faulty addDefsFromCopies in computeIntervals

Summary:
When computeIntervals is looking through COPY instruction to
extend the location mapping for a debug variable it did not
handle subregisters correctly.

For example
    DBG_VALUE debug-use %0.sub_8bit_hi, ...
    %1:gr16 = COPY %0
was transformed into
    DBG_VALUE debug-use %0.sub_8bit_hi, ...
    %1:gr16 = COPY %0
    DBG_VALUE debug-use %1, ...
So the subregister index was missing in the added DBG_VALUE.

As long as the subreg refered to the least significant bits
of the superreg, then I guess we could get the correct
result in a debugger even when referring to the superreg.
But as in the example above when the subreg refers to other
parts of the superreg, then debuginfo would be incorrect.

I'm not sure exactly how to fix this properly, so this patch
just avoids looking through the COPY when there is a subreg
involved (for more info, see the FIXME added in the code).

Reviewers: rnk, aprantl

Reviewed By: aprantl

Subscribers: JDevlieghere, llvm-commits

Tags: #debug-info

Differential Revision: https://reviews.llvm.org/D50788

Added:
    llvm/trunk/test/CodeGen/X86/dbg-value-superreg-copy.mir
Modified:
    llvm/trunk/lib/CodeGen/LiveDebugVariables.cpp

Modified: llvm/trunk/lib/CodeGen/LiveDebugVariables.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/LiveDebugVariables.cpp?rev=340679&r1=340678&r2=340679&view=diff
==============================================================================

--- llvm/trunk/lib/CodeGen/LiveDebugVariables.cpp (original)
+++ llvm/trunk/lib/CodeGen/LiveDebugVariables.cpp Sat Aug 25 03:02:03 2018
@@ -752,7 +752,15 @@ void UserValue::computeIntervals(Machine
       }
       SmallVector<SlotIndex, 16> Kills;
       extendDef(Idx, Loc, LI, VNI, &Kills, LIS);
-      if (LI)
+      // FIXME: Handle sub-registers in addDefsFromCopies. The problem is that
+      // if the original location for example is %vreg0:sub_hi, and we find a
+      // full register copy in addDefsFromCopies (at the moment it only handles
+      // full register copies), then we must add the sub1 sub-register index to
+      // the new location. However, that is only possible if the new virtual
+      // register is of the same regclass (or if there is an equivalent
+      // sub-register in that regclass). For now, simply skip handling copies if
+      // a sub-register is involved.
+      if (LI && !LocMO.getSubReg())
         addDefsFromCopies(LI, Loc.locNo(), Loc.wasIndirect(), Kills, Defs, MRI,
                           LIS);
       continue;

Added: llvm/trunk/test/CodeGen/X86/dbg-value-superreg-copy.mir
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/dbg-value-superreg-copy.mir?rev=340679&view=auto
==============================================================================
--- llvm/trunk/test/CodeGen/X86/dbg-value-superreg-copy.mir (added)
+++ llvm/trunk/test/CodeGen/X86/dbg-value-superreg-copy.mir Sat Aug 25 03:02:03 2018
@@ -0,0 +1,67 @@
+# RUN: llc -O1 -start-after simple-register-coalescing -o - %s | FileCheck %s
+
+--- |
+  target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+  target triple = "x86_64-pc-linux-gnu"
+
+  define i16 @foo(i8 %zzz) !dbg !4 {
+  entry:
+    ret i16 1
+  }
+
+  ; Function Attrs: nounwind readnone speculatable
+  declare void @llvm.dbg.value(metadata, metadata, metadata)
+
+  !llvm.dbg.cu = !{!0}
+  !llvm.module.flags = !{!3}
+
+  !0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, retainedTypes: !2)
+  !1 = !DIFile(filename: "test.c", directory: "")
+  !2 = !{}
+  !3 = !{i32 1, !"Debug Info Version", i32 3}
+  !4 = distinct !DISubprogram(name: "foo", scope: !1, file: !1, line: 3, type: !5, isLocal: false, isDefinition: true, scopeLine: 3, virtualIndex: 6, flags: DIFlagPrototyped, isOptimized: false, unit: !0)
+  !5 = !DISubroutineType(types: !6)
+  !6 = !{null}
+  !7 = !DILocalVariable(name: "zzz", arg: 1, scope: !4, file: !1, line: 3, type: !8)
+  !8 = !DIBasicType(name: "char", size: 8, align: 8, encoding: DW_ATE_signed)
+  !9 = !DILocation(line: 0, scope: !4)
+  !10 = !DILocation(line: 4, column: 22, scope: !11)
+  !11 = distinct !DILexicalBlock(scope: !4, file: !1, line: 3, column: 19)
+
+...
+---
+name:            foo
+tracksRegLiveness: true
+body:             |
+  bb.0:
+    %0:gr16_abcd = MOV16ri 1
+
+  bb.1:
+    DBG_VALUE debug-use %0.sub_8bit_hi, debug-use $noreg, !7, !DIExpression(), debug-location !9
+    %1:gr16 = COPY %0
+    %2:gr16 = COPY %0
+
+  bb.2:
+    $ax = COPY %1
+    $dx = COPY %2
+    RETQ killed $ax, killed $dx
+...
+
+# This test case was created as a reproducer for a bug when we got incorrect
+# DBG_VALUE instructions after regalloc like this:
+#
+#        movw    $1, %cx
+#        #DEBUG_VALUE: foo:zzz <- $ch
+#        movl    %ecx, %eax
+#        #DEBUG_VALUE: foo:zzz <- $ax
+#
+# The above is incorrect since the DBG_VALUE in the input is refering to the
+# hi subreg, so after the COPY/movl the value is in $ah and not $ax (nor $al).
+#
+# We currently only get one DEBUG_VALUE here. In the future we could allow a
+# second DEBUG_VALUE, as long as it is mapped to the hi subreg of the movl
+# dst.
+#
+# CHECK-NOT:    #DEBUG_VALUE:
+# CHECK:        #DEBUG_VALUE: foo:zzz <- ${{[abcd]+}}h
+# CHECK-NOT:    #DEBUG_VALUE:


