[SystemZ] Handle DBG_VALUE instructions in two places in backend

[jonpa]

Summary

Two backend optimizations failed to handle cases when compiled with -g, due to failing to consider DBG_VALUE instructions. This was in SystemZTargetLowering::emitSelect() and SystemZElimCompare::getRegReferences().

This patch makes sure that DBG_VALUEs are recognized so that they do not affect these optimizers.

Tests for branch-on-count, load-and-trap and consecutive selects.

[uweigand]

What's the reason for using SkipPHIsLabelsAndDebug instead of, say, skipDebugInstructionsForward? It's not obvious to me that skipping PHIs and labels is safe at all those places ...

Otherwise, this looks good to me.

[jonpa]

What's the reason for using SkipPHIsLabelsAndDebug instead of, say, skipDebugInstructionsForward? It's not obvious to me that skipping PHIs and labels is safe at all those places ...

No reason in particular, and I agree that skipDebugInstructionsForward() seems like a better choice. Patch updated.

[uweigand]

LGTM, thanks!

[jonpa]

Thanks for review. r351928.

lib/Target/SystemZ/SystemZElimCompare.cpp

if (MI.getNumOperands() > 0 && MI.getOperand(0).isReg() &&
     return true;
 
   return (preservesValueOf(MI, Reg));
 }
 
 // Describe the references to Reg or any of its aliases in MI.
 Reference SystemZElimCompare::getRegReferences(MachineInstr &MI, unsigned Reg) {
   Reference Ref;
+  if (MI.isDebugInstr())
+    return Ref;
+
   for (unsigned I = 0, E = MI.getNumOperands(); I != E; ++I) {
     const MachineOperand &MO = MI.getOperand(I);
     if (MO.isReg()) {
       if (unsigned MOReg = MO.getReg()) {
         if (TRI->regsOverlap(MOReg, Reg)) {
           if (MO.isUse())
             Ref.Use = true;
           else if (MO.isDef())

lib/Target/SystemZ/SystemZISelLowering.cpp

static void createPHIsForSelects(MachineBasicBlock::iterator MIItBegin,
   // As we are creating the PHIs, we have to be careful if there is more than
   // one.  Later Selects may reference the results of earlier Selects, but later
   // PHIs have to reference the individual true/false inputs from earlier PHIs.
   // That also means that PHI construction must work forward from earlier to
   // later, and that the code must maintain a mapping from earlier PHI's
   // destination registers, and the registers that went into the PHI.
   DenseMap<unsigned, std::pair<unsigned, unsigned>> RegRewriteTable;
 
-  for (MachineBasicBlock::iterator MIIt = MIItBegin; MIIt != MIItEnd; ++MIIt) {
+  for (MachineBasicBlock::iterator MIIt = MIItBegin; MIIt != MIItEnd;
+       MIIt = skipDebugInstructionsForward(++MIIt, MIItEnd)) {
     unsigned DestReg = MIIt->getOperand(0).getReg();
     unsigned TrueReg = MIIt->getOperand(1).getReg();
     unsigned FalseReg = MIIt->getOperand(2).getReg();
 
     // If this Select we are generating is the opposite condition from
     // the jump we generated, then we have to swap the operands for the
     // PHI that is going to be generated.
     if (MIIt->getOperand(4).getImm() == (CCValid ^ CCMask))
SystemZTargetLowering::emitSelect(MachineInstr &MI,
   unsigned CCValid = MI.getOperand(3).getImm();
   unsigned CCMask = MI.getOperand(4).getImm();
   DebugLoc DL = MI.getDebugLoc();
 
   // If we have a sequence of Select* pseudo instructions using the
   // same condition code value, we want to expand all of them into
   // a single pair of basic blocks using the same condition.
   MachineInstr *LastMI = &MI;
-  MachineBasicBlock::iterator NextMIIt =
-      std::next(MachineBasicBlock::iterator(MI));
+  MachineBasicBlock::iterator NextMIIt = skipDebugInstructionsForward(
+      std::next(MachineBasicBlock::iterator(MI)), MBB->end());
 
   if (isSelectPseudo(MI))
     while (NextMIIt != MBB->end() && isSelectPseudo(*NextMIIt) &&
            NextMIIt->getOperand(3).getImm() == CCValid &&
            (NextMIIt->getOperand(4).getImm() == CCMask ||
             NextMIIt->getOperand(4).getImm() == (CCValid ^ CCMask))) {
       LastMI = &*NextMIIt;
-      ++NextMIIt;
+      NextMIIt = skipDebugInstructionsForward(++NextMIIt, MBB->end());
     }
 
   MachineBasicBlock *StartMBB = MBB;
   MachineBasicBlock *JoinMBB  = splitBlockBefore(MI, MBB);
   MachineBasicBlock *FalseMBB = emitBlockAfter(StartMBB);
 
   // Unless CC was killed in the last Select instruction, mark it as
   // live-in to both FalseMBB and JoinMBB.
SystemZTargetLowering::emitSelect(MachineInstr &MI,
   MBB = FalseMBB;
   MBB->addSuccessor(JoinMBB);
 
   //  JoinMBB:
   //   %Result = phi [ %FalseReg, FalseMBB ], [ %TrueReg, StartMBB ]
   //  ...
   MBB = JoinMBB;
   MachineBasicBlock::iterator MIItBegin = MachineBasicBlock::iterator(MI);
-  MachineBasicBlock::iterator MIItEnd =
-      std::next(MachineBasicBlock::iterator(LastMI));
+  MachineBasicBlock::iterator MIItEnd = skipDebugInstructionsForward(
+      std::next(MachineBasicBlock::iterator(LastMI)), MBB->end());
   createPHIsForSelects(MIItBegin, MIItEnd, StartMBB, FalseMBB, MBB);
 
   StartMBB->erase(MIItBegin, MIItEnd);
   return JoinMBB;
 }
 
 // Implement EmitInstrWithCustomInserter for pseudo CondStore* instruction MI.
 // StoreOpcode is the store to use and Invert says whether the store should

test/CodeGen/SystemZ/debuginstr-00.mir

+# Check that the backend can produce a branch-on-count instruction also in
+# the presence of a DEBUG_VALUE machine instruction.
+#
+# RUN: llc %s -mtriple=s390x-linux-gnu -mcpu=z13 -start-before=postrapseudos \
+# RUN:   -o - 2>&1 | FileCheck %s
+#
+# CHECK: brctg
+
+--- |
+  
+  define dso_local void @put_charge_groups_in_box() local_unnamed_addr #0 !dbg !5 {
+  bb:
+    br label %bb2
+  
+  bb2:                                              ; preds = %bb2, %bb3, %bb
+    br i1 undef, label %bb2, label %bb3.preheader
+  
+  bb3.preheader:                                    ; preds = %bb2
+    br label %bb3
+  
+  bb3:                                              ; preds = %bb3.preheader, %bb3
+    %tmp4 = phi i64 [ %tmp6, %bb3 ], [ 2, %bb3.preheader ]
+    %tmp6 = add nsw i64 %tmp4, -1
+    ; Keep the dbg metadata live by referencing it in the IR.
+    call void @llvm.dbg.value(metadata i32 0, i64 0, metadata !7, metadata !DIExpression())
+    %tmp8 = icmp eq i64 %tmp6, 0
+    br i1 %tmp8, label %bb2, label %bb3
+  }
+  
+  declare void @llvm.dbg.value(metadata, i64, metadata, metadata) #1
+  
+  declare void @llvm.stackprotector(i8*, i8**) #2
+  
+  attributes #0 = { "target-cpu"="z13" "use-soft-float"="false" }
+  attributes #1 = { nounwind readnone speculatable "target-cpu"="z13" }
+  attributes #2 = { nounwind }
+  
+  !llvm.dbg.cu = !{!0}
+  !llvm.module.flags = !{!3, !4}
+  
+  !0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "llvm", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !2)
+  !1 = !DIFile(filename: "test.ll", directory: "/tmp")
+  !2 = !{}
+  !3 = !{i32 2, !"Dwarf Version", i32 4}
+  !4 = !{i32 2, !"Debug Info Version", i32 3}
+  !5 = distinct !DISubprogram(name: "test_dbg_value", scope: !1, file: !1, line: 1, type: !6, isLocal: false, isDefinition: true, scopeLine: 1, flags: DIFlagPrototyped, isOptimized: false, unit: !0, retainedNodes: !2)
+  !6 = !DISubroutineType(types: !2)
+  !7 = !DILocalVariable(name: "in", arg: 1, scope: !5, file: !1, line: 1, type: !8)
+  !8 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+  !9 = !DILocation(line: 1, column: 1, scope: !5)
+
+...
+---
+name:            put_charge_groups_in_box
+alignment:       4
+tracksRegLiveness: true
+frameInfo:       
+  maxCallFrameSize: 0
+body:             |
+  bb.0.bb:
+    renamable $r0l = LHIMux 0
+  
+  bb.1.bb2:
+    liveins: $r0l
+  
+    CHIMux renamable $r0l, 0, implicit-def $cc
+    BRC 14, 6, %bb.1, implicit killed $cc
+  
+  bb.2.bb3.preheader:
+    liveins: $r0l
+  
+    renamable $r1d = LGHI 2
+  
+  bb.3.bb3:
+    successors: %bb.1(0x04000000), %bb.3(0x7c000000)
+    liveins: $r1d, $r0l
+  
+    renamable $r1d = nsw AGHI killed renamable $r1d, -1, implicit-def dead $cc
+    DBG_VALUE $r1d, $noreg, !7, !DIExpression(), debug-location !9
+    CGHI renamable $r1d, 0, implicit-def $cc
+    DBG_VALUE $r1d, $noreg, !7, !DIExpression(), debug-location !9
+    BRC 14, 6, %bb.3, implicit $cc
+    J %bb.1
+
+...

test/CodeGen/SystemZ/debuginstr-01.mir

+# Check that the backend can produce a load-and-trap instruction also in
+# the presence of a DEBUG_VALUE machine instruction.
+#
+# RUN: llc %s -mtriple=s390x-linux-gnu -mcpu=zEC12 -start-before=postrapseudos \
+# RUN:   -o - 2>&1 | FileCheck %s
+#
+# CHECK: lat
+
+--- |
+  
+  define i32 @f1(i32* %ptr) #1 {
+  entry:
+    %val = load i32, i32* %ptr
+    ; Keep the dbg metadata live by referencing it in the IR.
+    call void @llvm.dbg.value(metadata i32 0, i64 0, metadata !7, metadata !DIExpression()), !dbg !9
+    %cmp = icmp eq i32 %val, 0
+    br i1 %cmp, label %if.then, label %if.end
+  
+  if.then:                                          ; preds = %entry
+    tail call void @llvm.trap()
+    unreachable
+  
+  if.end:                                           ; preds = %entry
+    ret i32 %val
+  }
+  
+  declare void @llvm.trap() #0
+  declare void @llvm.dbg.value(metadata, i64, metadata, metadata)
+  
+  declare void @llvm.stackprotector(i8*, i8**) #2
+
+  attributes #0 = { cold noreturn nounwind "target-cpu"="zEC12" }
+  attributes #1 = { "target-cpu"="zEC12" }
+  attributes #2 = { nounwind }
+
+  !llvm.dbg.cu = !{!0}
+  !llvm.module.flags = !{!3, !4}
+
+  !0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "llvm", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !2)
+  !1 = !DIFile(filename: "test.ll", directory: "/tmp")
+  !2 = !{}
+  !3 = !{i32 2, !"Dwarf Version", i32 4}
+  !4 = !{i32 2, !"Debug Info Version", i32 3}
+  !5 = distinct !DISubprogram(name: "test_dbg_value", scope: !1, file: !1, line: 1, type: !6, isLocal: false, isDefinition: true, scopeLine: 1, flags: DIFlagPrototyped, isOptimized: false, unit: !0, retainedNodes: !2)
+  !6 = !DISubroutineType(types: !2)
+  !7 = !DILocalVariable(name: "in", arg: 1, scope: !5, file: !1, line: 1, type: !8)
+  !8 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+  !9 = !DILocation(line: 1, column: 1, scope: !5)
+
+...
+---
+name:            f1
+alignment:       4
+tracksRegLiveness: true
+liveins:         
+  - { reg: '$r2d' }
+frameInfo:       
+  maxCallFrameSize: 0
+body:             |
+  bb.0.entry:
+    successors: %bb.1(0x00000001), %bb.2(0x7fffffff)
+    liveins: $r2d
+  
+    renamable $r2l = LMux killed renamable $r2d, 0, $noreg :: (load 4 from %ir.ptr)
+    DBG_VALUE $noreg, $r2l, !7, !DIExpression(), debug-location !9
+    CHIMux renamable $r2l, 0, implicit-def $cc
+    DBG_VALUE $noreg, $r2l, !7, !DIExpression(), debug-location !9
+    BRC 14, 6, %bb.2, implicit killed $cc
+  
+  bb.1.if.then:
+    successors: 
+  
+    Trap
+  
+  bb.2.if.end:
+    liveins: $r2l
+  
+    Return implicit $r2l
+
+...

test/CodeGen/SystemZ/debuginstr-02.mir

+# Check that the backend can handle consecutive select instructions also in
+# the presence of DEBUG_VALUE machine instructions.
+#
+# RUN: llc %s -mtriple=s390x-linux-gnu -mcpu=z13 -start-before=expand-isel-pseudos \
+# RUN:   -o - 2>&1 | FileCheck %s
+#
+# CHECK-LABEL: %bb.1:
+# CHECK:       ldr
+# CHECK-NEXT:  ldr
+# CHECK-NEXT:  ldr
+
+--- |
+  ; ModuleID = 'tc.ll'
+  source_filename = "tc.ll"
+  target datalayout = "E-m:e-i1:8:16-i8:8:16-i64:64-f128:64-v128:64-a:8:16-n32:64"
+  
+  define float @fun(i1 %cmp, float %arg0, float %arg1, float %arg2, float %arg3, float %arg4) #0 {
+    %fs0 = select i1 %cmp, float %arg0, float %arg1
+    ; Keep the dbg metadata live by referencing it in the IR.
+    call void @llvm.dbg.value(metadata i32 0, i64 0, metadata !7, metadata !DIExpression())
+    %fs1 = select i1 %cmp, float %arg2, float %arg3
+    call void @llvm.dbg.value(metadata i32 0, i64 0, metadata !7, metadata !DIExpression())
+    %fs2 = select i1 %cmp, float %arg0, float %arg3
+    %s0 = fadd float %fs0, %fs1
+    %s1 = fadd float %s0, %fs2
+    ret float %s1
+  }
+  
+  declare void @llvm.dbg.value(metadata, i64, metadata, metadata) #1
+  
+  attributes #0 = { "target-cpu"="z13" }
+
+  !llvm.dbg.cu = !{!0}
+  !llvm.module.flags = !{!3, !4}
+  
+  !0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "llvm", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !2)
+  !1 = !DIFile(filename: "test.ll", directory: "/tmp")
+  !2 = !{}
+  !3 = !{i32 2, !"Dwarf Version", i32 4}
+  !4 = !{i32 2, !"Debug Info Version", i32 3}
+  !5 = distinct !DISubprogram(name: "test_dbg_value", scope: !1, file: !1, line: 1, type: !6, isLocal: false, isDefinition: true, scopeLine: 1, flags: DIFlagPrototyped, isOptimized: false, unit: !0, retainedNodes: !2)
+  !6 = !DISubroutineType(types: !2)
+  !7 = !DILocalVariable(name: "in", arg: 1, scope: !5, file: !1, line: 1, type: !8)
+  !8 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+  !9 = !DILocation(line: 1, column: 1, scope: !5)
+
+...
+---
+name:            fun
+alignment:       4
+tracksRegLiveness: true
+registers:       
+  - { id: 0, class: gr32bit }
+  - { id: 1, class: fp32bit }
+  - { id: 2, class: fp32bit }
+  - { id: 3, class: fp32bit }
+  - { id: 4, class: fp32bit }
+  - { id: 5, class: fp32bit }
+  - { id: 6, class: fp32bit }
+  - { id: 7, class: fp32bit }
+  - { id: 8, class: fp32bit }
+  - { id: 9, class: fp32bit }
+liveins:         
+  - { reg: '$r2l', virtual-reg: '%0' }
+  - { reg: '$f0s', virtual-reg: '%1' }
+  - { reg: '$f2s', virtual-reg: '%2' }
+  - { reg: '$f4s', virtual-reg: '%3' }
+  - { reg: '$f6s', virtual-reg: '%4' }
+frameInfo:       
+  maxAlignment:    8
+fixedStack:      
+  - { id: 0, size: 4, alignment: 8, stack-id: 0, isImmutable: true }
+body:             |
+  bb.0 (%ir-block.0):
+    liveins: $r2l, $f0s, $f2s, $f4s, $f6s
+  
+    %4:fp32bit = COPY $f6s
+    %3:fp32bit = COPY $f4s
+    %2:fp32bit = COPY $f2s
+    %1:fp32bit = COPY $f0s
+    %0:gr32bit = COPY $r2l
+    TMLMux %0, 1, implicit-def $cc
+    %5:fp32bit = SelectVR32 %1, %2, 15, 7, implicit $cc
+    DBG_VALUE %5, $noreg, !7, !DIExpression(), debug-location !9
+    %6:fp32bit = SelectVR32 %3, %4, 15, 7, implicit $cc
+    DBG_VALUE %6, $noreg, !7, !DIExpression(), debug-location !9
+    %7:fp32bit = SelectVR32 %1, %4, 15, 7, implicit $cc
+    %8:fp32bit = AEBR %5, killed %6, implicit-def dead $cc
+    %9:fp32bit = AEBR %8, killed %7, implicit-def dead $cc
+    $f0s = COPY %9
+    Return implicit $f0s
+
+...