diff options
| -rw-r--r-- | llvm/lib/CodeGen/RegisterCoalescer.cpp | 178 | ||||
| -rw-r--r-- | llvm/test/DebugInfo/MIR/X86/regcoalescing-clears-dead-dbgvals.mir | 145 |
2 files changed, 321 insertions, 2 deletions
diff --git a/llvm/lib/CodeGen/RegisterCoalescer.cpp b/llvm/lib/CodeGen/RegisterCoalescer.cpp index ac4bf0ea8d0..59e2e85f29e 100644 --- a/llvm/lib/CodeGen/RegisterCoalescer.cpp +++ b/llvm/lib/CodeGen/RegisterCoalescer.cpp @@ -120,6 +120,8 @@ static cl::opt<unsigned> LargeIntervalFreqThreshold( namespace { + class JoinVals; + class RegisterCoalescer : public MachineFunctionPass, private LiveRangeEdit::Delegate { MachineFunction* MF = nullptr; @@ -131,6 +133,18 @@ namespace { AliasAnalysis *AA = nullptr; RegisterClassInfo RegClassInfo; + /// Debug variable location tracking -- for each VReg, maintain an + /// ordered-by-slot-index set of DBG_VALUEs, to help quick + /// identification of whether coalescing may change location validity. + using DbgValueLoc = std::pair<SlotIndex, MachineInstr*>; + DenseMap<unsigned, std::vector<DbgValueLoc>> DbgVRegToValues; + + /// VRegs may be repeatedly coalesced, and have many DBG_VALUEs attached. + /// To avoid repeatedly merging sets of DbgValueLocs, instead record + /// which vregs have been coalesced, and where to. This map is from + /// vreg => {set of vregs merged in}. + DenseMap<unsigned, SmallVector<unsigned, 4>> DbgMergedVRegNums; + /// A LaneMask to remember on which subregister live ranges we need to call /// shrinkToUses() later. LaneBitmask ShrinkMask; @@ -327,6 +341,19 @@ namespace { MI->eraseFromParent(); } + /// Walk over function and initialize the DbgVRegToValues map. + void buildVRegToDbgValueMap(MachineFunction &MF); + + /// Test whether, after merging, any DBG_VALUEs would refer to a + /// different value number than before merging, and whether this can + /// be resolved. If not, mark the DBG_VALUE as being undef. + void checkMergingChangesDbgValues(CoalescerPair &CP, LiveRange &LHS, + JoinVals &LHSVals, LiveRange &RHS, + JoinVals &RHSVals); + + void checkMergingChangesDbgValuesImpl(unsigned Reg, LiveRange &OtherRange, + LiveRange &RegRange, JoinVals &Vals2); + public: static char ID; ///< Class identification, replacement for typeinfo @@ -1650,8 +1677,7 @@ void RegisterCoalescer::addUndefFlag(const LiveInterval &Int, SlotIndex UseIdx, } } -void RegisterCoalescer::updateRegDefsUses(unsigned SrcReg, - unsigned DstReg, +void RegisterCoalescer::updateRegDefsUses(unsigned SrcReg, unsigned DstReg, unsigned SubIdx) { bool DstIsPhys = Register::isPhysicalRegister(DstReg); LiveInterval *DstInt = DstIsPhys ? nullptr : &LIS->getInterval(DstReg); @@ -2197,6 +2223,7 @@ class JoinVals { /// NewVNInfo. This is suitable for passing to LiveInterval::join(). SmallVector<int, 8> Assignments; + public: /// Conflict resolution for overlapping values. enum ConflictResolution { /// No overlap, simply keep this value. @@ -2225,6 +2252,7 @@ class JoinVals { CR_Impossible }; + private: /// Per-value info for LI. The lane bit masks are all relative to the final /// joined register, so they can be compared directly between SrcReg and /// DstReg. @@ -2385,6 +2413,11 @@ public: /// Get the value assignments suitable for passing to LiveInterval::join. const int *getAssignments() const { return Assignments.data(); } + + /// Get the conflict resolution for a value number. + ConflictResolution getResolution(unsigned Num) const { + return Vals[Num].Resolution; + } }; } // end anonymous namespace @@ -3387,6 +3420,9 @@ bool RegisterCoalescer::joinVirtRegs(CoalescerPair &CP) { while (!ShrinkRegs.empty()) shrinkToUses(&LIS->getInterval(ShrinkRegs.pop_back_val())); + // Scan and mark undef any DBG_VALUEs that would refer to a different value. + checkMergingChangesDbgValues(CP, LHS, LHSVals, RHS, RHSVals); + // Join RHS into LHS. LHS.join(RHS, LHSVals.getAssignments(), RHSVals.getAssignments(), NewVNInfo); @@ -3418,6 +3454,140 @@ bool RegisterCoalescer::joinIntervals(CoalescerPair &CP) { return CP.isPhys() ? joinReservedPhysReg(CP) : joinVirtRegs(CP); } +void RegisterCoalescer::buildVRegToDbgValueMap(MachineFunction &MF) +{ + const SlotIndexes &Slots = *LIS->getSlotIndexes(); + SmallVector<MachineInstr *, 8> ToInsert; + + // After collecting a block of DBG_VALUEs into ToInsert, enter them into the + // vreg => DbgValueLoc map. + auto CloseNewDVRange = [this, &ToInsert](SlotIndex Slot) { + for (auto *X : ToInsert) + DbgVRegToValues[X->getOperand(0).getReg()].push_back({Slot, X}); + + ToInsert.clear(); + }; + + // Iterate over all instructions, collecting them into the ToInsert vector. + // Once a non-debug instruction is found, record the slot index of the + // collected DBG_VALUEs. + for (auto &MBB : MF) { + SlotIndex CurrentSlot = Slots.getMBBStartIdx(&MBB); + + for (auto &MI : MBB) { + if (MI.isDebugValue() && MI.getOperand(0).isReg() && + MI.getOperand(0).getReg().isVirtual()) { + ToInsert.push_back(&MI); + } else if (!MI.isDebugInstr()) { + CurrentSlot = Slots.getInstructionIndex(MI); + CloseNewDVRange(CurrentSlot); + } + } + + // Close range of DBG_VALUEs at the end of blocks. + CloseNewDVRange(Slots.getMBBEndIdx(&MBB)); + } + + // Sort all DBG_VALUEs we've seen by slot number. + for (auto &Pair : DbgVRegToValues) + llvm::sort(Pair.second); +} + +void RegisterCoalescer::checkMergingChangesDbgValues(CoalescerPair &CP, + LiveRange &LHS, + JoinVals &LHSVals, + LiveRange &RHS, + JoinVals &RHSVals) { + auto ScanForDstReg = [&](unsigned Reg) { + checkMergingChangesDbgValuesImpl(Reg, RHS, LHS, LHSVals); + }; + + auto ScanForSrcReg = [&](unsigned Reg) { + checkMergingChangesDbgValuesImpl(Reg, LHS, RHS, RHSVals); + }; + + // Scan for potentially unsound DBG_VALUEs: examine first the register number + // Reg, and then any other vregs that may have been merged into it. + auto PerformScan = [this](unsigned Reg, std::function<void(unsigned)> Func) { + Func(Reg); + if (DbgMergedVRegNums.count(Reg)) + for (unsigned X : DbgMergedVRegNums[Reg]) + Func(X); + }; + + // Scan for unsound updates of both the source and destination register. + PerformScan(CP.getSrcReg(), ScanForSrcReg); + PerformScan(CP.getDstReg(), ScanForDstReg); +} + +void RegisterCoalescer::checkMergingChangesDbgValuesImpl(unsigned Reg, + LiveRange &OtherLR, + LiveRange &RegLR, + JoinVals &RegVals) { + // Are there any DBG_VALUEs to examine? + auto VRegMapIt = DbgVRegToValues.find(Reg); + if (VRegMapIt == DbgVRegToValues.end()) + return; + + auto &DbgValueSet = VRegMapIt->second; + auto DbgValueSetIt = DbgValueSet.begin(); + auto SegmentIt = OtherLR.begin(); + + bool LastUndefResult = false; + SlotIndex LastUndefIdx; + + // If the "Other" register is live at a slot Idx, test whether Reg can + // safely be merged with it, or should be marked undef. + auto ShouldUndef = [&RegVals, &RegLR, &LastUndefResult, + &LastUndefIdx](SlotIndex Idx) -> bool { + // Our worst-case performance typically happens with asan, causing very + // many DBG_VALUEs of the same location. Cache a copy of the most recent + // result for this edge-case. + if (LastUndefIdx == Idx) + return LastUndefResult; + + // If the other range was live, and Reg's was not, the register coalescer + // will not have tried to resolve any conflicts. We don't know whether + // the DBG_VALUE will refer to the same value number, so it must be made + // undef. + auto OtherIt = RegLR.find(Idx); + if (OtherIt == RegLR.end()) + return true; + + // Both the registers were live: examine the conflict resolution record for + // the value number Reg refers to. CR_Keep meant that this value number + // "won" and the merged register definitely refers to that value. CR_Erase + // means the value number was a redundant copy of the other value, which + // was coalesced and Reg deleted. It's safe to refer to the other register + // (which will be the source of the copy). + auto Resolution = RegVals.getResolution(OtherIt->valno->id); + LastUndefResult = Resolution != JoinVals::CR_Keep && + Resolution != JoinVals::CR_Erase; + LastUndefIdx = Idx; + return LastUndefResult; + }; + + // Iterate over both the live-range of the "Other" register, and the set of + // DBG_VALUEs for Reg at the same time. Advance whichever one has the lowest + // slot index. This relies on the DbgValueSet being ordered. + while (DbgValueSetIt != DbgValueSet.end() && SegmentIt != OtherLR.end()) { + if (DbgValueSetIt->first < SegmentIt->end) { + // "Other" is live and there is a DBG_VALUE of Reg: test if we should + // set it undef. + if (DbgValueSetIt->first >= SegmentIt->start && + DbgValueSetIt->second->getOperand(0).getReg() != 0 && + ShouldUndef(DbgValueSetIt->first)) { + // Mark undef, erase record of this DBG_VALUE to avoid revisiting. + DbgValueSetIt->second->getOperand(0).setReg(0); + continue; + } + ++DbgValueSetIt; + } else { + ++SegmentIt; + } + } +} + namespace { /// Information concerning MBB coalescing priority. @@ -3700,6 +3870,10 @@ bool RegisterCoalescer::runOnMachineFunction(MachineFunction &fn) { if (VerifyCoalescing) MF->verify(this, "Before register coalescing"); + DbgVRegToValues.clear(); + DbgMergedVRegNums.clear(); + buildVRegToDbgValueMap(fn); + RegClassInfo.runOnMachineFunction(fn); // Join (coalesce) intervals if requested. diff --git a/llvm/test/DebugInfo/MIR/X86/regcoalescing-clears-dead-dbgvals.mir b/llvm/test/DebugInfo/MIR/X86/regcoalescing-clears-dead-dbgvals.mir new file mode 100644 index 00000000000..feadfb26f41 --- /dev/null +++ b/llvm/test/DebugInfo/MIR/X86/regcoalescing-clears-dead-dbgvals.mir @@ -0,0 +1,145 @@ +# 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 + } + + ; 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. + ; RegisterCoalescer resolves %0 as CR_Erase: %0 is a redundant copy and + ; can be erased. + ; + ; 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) + %8:gr32 = XOR32rr %8, %4, implicit-def dead $eflags + DBG_VALUE %0, $noreg, !3, !DIExpression(), debug-location !5 + %0:gr32 = ADD32rr %0, killed %8, implicit-def dead $eflags + CMP32ri %0, 1000001, implicit-def $eflags + %7:gr32 = COPY %0 + JCC_1 %bb.1, 2, implicit killed $eflags + JMP_1 %bb.2 + + bb.2.leave: + $eax = COPY killed %7 + RET 0, killed $eax + +... + |
