Revert r364515 and r364524
Jordan reports on llvm-commits a performance regression with r364515, backing the patch out while it's investigated. llvm-svn: 365448
This commit is contained in:
Jeremy Morse
parent
09329ce6c4
commit
9bebc65d79
@ -265,14 +265,6 @@ namespace {
|
||||
/// Return true if a copy involving a physreg should be joined.
|
||||
bool canJoinPhys(const CoalescerPair &CP);
|
||||
|
||||
/// When merging SrcReg and DstReg together, and the operand of the
|
||||
/// specified DBG_VALUE refers to one of them, would the def that a
|
||||
/// DBG_VALUE refers to change? This can happen when the DBG_VALUEs
|
||||
/// operand is dead and it's merged into a different live value,
|
||||
/// meaning the DBG_VALUE operands must be updated.
|
||||
bool mergingChangesDbgValue(MachineInstr *DbgV, unsigned SrcReg,
|
||||
unsigned DstReg) const;
|
||||
|
||||
/// Replace all defs and uses of SrcReg to DstReg and update the subregister
|
||||
/// number if it is not zero. If DstReg is a physical register and the
|
||||
/// existing subregister number of the def / use being updated is not zero,
|
||||
@ -1656,59 +1648,8 @@ void RegisterCoalescer::addUndefFlag(const LiveInterval &Int, SlotIndex UseIdx,
|
||||
}
|
||||
}
|
||||
|
||||
bool RegisterCoalescer::mergingChangesDbgValue(MachineInstr *DbgV,
|
||||
unsigned SrcReg,
|
||||
unsigned DstReg) const {
|
||||
assert(DbgV->isDebugValue());
|
||||
assert(DbgV->getParent() && "DbgValue with no parent");
|
||||
assert(DbgV->getOperand(0).isReg());
|
||||
unsigned DbgReg = DbgV->getOperand(0).getReg();
|
||||
|
||||
SlotIndex MIIdx = LIS->getSlotIndexes()->getIndexAfter(*DbgV);
|
||||
const LiveInterval &SrcLI = LIS->getInterval(SrcReg);
|
||||
|
||||
// Is the source register live across the DBG_VALUE?
|
||||
bool SrcLive = false;
|
||||
auto LII = SrcLI.find(MIIdx);
|
||||
if (LII != SrcLI.end() && LII->contains(MIIdx))
|
||||
SrcLive = true;
|
||||
|
||||
bool DstLive = false;
|
||||
// Destination register can be physical or virtual.
|
||||
if (TargetRegisterInfo::isVirtualRegister(DstReg)) {
|
||||
// Is DstReg live across the DBG_VALUE?
|
||||
const LiveInterval &DstLI = LIS->getInterval(DstReg);
|
||||
LII = DstLI.find(MIIdx);
|
||||
DstLive = (LII != DstLI.end() && LII->contains(MIIdx));
|
||||
} else if (MRI->isConstantPhysReg(DstReg)) {
|
||||
// Constant physical registers are always live.
|
||||
DstLive = true;
|
||||
} else {
|
||||
// For physical registers, see if any register unit containing DstReg
|
||||
// is live across the DBG_VALUE.
|
||||
for (MCRegUnitIterator UI(DstReg, TRI); UI.isValid(); ++UI) {
|
||||
const LiveRange &DstLI = LIS->getRegUnit(*UI);
|
||||
auto DstLII = DstLI.find(MIIdx);
|
||||
if (DstLII != DstLI.end() && DstLII->contains(MIIdx)) {
|
||||
DstLive = true;
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// We now know whether src and dst are live. Best case: we have a DBG_VALUE
|
||||
// of a live register, coalesing won't change its value.
|
||||
if ((DstLive && DbgReg == DstReg) || (SrcLive && DbgReg == SrcReg))
|
||||
return false;
|
||||
// If neither register are live, no damage done.
|
||||
if (!DstLive && !SrcLive)
|
||||
return false;
|
||||
// Otherwise, we will end up resurrecting the DBG_VALUE with a different
|
||||
// register, which is unsafe.
|
||||
return true;
|
||||
}
|
||||
|
||||
void RegisterCoalescer::updateRegDefsUses(unsigned SrcReg, unsigned DstReg,
|
||||
void RegisterCoalescer::updateRegDefsUses(unsigned SrcReg,
|
||||
unsigned DstReg,
|
||||
unsigned SubIdx) {
|
||||
bool DstIsPhys = TargetRegisterInfo::isPhysicalRegister(DstReg);
|
||||
LiveInterval *DstInt = DstIsPhys ? nullptr : &LIS->getInterval(DstReg);
|
||||
@ -1920,20 +1861,6 @@ bool RegisterCoalescer::joinCopy(MachineInstr *CopyMI, bool &Again) {
|
||||
ShrinkMask = LaneBitmask::getNone();
|
||||
ShrinkMainRange = false;
|
||||
|
||||
// Although we can update the DBG_VALUEs to the merged register, as debug uses
|
||||
// do not contribute to liveness it might not be a sound update. Collect
|
||||
// DBG_VALUEs that would change value were this interval merging to succeed.
|
||||
SmallVector<MachineInstr *, 4> DbgValuesToChange;
|
||||
auto CheckForDbgUser = [this, &CP, &DbgValuesToChange](MachineInstr &MI) {
|
||||
if (MI.isDebugValue() && MI.getOperand(0).isReg() &&
|
||||
mergingChangesDbgValue(&MI, CP.getSrcReg(), CP.getDstReg()))
|
||||
DbgValuesToChange.push_back(&MI);
|
||||
};
|
||||
for (auto &RegIt : MRI->reg_instructions(CP.getSrcReg()))
|
||||
CheckForDbgUser(RegIt);
|
||||
for (auto &RegIt : MRI->reg_instructions(CP.getDstReg()))
|
||||
CheckForDbgUser(RegIt);
|
||||
|
||||
// Okay, attempt to join these two intervals. On failure, this returns false.
|
||||
// Otherwise, if one of the intervals being joined is a physreg, this method
|
||||
// always canonicalizes DstInt to be it. The output "SrcInt" will not have
|
||||
@ -2002,16 +1929,6 @@ bool RegisterCoalescer::joinCopy(MachineInstr *CopyMI, bool &Again) {
|
||||
updateRegDefsUses(CP.getDstReg(), CP.getDstReg(), CP.getDstIdx());
|
||||
updateRegDefsUses(CP.getSrcReg(), CP.getDstReg(), CP.getSrcIdx());
|
||||
|
||||
// The updates to these DBG_VALUEs are not sound -- mark them undef.
|
||||
// FIXME: further analysis might recover them, this is the minimal sound
|
||||
// solution.
|
||||
for (MachineInstr *MI : DbgValuesToChange) {
|
||||
assert(MI->getOperand(0).isReg());
|
||||
LLVM_DEBUG(dbgs() << "Update of " << MI->getOperand(0) << " would be "
|
||||
<< "unsound, setting undef\n");
|
||||
MI->getOperand(0).setReg(0);
|
||||
}
|
||||
|
||||
// Shrink subregister ranges if necessary.
|
||||
if (ShrinkMask.any()) {
|
||||
LiveInterval &LI = LIS->getInterval(CP.getDstReg());
|
||||
|
||||
@ -1,243 +0,0 @@
|
||||
# RUN: llc -mtriple=x86_64-unknown-unknown %s -o - -run-pass=simple-register-coalescing | FileCheck %s
|
||||
# PR40010: DBG_VALUEs do not contribute to the liveness of virtual registers,
|
||||
# and the register coalescer would merge new live values on top of DBG_VALUEs,
|
||||
# leading to them presenting new (wrong) values to the debugger. Test that
|
||||
# when out of liveness, coalescing will mark DBG_VALUEs in non-live locations
|
||||
# as undef.
|
||||
--- |
|
||||
; ModuleID = './test.ll'
|
||||
source_filename = "./test.ll"
|
||||
target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
|
||||
|
||||
; Function Attrs: nounwind readnone speculatable
|
||||
declare void @llvm.dbg.value(metadata, metadata, metadata) #0
|
||||
|
||||
; Original IR source here:
|
||||
define i32 @test(i32* %pin) {
|
||||
entry:
|
||||
br label %start.test1
|
||||
|
||||
start.test1: ; preds = %start, %entry
|
||||
%foo = phi i32 [ 0, %entry ], [ %bar, %start.test1 ]
|
||||
%baz = load i32, i32* %pin, align 1
|
||||
%qux = xor i32 %baz, 1234
|
||||
%bar = add i32 %qux, %foo
|
||||
call void @llvm.dbg.value(metadata i32 %foo, metadata !3, metadata !DIExpression()), !dbg !5
|
||||
%cmp = icmp ugt i32 %bar, 1000000
|
||||
br i1 %cmp, label %leave, label %start.test1
|
||||
|
||||
leave: ; preds = %start
|
||||
ret i32 %bar
|
||||
}
|
||||
|
||||
; Stubs to appease the MIR parser
|
||||
define i32 @test2(i32* %pin) {
|
||||
entry:
|
||||
ret i32 0
|
||||
start.test2:
|
||||
ret i32 0
|
||||
leave:
|
||||
ret i32 0
|
||||
}
|
||||
|
||||
define i32 @test3(i32* %pin) {
|
||||
entry:
|
||||
ret i32 0
|
||||
start.test3:
|
||||
ret i32 0
|
||||
leave:
|
||||
ret i32 0
|
||||
}
|
||||
|
||||
define i32 @test4(i32* %pin) {
|
||||
entry:
|
||||
ret i32 0
|
||||
start.test4:
|
||||
ret i32 0
|
||||
leave:
|
||||
ret i32 0
|
||||
}
|
||||
|
||||
; Function Attrs: nounwind
|
||||
declare void @llvm.stackprotector(i8*, i8**) #1
|
||||
|
||||
attributes #0 = { nounwind readnone speculatable }
|
||||
attributes #1 = { nounwind }
|
||||
|
||||
!llvm.module.flags = !{!0}
|
||||
!llvm.dbg.cu = !{!1}
|
||||
|
||||
!0 = !{i32 2, !"Debug Info Version", i32 3}
|
||||
!1 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !2, producer: "beards", isOptimized: true, runtimeVersion: 4, emissionKind: FullDebug)
|
||||
!2 = !DIFile(filename: "bees.cpp", directory: "")
|
||||
!3 = !DILocalVariable(name: "bees", scope: !4)
|
||||
!4 = distinct !DISubprogram(name: "nope", scope: !1, file: !2, line: 1, spFlags: DISPFlagDefinition, unit: !1)
|
||||
!5 = !DILocation(line: 0, scope: !4)
|
||||
|
||||
...
|
||||
---
|
||||
name: test
|
||||
tracksRegLiveness: true
|
||||
body: |
|
||||
bb.0.entry:
|
||||
successors: %bb.1(0x80000000)
|
||||
liveins: $rdi
|
||||
|
||||
%2:gr64 = COPY killed $rdi
|
||||
%3:gr32 = MOV32r0 implicit-def dead $eflags
|
||||
%4:gr32 = MOV32ri 1234
|
||||
%7:gr32 = COPY killed %3
|
||||
|
||||
bb.1.start.test1:
|
||||
successors: %bb.2(0x04000000), %bb.1(0x7c000000)
|
||||
|
||||
; CHECK-LABEL: name: test
|
||||
;
|
||||
; We currently expect %1 and %0 to merge into %7
|
||||
;
|
||||
; CHECK: %[[REG1:[0-9]+]]:gr32 = MOV32rm
|
||||
; CHECK-NEXT: %[[REG2:[0-9]+]]:gr32 = XOR32rr %[[REG1]]
|
||||
; CHECK-NEXT: %[[REG3:[0-9]+]]:gr32 = ADD32rr %[[REG3]], %[[REG2]]
|
||||
; CHECK-NEXT: DBG_VALUE $noreg
|
||||
|
||||
%0:gr32 = COPY killed %7
|
||||
%8:gr32 = MOV32rm %2, 1, $noreg, 0, $noreg :: (load 4 from %ir.pin, align 1)
|
||||
%5:gr32 = COPY killed %8
|
||||
%5:gr32 = XOR32rr %5, %4, implicit-def dead $eflags
|
||||
%1:gr32 = COPY killed %0
|
||||
%1:gr32 = ADD32rr %1, killed %5, implicit-def dead $eflags
|
||||
DBG_VALUE %0, $noreg, !3, !DIExpression(), debug-location !5
|
||||
CMP32ri %1, 1000001, implicit-def $eflags
|
||||
%7:gr32 = COPY %1
|
||||
JCC_1 %bb.1, 2, implicit killed $eflags
|
||||
JMP_1 %bb.2
|
||||
|
||||
bb.2.leave:
|
||||
$eax = COPY killed %1
|
||||
RET 0, killed $eax
|
||||
|
||||
...
|
||||
---
|
||||
name: test2
|
||||
tracksRegLiveness: true
|
||||
body: |
|
||||
bb.0.entry:
|
||||
successors: %bb.1(0x80000000)
|
||||
liveins: $rdi
|
||||
|
||||
%2:gr64 = COPY killed $rdi
|
||||
%3:gr32 = MOV32r0 implicit-def dead $eflags
|
||||
%4:gr32 = MOV32ri 1234
|
||||
%7:gr32 = COPY killed %3
|
||||
|
||||
bb.1.start.test2:
|
||||
successors: %bb.2(0x04000000), %bb.1(0x7c000000)
|
||||
|
||||
; CHECK-LABEL: name: test2
|
||||
;
|
||||
; %0 should be merged into %7, but as %0 is live at this location the
|
||||
; DBG_VALUE should be preserved and point at the operand of ADD32rr.
|
||||
;
|
||||
; CHECK: %[[REG11:[0-9]+]]:gr32 = MOV32rm
|
||||
; CHECK-NEXT: %[[REG12:[0-9]+]]:gr32 = XOR32rr %[[REG11]]
|
||||
; CHECK-NEXT: DBG_VALUE %[[REG13:[0-9]+]]
|
||||
; CHECK-NEXT: %[[REG13]]:gr32 = ADD32rr %[[REG13]], %[[REG12]]
|
||||
|
||||
%0:gr32 = COPY killed %7
|
||||
%8:gr32 = MOV32rm %2, 1, $noreg, 0, $noreg :: (load 4 from %ir.pin, align 1)
|
||||
%5:gr32 = COPY killed %8
|
||||
%5:gr32 = XOR32rr %5, %4, implicit-def dead $eflags
|
||||
DBG_VALUE %0, $noreg, !3, !DIExpression(), debug-location !5
|
||||
%1:gr32 = COPY killed %0
|
||||
%1:gr32 = ADD32rr %1, killed %5, implicit-def dead $eflags
|
||||
CMP32ri %1, 1000001, implicit-def $eflags
|
||||
%7:gr32 = COPY %1
|
||||
JCC_1 %bb.1, 2, implicit killed $eflags
|
||||
JMP_1 %bb.2
|
||||
|
||||
bb.2.leave:
|
||||
$eax = COPY killed %1
|
||||
RET 0, killed $eax
|
||||
|
||||
...
|
||||
---
|
||||
name: test3
|
||||
tracksRegLiveness: true
|
||||
body: |
|
||||
bb.0.entry:
|
||||
successors: %bb.1(0x80000000)
|
||||
liveins: $rdi
|
||||
|
||||
%2:gr64 = COPY killed $rdi
|
||||
%3:gr32 = MOV32r0 implicit-def dead $eflags
|
||||
%4:gr32 = MOV32ri 1234
|
||||
%7:gr32 = COPY killed %3
|
||||
|
||||
bb.1.start.test3:
|
||||
successors: %bb.2(0x04000000), %bb.1(0x7c000000)
|
||||
|
||||
; CHECK-LABEL: name: test3
|
||||
;
|
||||
; This is a use-before-def, merging new registers into %0 could unsoundly
|
||||
; make it live again, on merge mark it undef.
|
||||
;
|
||||
; CHECK: DBG_VALUE $noreg
|
||||
|
||||
DBG_VALUE %0, $noreg, !3, !DIExpression(), debug-location !5
|
||||
%0:gr32 = COPY killed %7
|
||||
%8:gr32 = MOV32rm %2, 1, $noreg, 0, $noreg :: (load 4 from %ir.pin, align 1)
|
||||
%5:gr32 = COPY killed %8
|
||||
%5:gr32 = XOR32rr %5, %4, implicit-def dead $eflags
|
||||
%1:gr32 = COPY killed %0
|
||||
%1:gr32 = ADD32rr %1, killed %5, implicit-def dead $eflags
|
||||
CMP32ri %1, 1000001, implicit-def $eflags
|
||||
%7:gr32 = COPY %1
|
||||
JCC_1 %bb.1, 2, implicit killed $eflags
|
||||
JMP_1 %bb.2
|
||||
|
||||
bb.2.leave:
|
||||
$eax = COPY killed %1
|
||||
RET 0, killed $eax
|
||||
|
||||
...
|
||||
---
|
||||
name: test4
|
||||
tracksRegLiveness: true
|
||||
body: |
|
||||
bb.0.entry:
|
||||
successors: %bb.1(0x80000000)
|
||||
liveins: $rdi
|
||||
|
||||
%2:gr64 = COPY killed $rdi
|
||||
%3:gr32 = MOV32r0 implicit-def dead $eflags
|
||||
%4:gr32 = MOV32ri 1234
|
||||
%7:gr32 = COPY killed %3
|
||||
|
||||
bb.1.start.test4:
|
||||
successors: %bb.2(0x04000000), %bb.1(0x7c000000)
|
||||
|
||||
; CHECK-LABEL: name: test4
|
||||
;
|
||||
; Using a dead register, even if we coalesce it to the right value, should
|
||||
; be marked undef. The coalescer can't prove it's correct without
|
||||
; considering control flow in the general case.
|
||||
;
|
||||
; CHECK: DBG_VALUE $noreg
|
||||
|
||||
%0:gr32 = COPY killed %7
|
||||
DBG_VALUE %7, $noreg, !3, !DIExpression(), debug-location !5
|
||||
%8:gr32 = MOV32rm %2, 1, $noreg, 0, $noreg :: (load 4 from %ir.pin, align 1)
|
||||
%5:gr32 = COPY killed %8
|
||||
%5:gr32 = XOR32rr %5, %4, implicit-def dead $eflags
|
||||
%1:gr32 = COPY killed %0
|
||||
%1:gr32 = ADD32rr %1, killed %5, implicit-def dead $eflags
|
||||
CMP32ri %1, 1000001, implicit-def $eflags
|
||||
%7:gr32 = COPY %1
|
||||
JCC_1 %bb.1, 2, implicit killed $eflags
|
||||
JMP_1 %bb.2
|
||||
|
||||
bb.2.leave:
|
||||
$eax = COPY killed %1
|
||||
RET 0, killed $eax
|
||||
|
||||
...
|
||||
Loading…
x
Reference in New Issue
Block a user