[TailDuplicator] Don't constrain register classes due to debug instructions

[uabelho]

Summary

If cloning a DBG_VALUE instruction, register uses in that instruction could
lead to constraining of a virtual register that would not happen if the
DBG_VALUE was not present at all. This lead to different code with/without
debug info.

Now we only do that register class constraining if we dealing with a non
debug instruction.

[uabelho]

I don't know if this is the best fix for the problem but it does solve a case where we got different codegen with/without debug info present. Originally found for my out of tree target but I think the attached AArch64 testcase exposes the problem as well.

I'm also not sure who can review this. Added @kparzysz since I think you wrote the original code and @foad and @arsenm since I think I've seen you in other reviews of TailDuplicator.

[foad]

llvm/lib/CodeGen/TailDuplicator.cpp

434

Just a cosmetic thing, I'd prefer you swap the "if" and "else" cases so you don't have an "else" after an already negated condition.

[uabelho]

Swap if/else according to comment.

[uabelho]

llvm/lib/CodeGen/TailDuplicator.cpp

434

Sure, done!

[arsenm]

llvm/lib/CodeGen/TailDuplicator.cpp

434

ConstRC = isDebug ? MappedRC : MRI->constrain()?

Could you also just initialize ConstRC with MappedRC above?

[uabelho]

llvm/lib/CodeGen/TailDuplicator.cpp

434

I guess initializing

const TargetRegisterClass *ConstrRC = Mapped;

plus then doing

if (!NewMI.isDebugInstr())
  ConstRC = MRI->constrainRegClass(VI->second.Reg, OrigRC);

would be the same thing.

I think I prefer just

ConstrRC = NewMI.isDebugInstr()
               ? MappedRC
               : MRI->constrainRegClass(VI->second.Reg, OrigRC);

though so I'll update the patch with that.

[uabelho]

Change if/else to ? accordiing to review comment.

[uabelho]

Thanks!

llvm/lib/CodeGen/TailDuplicator.cpp

for (unsigned i = 0, e = NewMI.getNumOperands(); i != e; ++i) {
               // The actual constraining (as in "find appropriate new class")
               // is done by getMatchingSuperRegClass, so now we only need to
               // change the class of the mapped register.
               MRI->setRegClass(VI->second.Reg, ConstrRC);
             }
           } else {
             // For mapped registers that do not have sub-registers, simply
             // restrict their class to match the original one.
-            ConstrRC = MRI->constrainRegClass(VI->second.Reg, OrigRC);
+
+            // We don't want debug instructions affecting the resulting code so
+            // if we're cloning a debug instruction then just use MappedRC
+            // rather than constraining the register class further.
+            ConstrRC = NewMI.isDebugInstr()
+                           ? MappedRC
+                           : MRI->constrainRegClass(VI->second.Reg, OrigRC);
           }
 
           if (ConstrRC) {
             // If the class constraining succeeded, we can simply replace
             // the old register with the mapped one.
             MO.setReg(VI->second.Reg);
             // We have Reg -> VI.Reg:VI.SubReg, so if Reg is used with a
             // sub-register, we need to compose the sub-register indices.

llvm/test/CodeGen/AArch64/tail-duplicate-debug-variance.mir

+# RUN: llc %s -o - -run-pass=early-tailduplication | FileCheck %s --check-prefixes=CHECK,DEBUG
+# RUN: llc %s -o - -run-pass=mir-strip-debug,early-tailduplication -mir-strip-debugify-only=0 | FileCheck %s --check-prefix=CHECK
+
+# We should get the same result with and without debug information.
+# Specifically, the register class of [[COPY]] should not be changed to
+# gpr32common with debug info while staying as gpr32sp without.
+
+--- |
+  target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+  target triple = "aarch64-none-linux-gnu"
+
+  define void @test() !dbg !8 {
+  entry:
+    ret void
+  }
+
+  !llvm.dbg.cu = !{!0}
+  !llvm.module.flags = !{!2, !3, !4, !5}
+  !llvm.ident = !{!6}
+
+  !0 = distinct !DICompileUnit(language: DW_LANG_C11, file: !1, producer: "My compiler", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, splitDebugInlining: false, nameTableKind: None)
+  !1 = !DIFile(filename: "foo.c", directory: "/bar")
+  !2 = !{i32 7, !"Dwarf Version", i32 4}
+  !3 = !{i32 2, !"Debug Info Version", i32 3}
+  !4 = !{i32 1, !"wchar_size", i32 1}
+  !5 = !{i32 7, !"frame-pointer", i32 2}
+  !6 = !{!"My compiler"}
+  !7 = !DILocalVariable(name: "i", scope: !8, file: !1, line: 6, type: !12)
+  !8 = distinct !DISubprogram(name: "test", scope: !1, file: !1, line: 5, type: !9, scopeLine: 5, flags: DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !11)
+  !9 = !DISubroutineType(types: !10)
+  !10 = !{null}
+  !11 = !{!7}
+  !12 = !DIBasicType(name: "int", size: 16, encoding: DW_ATE_signed)
+  !13 = !DILocation(line: 0, scope: !8)
+...
+---
+name:            test
+tracksRegLiveness: true
+liveins:         []
+body:             |
+  ; CHECK-LABEL: name: test
+  ; CHECK: bb.0:
+  ; CHECK-NEXT:   successors: %bb.2(0x40000000), %bb.1(0x40000000)
+  ; CHECK-NEXT:   liveins: $w0, $wzr
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   [[COPY:%[0-9]+]]:gpr32sp = COPY $w0
+  ; CHECK-NEXT:   CBNZW $wzr, %bb.2
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.1:
+  ; CHECK-NEXT:   successors: %bb.4(0x80000000)
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   $x0 = COPY undef $x0
+  ; DEBUG-NEXT:   DBG_VALUE [[COPY]], $noreg, !11, !DIExpression(), debug-location !DILocation(line: 0, scope: !7)
+  ; CHECK-NEXT:   [[COPY1:%[0-9]+]]:gpr32 = COPY [[COPY]]
+  ; CHECK-NEXT:   B %bb.4
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.2:
+  ; CHECK-NEXT:   successors: %bb.4(0x80000000)
+  ; CHECK-NEXT: {{  $}}
+  ; DEBUG-NEXT:   DBG_VALUE [[COPY]], $noreg, !11, !DIExpression(), debug-location !DILocation(line: 0, scope: !7)
+  ; CHECK-NEXT:   [[COPY2:%[0-9]+]]:gpr32 = COPY [[COPY]]
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.4:
+  ; CHECK-NEXT:   RET undef $lr, implicit undef $w0
+  bb.0:
+    successors: %bb.2, %bb.1
+    liveins: $w0, $wzr
+
+    %1:gpr32sp = COPY $w0
+    CBNZW $wzr, %bb.2
+
+  bb.1:
+    successors: %bb.3
+
+    $x0 = COPY undef $x0
+    B %bb.3
+
+  bb.2:
+    successors: %bb.3
+
+  bb.3:
+    successors: %bb.5
+
+    %2:gpr32 = PHI %1, %bb.1, %1, %bb.2
+    DBG_VALUE %2, $noreg, !7, !DIExpression(), debug-location !13
+    B %bb.5
+
+  bb.4:
+    RET undef $lr, implicit undef $w0
+
+  bb.5:
+    successors: %bb.4
+
+    B %bb.4
+...