From a8db456b53a1783e2c8b3f3a6666dfa715a885d4 Mon Sep 17 00:00:00 2001 From: Jeremy Morse Date: Thu, 31 Oct 2019 12:34:17 +0000 Subject: [PATCH] Revert "[DebugInfo] MachineSink: Insert undef DBG_VALUEs when sinking instructions" This reverts commit ee50590e1684c197bc4336984795e48bf53c7a4e. PR43855 reports a performance regression from this commit, which I'll look into. --- llvm/lib/CodeGen/MachineSink.cpp | 53 +---------- llvm/test/DebugInfo/MIR/X86/postra-subreg-sink.mir | 3 - llvm/test/DebugInfo/MIR/X86/sink-leaves-undef.mir | 105 --------------------- 3 files changed, 3 insertions(+), 158 deletions(-) delete mode 100644 llvm/test/DebugInfo/MIR/X86/sink-leaves-undef.mir diff --git a/llvm/lib/CodeGen/MachineSink.cpp b/llvm/lib/CodeGen/MachineSink.cpp index 4c55158..27a2e70 100644 --- a/llvm/lib/CodeGen/MachineSink.cpp +++ b/llvm/lib/CodeGen/MachineSink.cpp @@ -736,9 +736,6 @@ static bool SinkingPreventsImplicitNullCheck(MachineInstr &MI, static void performSink(MachineInstr &MI, MachineBasicBlock &SuccToSinkTo, MachineBasicBlock::iterator InsertPos, SmallVectorImpl *DbgVals = nullptr) { - const MachineRegisterInfo &MRI = MI.getMF()->getRegInfo(); - const TargetInstrInfo &TII = *MI.getMF()->getSubtarget().getInstrInfo(); - // If debug values are provided use those, otherwise call collectDebugValues. SmallVector DbgValuesToSink; if (DbgVals) @@ -761,57 +758,13 @@ static void performSink(MachineInstr &MI, MachineBasicBlock &SuccToSinkTo, SuccToSinkTo.splice(InsertPos, ParentBlock, MI, ++MachineBasicBlock::iterator(MI)); - // Sink a copy of debug users to the insert position. Mark the original - // DBG_VALUE location as 'undef', indicating that any earlier variable - // location should be terminated as we've optimised away the value at this - // point. - // If the sunk instruction is a copy, try to forward the copy instead of - // leaving an 'undef' DBG_VALUE in the original location. Don't do this if - // there's any subregister weirdness involved. + // Move previously adjacent debug value instructions to the insert position. for (SmallVectorImpl::iterator DBI = DbgValuesToSink.begin(), DBE = DbgValuesToSink.end(); DBI != DBE; ++DBI) { MachineInstr *DbgMI = *DBI; - MachineInstr *NewDbgMI = DbgMI->getMF()->CloneMachineInstr(*DBI); - SuccToSinkTo.insert(InsertPos, NewDbgMI); - - // Copy DBG_VALUE operand and set the original to undef. We then check to - // see whether this is something that can be copy-forwarded. If it isn't, - // continue around the loop. - MachineOperand DbgMO = DbgMI->getOperand(0); - DbgMI->getOperand(0).setReg(0); - - const MachineOperand *SrcMO = nullptr, *DstMO = nullptr; - if (!TII.isCopyInstr(MI, SrcMO, DstMO)) - continue; - - // Check validity of forwarding this copy. - bool PostRA = MRI.getNumVirtRegs() == 0; - - // Trying to forward between physical and virtual registers is too hard. - if (DbgMO.getReg().isVirtual() != SrcMO->getReg().isVirtual()) - continue; - - // Only try virtual register copy-forwarding before regalloc, and physical - // register copy-forwarding after regalloc. - bool arePhysRegs = !DbgMO.getReg().isVirtual(); - if (arePhysRegs != PostRA) - continue; - - // Pre-regalloc, only forward if all subregisters agree (or there are no - // subregs at all). More analysis might recover some forwardable copies. - if (!PostRA && (DbgMO.getSubReg() != SrcMO->getSubReg() || - DbgMO.getSubReg() != DstMO->getSubReg())) - continue; - - // Post-regalloc, we may be sinking a DBG_VALUE of a sub or super-register - // of this copy. Only forward the copy if the DBG_VALUE operand exactly - // matches the copy destination. - if (PostRA && DbgMO.getReg() != DstMO->getReg()) - continue; - - DbgMI->getOperand(0).setReg(SrcMO->getReg()); - DbgMI->getOperand(0).setSubReg(SrcMO->getSubReg()); + SuccToSinkTo.splice(InsertPos, ParentBlock, DbgMI, + ++MachineBasicBlock::iterator(DbgMI)); } } diff --git a/llvm/test/DebugInfo/MIR/X86/postra-subreg-sink.mir b/llvm/test/DebugInfo/MIR/X86/postra-subreg-sink.mir index 8cca1de1..01e81ea 100644 --- a/llvm/test/DebugInfo/MIR/X86/postra-subreg-sink.mir +++ b/llvm/test/DebugInfo/MIR/X86/postra-subreg-sink.mir @@ -69,10 +69,7 @@ body: | ; CHECK: successors: %bb.1(0x80000000) ; CHECK: liveins: $edi ; CHECK: DBG_VALUE $edi, $noreg, ![[BARVAR]] - ; CHECK-NEXT: DBG_VALUE $edi, $noreg, ![[ARGVAR]] - ; CHECK-NEXT: DBG_VALUE $noreg, $noreg, ![[BAZVAR]] ; CHECK-NEXT: renamable $cl = MOV8ri 1 - ; CHECK-NEXT: DBG_VALUE $noreg, $noreg, ![[FOOVAR]] ; CHECK-NEXT: JMP_1 %bb.1 ; CHECK: bb.1.return: ; CHECK: liveins: $cl, $edi diff --git a/llvm/test/DebugInfo/MIR/X86/sink-leaves-undef.mir b/llvm/test/DebugInfo/MIR/X86/sink-leaves-undef.mir deleted file mode 100644 index 21668fc..0000000 --- a/llvm/test/DebugInfo/MIR/X86/sink-leaves-undef.mir +++ /dev/null @@ -1,105 +0,0 @@ -# RUN: llc %s -o - -run-pass=machine-sink -mtriple=x86_64-- | FileCheck %s -# This is a copy of test/CodeGen/X86/MachineSink-DbgValue.ll, where we -# additionally test that when the MOV32rm defining %0 is sunk, it leaves -# an 'undef' DBG_VALUE behind to terminate earlier location ranges. ---- | - target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128" - target triple = "x86_64-apple-macosx10.7.0" - - ; Function Attrs: nounwind readonly ssp uwtable - define i32 @foo(i32 %i, i32* nocapture %c) !dbg !4 { - call void @llvm.dbg.value(metadata i32 %i, metadata !9, metadata !DIExpression()), !dbg !14 - %ab = load i32, i32* %c, align 1, !dbg !15 - call void @llvm.dbg.value(metadata i32* %c, metadata !10, metadata !DIExpression()), !dbg !16 - call void @llvm.dbg.value(metadata i32 %ab, metadata !12, metadata !DIExpression()), !dbg !15 - %cd = icmp eq i32 %i, 42, !dbg !17 - br i1 %cd, label %bb1, label %bb2, !dbg !17 - - bb1: ; preds = %0 - %gh = add nsw i32 %ab, 2, !dbg !18 - br label %bb2, !dbg !18 - - bb2: ; preds = %bb1, %0 - %.0 = phi i32 [ %gh, %bb1 ], [ 0, %0 ] - ret i32 %.0, !dbg !19 - } - - ; Function Attrs: nounwind readnone speculatable - declare void @llvm.dbg.value(metadata, metadata, metadata) - - ; Function Attrs: nounwind - declare void @llvm.stackprotector(i8*, i8**) - - !llvm.dbg.cu = !{!0} - !llvm.module.flags = !{!3} - - !0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "Apple clang version 3.0 (tags/Apple/clang-211.10.1) (based on LLVM 3.0svn)", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, retainedTypes: !2) - !1 = !DIFile(filename: "a.c", directory: "/private/tmp") - !2 = !{} - !3 = !{i32 1, !"Debug Info Version", i32 3} - !4 = distinct !DISubprogram(name: "foo", scope: !1, file: !1, line: 2, type: !5, virtualIndex: 6, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !8) - !5 = !DISubroutineType(types: !6) - !6 = !{!7} - !7 = !DIBasicType(name: "int", size: 32, align: 32, encoding: DW_ATE_signed) - !8 = !{!9, !10, !12} - !9 = !DILocalVariable(name: "i", arg: 1, scope: !4, file: !1, line: 2, type: !7) - !10 = !DILocalVariable(name: "c", arg: 2, scope: !4, file: !1, line: 2, type: !11) - !11 = !DIDerivedType(tag: DW_TAG_pointer_type, scope: !0, baseType: !7, size: 64, align: 64) - !12 = !DILocalVariable(name: "a", scope: !13, file: !1, line: 3, type: !7) - !13 = distinct !DILexicalBlock(scope: !4, file: !1, line: 2, column: 25) - !14 = !DILocation(line: 2, column: 13, scope: !4) - !15 = !DILocation(line: 3, column: 14, scope: !13) - !16 = !DILocation(line: 2, column: 22, scope: !4) - !17 = !DILocation(line: 4, column: 3, scope: !13) - !18 = !DILocation(line: 5, column: 5, scope: !13) - !19 = !DILocation(line: 7, column: 1, scope: !13) - ; CHECK: ![[VARNUM:[0-9]+]] = !DILocalVariable(name: "a", -... ---- -name: foo -alignment: 4 -tracksRegLiveness: true -registers: - - { id: 0, class: gr32 } - - { id: 1, class: gr32 } - - { id: 2, class: gr32 } - - { id: 3, class: gr32 } - - { id: 4, class: gr64 } - - { id: 5, class: gr32 } - - { id: 6, class: gr32 } -liveins: - - { reg: '$edi', virtual-reg: '%3' } - - { reg: '$rsi', virtual-reg: '%4' } -body: | - bb.0 (%ir-block.0): - successors: %bb.1, %bb.2 - liveins: $edi, $rsi - ; CHECK-LABEL: bb.0 (%ir-block.0): - ; CHECK: DBG_VALUE $noreg, $noreg, ![[VARNUM]] - - DBG_VALUE $edi, $noreg, !9, !DIExpression(), debug-location !14 - DBG_VALUE $rsi, $noreg, !10, !DIExpression(), debug-location !16 - %4:gr64 = COPY $rsi - DBG_VALUE %4, $noreg, !10, !DIExpression(), debug-location !16 - %3:gr32 = COPY $edi - DBG_VALUE %3, $noreg, !9, !DIExpression(), debug-location !14 - %0:gr32 = MOV32rm %4, 1, $noreg, 0, $noreg, debug-location !15 :: (load 4 from %ir.c, align 1) - DBG_VALUE %0, $noreg, !12, !DIExpression(), debug-location !15 - %5:gr32 = MOV32r0 implicit-def dead $eflags - %6:gr32 = SUB32ri8 %3, 42, implicit-def $eflags, debug-location !17 - JCC_1 %bb.2, 5, implicit $eflags, debug-location !17 - JMP_1 %bb.1, debug-location !17 - - bb.1.bb1: - ; CHECK-LABEL: bb.1.bb1: - ; CHECK: %[[VREG:[0-9]+]]:gr32 = MOV32rm - ; CHECK-NEXT: DBG_VALUE %[[VREG]], $noreg, ![[VARNUM]] - ; CHECK-NEXT: ADD32ri8 - %1:gr32 = nsw ADD32ri8 %0, 2, implicit-def dead $eflags, debug-location !18 - - bb.2.bb2: - %2:gr32 = PHI %5, %bb.0, %1, %bb.1 - $eax = COPY %2, debug-location !19 - RET 0, $eax, debug-location !19 - -... -- 2.7.4