[llvm] 6356598 - Reapply "[DebugInfo][InstrRef] Instrument x86 CMOV conversion to preserve variable values"

via llvm-commits llvm-commits at lists.llvm.org
Wed Jun 28 02:40:11 PDT 2023


Author: Jeremy Morse
Date: 2023-06-28T10:37:30+01:00
New Revision: 63565981a2726cc0405e143e7a91d4bcd021f731

URL: https://github.com/llvm/llvm-project/commit/63565981a2726cc0405e143e7a91d4bcd021f731
DIFF: https://github.com/llvm/llvm-project/commit/63565981a2726cc0405e143e7a91d4bcd021f731.diff

LOG: Reapply "[DebugInfo][InstrRef] Instrument x86 CMOV conversion to preserve variable values"

X86's CMOV conversion transforms CMOV instructions into control flow between
blocks, meaning the value is computed by a PHI rather than a "real" machine
instruction. In instruction-referencing mode, we need to transfer the
instruction label between the old CMOV and the new PHI instruction to mark
where the variable value is computed.

There's an extra complication in that memory operands can be unfolded from the
CMOV and sunk into the new blocks -- the test checks both scenarios where the
instruction number has to hop between instructions.

This omission exposed by Dexter testing.

Reviewed By: Orlando

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

Added: 
    llvm/test/DebugInfo/MIR/InstrRef/x86-cmov-converter.mir

Modified: 
    llvm/lib/Target/X86/X86CmovConversion.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Target/X86/X86CmovConversion.cpp b/llvm/lib/Target/X86/X86CmovConversion.cpp
index fae817f9485c4..8dc3b91f08e29 100644
--- a/llvm/lib/Target/X86/X86CmovConversion.cpp
+++ b/llvm/lib/Target/X86/X86CmovConversion.cpp
@@ -774,6 +774,8 @@ void X86CmovConverterPass::convertCmovInstsToBranches(
     const TargetRegisterClass *RC = MRI->getRegClass(MI.getOperand(0).getReg());
     Register TmpReg = MRI->createVirtualRegister(RC);
 
+    // Retain debug instr number when unfolded.
+    unsigned OldDebugInstrNum = MI.peekDebugInstrNum();
     SmallVector<MachineInstr *, 4> NewMIs;
     bool Unfolded = TII->unfoldMemoryOperand(*MBB->getParent(), MI, TmpReg,
                                              /*UnfoldLoad*/ true,
@@ -791,6 +793,9 @@ void X86CmovConverterPass::convertCmovInstsToBranches(
     if (&*MIItBegin == &MI)
       MIItBegin = MachineBasicBlock::iterator(NewCMOV);
 
+    if (OldDebugInstrNum)
+      NewCMOV->setDebugInstrNum(OldDebugInstrNum);
+
     // Sink whatever instructions were needed to produce the unfolded operand
     // into the false block.
     for (auto *NewMI : NewMIs) {
@@ -858,6 +863,11 @@ void X86CmovConverterPass::convertCmovInstsToBranches(
     LLVM_DEBUG(dbgs() << "\tFrom: "; MIIt->dump());
     LLVM_DEBUG(dbgs() << "\tTo: "; MIB->dump());
 
+    // debug-info: we can just copy the instr-ref number from one instruction
+    // to the other, seeing how it's a one-for-one substitution.
+    if (unsigned InstrNum = MIIt->peekDebugInstrNum())
+      MIB->setDebugInstrNum(InstrNum);
+
     // Add this PHI to the rewrite table.
     RegRewriteTable[DestReg] = std::make_pair(Op1Reg, Op2Reg);
   }

diff  --git a/llvm/test/DebugInfo/MIR/InstrRef/x86-cmov-converter.mir b/llvm/test/DebugInfo/MIR/InstrRef/x86-cmov-converter.mir
new file mode 100644
index 0000000000000..0749964292cd6
--- /dev/null
+++ b/llvm/test/DebugInfo/MIR/InstrRef/x86-cmov-converter.mir
@@ -0,0 +1,192 @@
+# RUN: llc %s -o - --run-pass=x86-cmov-conversion -mtriple=x86_64-- | FileCheck %s
+# REQUIRES: x86-registered-target
+#
+# Check that when we create new blocks and PHIs to represent CMOVs, that the
+# debug-instr-number is replaced on the new instruction, to preserve variable
+# locations. Check that this still works when unfolding memory operands, which
+# involves more decomposition of instructions.
+#
+# CHECK-LABEL: name: CmovInHotPath
+# CHECK-LABEL: bb.3.for.body:
+# CHECK: CMOV32rr {{.*}}, debug-instr-number 1
+#
+# CHECK-LABEL: name: test_cmov_memoperand_in_group_reuse_for_addr2
+# CHECK-LABEL: bb.2.entry:
+# CHECK-NEXT:  PHI {{.*}} debug-instr-number 1,
+# CHECK-NEXT:  PHI {{.*}} debug-instr-number 2,
+--- |
+  ; ModuleID = 'x86-cmov-converter.ll'
+  source_filename = "x86-cmov-converter.ll"
+  target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+
+  ; Function Attrs: nocallback nofree nosync nounwind speculatable willreturn memory(none)
+  declare void @llvm.dbg.value(metadata, metadata, metadata) #0
+
+  define void @CmovInHotPath(i32 %n, i32 %a, i32 %b, ptr nocapture %c, ptr nocapture readnone %d) !dbg !7 {
+  entry:
+    %cmp14 = icmp sgt i32 %n, 0, !dbg !12
+    br i1 %cmp14, label %for.body.preheader, label %for.cond.cleanup
+
+  for.body.preheader:                               ; preds = %entry
+    %wide.trip.count = zext i32 %n to i64, !dbg !13
+    br label %for.body
+
+  for.cond.cleanup:                                 ; preds = %for.body, %entry
+    ret void
+
+  for.body:                                         ; preds = %for.body, %for.body.preheader
+    %lsr.iv1 = phi ptr [ %uglygep, %for.body ], [ %c, %for.body.preheader ]
+    %lsr.iv = phi i64 [ %lsr.iv.next, %for.body ], [ %wide.trip.count, %for.body.preheader ]
+    %0 = load i32, ptr %lsr.iv1, align 4, !dbg !13
+    %add = add nsw i32 %0, 1, !dbg !13
+    %mul = mul nsw i32 %0, %a, !dbg !13
+    %cmp3 = icmp sgt i32 %mul, %b, !dbg !13
+    %. = select i1 %cmp3, i32 10, i32 %add, !dbg !13
+    call void @llvm.dbg.value(metadata i32 %., metadata !14, metadata !DIExpression()), !dbg !13
+    %mul7 = mul nsw i32 %., %add, !dbg !13
+    store i32 %mul7, ptr %lsr.iv1, align 4, !dbg !13
+    %lsr.iv.next = add nsw i64 %lsr.iv, -1, !dbg !13
+    %uglygep = getelementptr i8, ptr %lsr.iv1, i64 4, !dbg !13
+    %exitcond = icmp eq i64 %lsr.iv.next, 0, !dbg !13
+    br i1 %exitcond, label %for.cond.cleanup, label %for.body, !dbg !13
+  }
+
+  define i32 @test_cmov_memoperand_in_group_reuse_for_addr2(i32 %a, i32 %b, ptr %x, ptr %y) !dbg !15 {
+  entry:
+    %cond = icmp ugt i32 %a, %b, !dbg !16
+    %load1 = load ptr, ptr %y, align 8, !dbg !16
+    %p = select i1 %cond, ptr %x, ptr %load1, !dbg !16
+    call void @llvm.dbg.value(metadata ptr %p, metadata !17, metadata !DIExpression()), !dbg !16
+    %load2 = load i32, ptr %p, align 4, !dbg !16
+    %z = select i1 %cond, i32 %a, i32 %load2, !dbg !16
+    call void @llvm.dbg.value(metadata i32 %z, metadata !18, metadata !DIExpression()), !dbg !16
+    ret i32 %z, !dbg !16
+  }
+
+  attributes #0 = { nocallback nofree nosync nounwind speculatable willreturn memory(none) }
+
+  !llvm.dbg.cu = !{!0}
+  !llvm.module.flags = !{!3, !4, !5}
+  !llvm.ident = !{!6}
+
+  !0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, splitDebugInlining: false, nameTableKind: None)
+  !1 = !DIFile(filename: "test.c", directory: "/tmp/out.c")
+  !2 = !{}
+  !3 = !{i32 7, !"Dwarf Version", i32 4}
+  !4 = !{i32 2, !"Debug Info Version", i32 3}
+  !5 = !{i32 1, !"wchar_size", i32 4}
+  !6 = !{!""}
+  !7 = distinct !DISubprogram(name: "foo", scope: !1, file: !1, line: 3, type: !8, scopeLine: 3, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !2)
+  !8 = !DISubroutineType(types: !9)
+  !9 = !{!10, !11, !11}
+  !10 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+  !11 = !DIBasicType(name: "long int", size: 64, encoding: DW_ATE_signed)
+  !12 = !DILocation(line: 16, column: 3, scope: !7)
+  !13 = !DILocation(line: 15, column: 3, scope: !7)
+  !14 = !DILocalVariable(name: "bar", arg: 1, scope: !7, file: !1, line: 3, type: !11)
+  !15 = distinct !DISubprogram(name: "foo", scope: !1, file: !1, line: 3, type: !8, scopeLine: 3, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !2)
+  !16 = !DILocation(line: 16, column: 3, scope: !15)
+  !17 = !DILocalVariable(name: "bar", arg: 1, scope: !15, file: !1, line: 3, type: !11)
+  !18 = !DILocalVariable(name: "baz", arg: 2, scope: !15, file: !1, line: 3, type: !11)
+
+...
+---
+name:            CmovInHotPath
+tracksRegLiveness: true
+debugInstrRef:   true
+registers:
+  - { id: 0, class: gr64, preferred-register: '' }
+  - { id: 1, class: gr64, preferred-register: '' }
+  - { id: 2, class: gr64, preferred-register: '' }
+  - { id: 3, class: gr64, preferred-register: '' }
+  - { id: 4, class: gr64, preferred-register: '' }
+  - { id: 5, class: gr32, preferred-register: '' }
+  - { id: 6, class: gr32, preferred-register: '' }
+  - { id: 7, class: gr32, preferred-register: '' }
+liveins:
+  - { reg: '$edi', virtual-reg: '%5' }
+  - { reg: '$esi', virtual-reg: '%6' }
+  - { reg: '$edx', virtual-reg: '%7' }
+  - { reg: '$rcx', virtual-reg: '%8' }
+machineFunctionInfo: {}
+body:             |
+  bb.0.entry:
+    successors: %bb.1(0x50000000), %bb.2(0x30000000)
+    liveins: $edi, $esi, $edx, $rcx
+
+    %8:gr64 = COPY $rcx
+    %7:gr32 = COPY $edx
+    %6:gr32 = COPY $esi
+    %5:gr32 = COPY $edi
+    TEST32rr %5, %5, implicit-def $eflags, debug-location !12
+    JCC_1 %bb.2, 14, implicit $eflags
+    JMP_1 %bb.1
+
+  bb.1.for.body.preheader:
+    successors: %bb.3(0x80000000)
+
+    %10:gr32 = MOV32rr %5, debug-location !13
+    %0:gr64 = SUBREG_TO_REG 0, killed %10, %subreg.sub_32bit, debug-location !13
+    JMP_1 %bb.3
+
+  bb.2.for.cond.cleanup:
+    RET 0
+
+  bb.3.for.body:
+    successors: %bb.2(0x04000000), %bb.3(0x7c000000)
+
+    %1:gr64 = PHI %8, %bb.1, %4, %bb.3
+    %2:gr64 = PHI %0, %bb.1, %3, %bb.3
+    %11:gr32 = MOV32rm %1, 1, $noreg, 0, $noreg, debug-location !13 :: (load (s32) from %ir.lsr.iv1)
+    %12:gr32 = nsw INC32r %11, implicit-def dead $eflags, debug-location !13
+    %13:gr32 = nsw IMUL32rr %11, %6, implicit-def dead $eflags, debug-location !13
+    %14:gr32 = SUB32rr %13, %7, implicit-def $eflags, debug-location !13
+    %15:gr32 = MOV32ri 10
+    %16:gr32 = CMOV32rr %12, killed %15, 15, implicit $eflags, debug-instr-number 1, debug-location !13
+    DBG_INSTR_REF !14, !DIExpression(DW_OP_LLVM_arg, 0), dbg-instr-ref(1, 0), debug-location !13
+    %17:gr32 = nsw IMUL32rr %16, %12, implicit-def dead $eflags, debug-location !13
+    MOV32mr %1, 1, $noreg, 0, $noreg, killed %17, debug-location !13 :: (store (s32) into %ir.lsr.iv1)
+    %4:gr64 = ADD64ri8 %1, 4, implicit-def dead $eflags, debug-location !13
+    %3:gr64 = DEC64r %2, implicit-def $eflags, debug-location !13
+    JCC_1 %bb.2, 4, implicit $eflags, debug-location !13
+    JMP_1 %bb.3, debug-location !13
+
+...
+---
+name:            test_cmov_memoperand_in_group_reuse_for_addr2
+alignment:       16
+tracksRegLiveness: true
+debugInstrRef:   true
+registers:
+  - { id: 0, class: gr32, preferred-register: '' }
+  - { id: 1, class: gr32, preferred-register: '' }
+  - { id: 2, class: gr64, preferred-register: '' }
+  - { id: 3, class: gr64, preferred-register: '' }
+  - { id: 4, class: gr32, preferred-register: '' }
+  - { id: 5, class: gr64, preferred-register: '' }
+  - { id: 6, class: gr32, preferred-register: '' }
+liveins:
+  - { reg: '$edi', virtual-reg: '%0' }
+  - { reg: '$esi', virtual-reg: '%1' }
+  - { reg: '$rdx', virtual-reg: '%2' }
+  - { reg: '$rcx', virtual-reg: '%3' }
+debugValueSubstitutions: []
+constants:       []
+machineFunctionInfo: {}
+body:             |
+  bb.0.entry:
+    liveins: $edi, $esi, $rdx, $rcx
+
+    %3:gr64 = COPY $rcx
+    %2:gr64 = COPY $rdx
+    %1:gr32 = COPY $esi
+    %0:gr32 = COPY $edi
+    %4:gr32 = SUB32rr %0, %1, implicit-def $eflags, debug-location !16
+    %5:gr64 = CMOV64rm %2, %3, 1, $noreg, 0, $noreg, 6, implicit $eflags, debug-instr-number 1, debug-location !16 :: (load (s64) from %ir.y)
+    DBG_INSTR_REF !17, !DIExpression(DW_OP_LLVM_arg, 0), dbg-instr-ref(1, 0), debug-location !16
+    %6:gr32 = CMOV32rm %0, killed %5, 1, $noreg, 0, $noreg, 6, implicit $eflags, debug-instr-number 2, debug-location !16 :: (load (s32) from %ir.p)
+    DBG_INSTR_REF !18, !DIExpression(DW_OP_LLVM_arg, 0), dbg-instr-ref(2, 0), debug-location !16
+    $eax = COPY %6, debug-location !16
+    RET 0, $eax, debug-location !16
+
+...


        


More information about the llvm-commits mailing list