1
0
mirror of https://github.com/RPCS3/llvm-mirror.git synced 2024-11-23 11:13:28 +01:00

[DebugInfo] Re-apply two patches to MachineSink

These were:
 * D58386 / f5e1b718a67 / reverted in d382a8a768b
 * D58238 / ee50590e168 / reverted in a8db456b53a

Of which the latter has a performance regression tracked in PR43855,
fixed by D70672 / D70676, which will be committed atomically with this
reapplication.

Contains a minor difference to account for a change in the IsCopyInstr
signature.
This commit is contained in:
Jeremy Morse 2019-12-05 14:46:11 +00:00 committed by Jeremy Morse
parent 902881f779
commit 4ecf048a1c
4 changed files with 351 additions and 17 deletions

View File

@ -106,6 +106,9 @@ namespace {
using AllSuccsCache =
std::map<MachineBasicBlock *, SmallVector<MachineBasicBlock *, 4>>;
// Remember debug uses of vregs seen, so we know what to sink out of blocks.
DenseMap<unsigned, TinyPtrVector<MachineInstr *>> SeenDbgUsers;
public:
static char ID; // Pass identification
@ -133,6 +136,7 @@ namespace {
private:
bool ProcessBlock(MachineBasicBlock &MBB);
void ProcessDbgInst(MachineInstr &MI);
bool isWorthBreakingCriticalEdge(MachineInstr &MI,
MachineBasicBlock *From,
MachineBasicBlock *To);
@ -154,8 +158,14 @@ namespace {
MachineBasicBlock *To,
bool BreakPHIEdge);
bool SinkInstruction(MachineInstr &MI, bool &SawStore,
AllSuccsCache &AllSuccessors);
/// If we sink a COPY inst, some debug users of it's destination may no
/// longer be dominated by the COPY, and will eventually be dropped.
/// This is easily rectified by forwarding the non-dominated debug uses
/// to the copy source.
void SalvageUnsunkDebugUsersOfCopy(MachineInstr &,
MachineBasicBlock *TargetBlock);
bool AllUsesDominatedByBlock(unsigned Reg, MachineBasicBlock *MBB,
MachineBasicBlock *DefMBB,
bool &BreakPHIEdge, bool &LocalUse) const;
@ -368,8 +378,11 @@ bool MachineSinking::ProcessBlock(MachineBasicBlock &MBB) {
if (!ProcessedBegin)
--I;
if (MI.isDebugInstr())
if (MI.isDebugInstr()) {
if (MI.isDebugValue())
ProcessDbgInst(MI);
continue;
}
bool Joined = PerformTrivialForwardCoalescing(MI, &MBB);
if (Joined) {
@ -385,9 +398,23 @@ bool MachineSinking::ProcessBlock(MachineBasicBlock &MBB) {
// If we just processed the first instruction in the block, we're done.
} while (!ProcessedBegin);
SeenDbgUsers.clear();
return MadeChange;
}
void MachineSinking::ProcessDbgInst(MachineInstr &MI) {
// When we see DBG_VALUEs for registers, record any vreg it reads, so that
// we know what to sink if the vreg def sinks.
assert(MI.isDebugValue() && "Expected DBG_VALUE for processing");
MachineOperand &MO = MI.getOperand(0);
if (!MO.isReg() || !MO.getReg().isVirtual())
return;
SeenDbgUsers[MO.getReg()].push_back(&MI);
}
bool MachineSinking::isWorthBreakingCriticalEdge(MachineInstr &MI,
MachineBasicBlock *From,
MachineBasicBlock *To) {
@ -732,18 +759,12 @@ static bool SinkingPreventsImplicitNullCheck(MachineInstr &MI,
MBP.LHS.getReg() == BaseOp->getReg();
}
/// Sink an instruction and its associated debug instructions. If the debug
/// instructions to be sunk are already known, they can be provided in DbgVals.
/// Sink an instruction and its associated debug instructions.
static void performSink(MachineInstr &MI, MachineBasicBlock &SuccToSinkTo,
MachineBasicBlock::iterator InsertPos,
SmallVectorImpl<MachineInstr *> *DbgVals = nullptr) {
// If debug values are provided use those, otherwise call collectDebugValues.
SmallVector<MachineInstr *, 2> DbgValuesToSink;
if (DbgVals)
DbgValuesToSink.insert(DbgValuesToSink.begin(),
DbgVals->begin(), DbgVals->end());
else
MI.collectDebugValues(DbgValuesToSink);
SmallVectorImpl<MachineInstr *> &DbgValuesToSink) {
const MachineRegisterInfo &MRI = MI.getMF()->getRegInfo();
const TargetInstrInfo &TII = *MI.getMF()->getSubtarget().getInstrInfo();
// If we cannot find a location to use (merge with), then we erase the debug
// location to prevent debug-info driven tools from potentially reporting
@ -759,13 +780,60 @@ static void performSink(MachineInstr &MI, MachineBasicBlock &SuccToSinkTo,
SuccToSinkTo.splice(InsertPos, ParentBlock, MI,
++MachineBasicBlock::iterator(MI));
// Move previously adjacent debug value instructions to the insert position.
// 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.
for (SmallVectorImpl<MachineInstr *>::iterator DBI = DbgValuesToSink.begin(),
DBE = DbgValuesToSink.end();
DBI != DBE; ++DBI) {
MachineInstr *DbgMI = *DBI;
SuccToSinkTo.splice(InsertPos, ParentBlock, DbgMI,
++MachineBasicBlock::iterator(DbgMI));
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;
auto CopyOperands = TII.isCopyInstr(MI);
if (!CopyOperands)
continue;
SrcMO = CopyOperands->Source;
DstMO = CopyOperands->Destination;
// 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());
}
}
@ -883,7 +951,25 @@ bool MachineSinking::SinkInstruction(MachineInstr &MI, bool &SawStore,
while (InsertPos != SuccToSinkTo->end() && InsertPos->isPHI())
++InsertPos;
performSink(MI, *SuccToSinkTo, InsertPos);
// Collect debug users of any vreg that this inst defines.
SmallVector<MachineInstr *, 4> DbgUsersToSink;
for (auto &MO : MI.operands()) {
if (!MO.isReg() || !MO.isDef() || !MO.getReg().isVirtual())
continue;
if (!SeenDbgUsers.count(MO.getReg()))
continue;
auto &Users = SeenDbgUsers[MO.getReg()];
DbgUsersToSink.insert(DbgUsersToSink.end(), Users.begin(), Users.end());
}
// After sinking, some debug users may not be dominated any more. If possible,
// copy-propagate their operands. As it's expensive, don't do this if there's
// no debuginfo in the program.
if (MI.getMF()->getFunction().getSubprogram() && MI.isCopy())
SalvageUnsunkDebugUsersOfCopy(MI, SuccToSinkTo);
performSink(MI, *SuccToSinkTo, InsertPos, DbgUsersToSink);
// Conservatively, clear any kill flags, since it's possible that they are no
// longer correct.
@ -898,6 +984,41 @@ bool MachineSinking::SinkInstruction(MachineInstr &MI, bool &SawStore,
return true;
}
void MachineSinking::SalvageUnsunkDebugUsersOfCopy(
MachineInstr &MI, MachineBasicBlock *TargetBlock) {
assert(MI.isCopy());
assert(MI.getOperand(1).isReg());
// Enumerate all users of vreg operands that are def'd. Skip those that will
// be sunk. For the rest, if they are not dominated by the block we will sink
// MI into, propagate the copy source to them.
SmallVector<MachineInstr *, 4> DbgDefUsers;
const MachineRegisterInfo &MRI = MI.getMF()->getRegInfo();
for (auto &MO : MI.operands()) {
if (!MO.isReg() || !MO.isDef() || !MO.getReg().isVirtual())
continue;
for (auto &User : MRI.use_instructions(MO.getReg())) {
if (!User.isDebugValue() || DT->dominates(TargetBlock, User.getParent()))
continue;
// If is in same block, will either sink or be use-before-def.
if (User.getParent() == MI.getParent())
continue;
assert(User.getOperand(0).isReg() &&
"DBG_VALUE user of vreg, but non reg operand?");
DbgDefUsers.push_back(&User);
}
}
// Point the users of this copy that are no longer dominated, at the source
// of the copy.
for (auto *User : DbgDefUsers) {
User->getOperand(0).setReg(MI.getOperand(1).getReg());
User->getOperand(0).setSubReg(MI.getOperand(1).getSubReg());
}
}
//===----------------------------------------------------------------------===//
// This pass is not intended to be a replacement or a complete alternative
// for the pre-ra machine sink pass. It is only designed to sink COPY
@ -1207,7 +1328,7 @@ bool PostRAMachineSinking::tryToSinkCopy(MachineBasicBlock &CurBB,
// block.
clearKillFlags(MI, CurBB, UsedOpsInCopy, UsedRegUnits, TRI);
MachineBasicBlock::iterator InsertPos = SuccBB->getFirstNonPHI();
performSink(*MI, *SuccBB, InsertPos, &DbgValsToSink);
performSink(*MI, *SuccBB, InsertPos, DbgValsToSink);
updateLiveIn(MI, SuccBB, UsedOpsInCopy, DefedRegsInCopy);
Changed = true;

View File

@ -0,0 +1,105 @@
# RUN: llc -mtriple=x86_64-unknown-unknown -run-pass=machine-sink -o - %s | FileCheck %s
# Check various things when we sink machine instructions:
# a) DBG_VALUEs should sink with defs
# b) Undefs should be left behind
# c) DBG_VALUEs not immediately following the defining inst should sink too
# d) If we generate debug-use-before-defs through sinking, and can copy
# propagate to a different value, we should do that.
--- |
target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"
@x = common local_unnamed_addr global i32 0, align 4, !dbg !0
; Function Attrs: noreturn nounwind uwtable
define void @Process(i32* nocapture readonly %p) local_unnamed_addr !dbg !9 {
; Stripped
entry:
br label %nou
nou:
br label %exit
exit:
ret void
}
; Function Attrs: nounwind readnone
declare void @llvm.dbg.value(metadata, i64, metadata, metadata)
!llvm.dbg.cu = !{!1}
!llvm.module.flags = !{!6, !7}
!llvm.ident = !{!8}
!0 = !DIGlobalVariableExpression(var: !DIGlobalVariable(name: "x", scope: !1, file: !2, line: 1, type: !5, isLocal: false, isDefinition: true), expr: !DIExpression())
!1 = distinct !DICompileUnit(language: DW_LANG_C99, file: !2, producer: "clang", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !3, globals: !4)
!2 = !DIFile(filename: "t.c", directory: "")
!3 = !{}
!4 = !{!0}
!5 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
!6 = !{i32 2, !"Dwarf Version", i32 4}
!7 = !{i32 2, !"Debug Info Version", i32 3}
!8 = !{!"clang version 4.0.0 "}
!9 = distinct !DISubprogram(name: "Process", scope: !2, file: !2, line: 2, type: !10, isLocal: false, isDefinition: true, scopeLine: 3, flags: DIFlagPrototyped, isOptimized: true, unit: !1, retainedNodes: !15)
!10 = !DISubroutineType(types: !11)
!11 = !{null, !12}
!12 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !13, size: 64)
!13 = !DIDerivedType(tag: DW_TAG_const_type, baseType: !14)
!14 = !DIBasicType(name: "unsigned int", size: 32, encoding: DW_ATE_unsigned)
!15 = !{!16}
!16 = !DILocalVariable(name: "p", arg: 1, scope: !9, file: !2, line: 2, type: !12)
!17 = !DIExpression()
!18 = !DILocation(line: 2, column: 34, scope: !9)
!28 = !DILexicalBlockFile(scope: !9, file: !2, discriminator: 1)
; CHECK: [[VARNUM:![0-9]+]] = !DILocalVariable(name: "p",
...
---
name: Process
tracksRegLiveness: true
liveins:
- { reg: '$rdi', virtual-reg: '%2' }
- { reg: '$rsi', virtual-reg: '%2' }
body: |
bb.0.entry:
successors: %bb.1.nou(0x80000000), %bb.2.exit
liveins: $rdi, $esi
; This block should have the vreg copy sunk from it, the DBG_VALUE with it,
; and a copy-prop'd DBG_VALUE left over.
; CHECK-LABEL: bb.0.entry:
; CHECK: [[ARG0VREG:%[0-9]+]]:gr64 = COPY $rdi
; CHECK-NEXT: CMP32ri $esi, 0
; CHECK-NEXT: DBG_VALUE [[ARG0VREG]], $noreg, [[VARNUM]]
; CHECK-NEXT: JCC_1 %bb.1, 4
; CHECK-NEXT: JMP_1
%2:gr64 = COPY $rdi
%5:gr64 = COPY %2
CMP32ri $esi, 0, implicit-def $eflags
DBG_VALUE %5, $noreg, !16, !17, debug-location !18
JCC_1 %bb.1.nou, 4, implicit $eflags
JMP_1 %bb.2.exit
bb.1.nou:
successors: %bb.2.exit(0x80000000)
; This block should receive the sunk copy and DBG_VALUE
; CHECK-LABEL: bb.1.nou:
; CHECK: [[SUNKVREG:%[0-9]+]]:gr64 = COPY [[ARG0VREG]]
; CHECK-NEXT: DBG_VALUE [[SUNKVREG]], $noreg, [[VARNUM]]
; CHECK-NEXT: ADD64ri8
; CHECK-NEXT: JMP_1
%1:gr64 = ADD64ri8 %5, 4, implicit-def dead $eflags
JMP_1 %bb.2.exit
bb.2.exit:
; The DBG_VALUE below should have its operand copy-propagated after
; the copy to %5 is sunk.
; CHECK-LABEL: bb.2.exit:
; CHECK: DBG_VALUE [[ARG0VREG]], $noreg, [[VARNUM]]
; CHECK-NEXT: $rax = MOV64rr [[ARG0VREG]]
; CHECK-NEXT: RET 0
DBG_VALUE %5, _, !16, !17, debug-location !18
$rax = MOV64rr %2
RET 0
...

View File

@ -69,7 +69,10 @@ 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

View File

@ -0,0 +1,105 @@
# 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
...