Skip to content

[RegisterCoalescer] Clear instructions not recorded in ErasedInstrs but erased #79820

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Feb 9, 2024
Merged

[RegisterCoalescer] Clear instructions not recorded in ErasedInstrs but erased #79820

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
646c95f
Pre-commit test cases
dianqk Feb 8, 2024
00254d3
[RegisterCoalescer] Deferring deletion of instructions in ErasedInstr…
dianqk Feb 8, 2024
4e0d1c2
fixup! Pre-commit test cases
dianqk Feb 8, 2024
0821515
[RegisterCoalescer] Clear instructions not recorded in `ErasedInstrs`…
dianqk Feb 9, 2024
a27d5b3
fixup! [RegisterCoalescer] Clear instructions not recorded in `Erased…
dianqk Feb 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 11 additions & 5 deletions llvm/lib/CodeGen/RegisterCoalescer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,8 @@ namespace {
/// was successfully coalesced away. If it is not currently possible to
/// coalesce this interval, but it may be possible if other things get
/// coalesced, then it returns true by reference in 'Again'.
bool joinCopy(MachineInstr *CopyMI, bool &Again);
bool joinCopy(MachineInstr *CopyMI, bool &Again,
SmallPtrSetImpl<MachineInstr *> &CurrentErasedInstrs);

/// Attempt to join these two intervals. On failure, this
/// returns false. The output "SrcInt" will not have been modified, so we
Expand Down Expand Up @@ -1964,7 +1965,9 @@ void RegisterCoalescer::setUndefOnPrunedSubRegUses(LiveInterval &LI,
LIS->shrinkToUses(&LI);
}

bool RegisterCoalescer::joinCopy(MachineInstr *CopyMI, bool &Again) {
bool RegisterCoalescer::joinCopy(
MachineInstr *CopyMI, bool &Again,
SmallPtrSetImpl<MachineInstr *> &CurrentErasedInstrs) {
Again = false;
LLVM_DEBUG(dbgs() << LIS->getInstructionIndex(*CopyMI) << '\t' << *CopyMI);

Expand Down Expand Up @@ -2156,7 +2159,9 @@ bool RegisterCoalescer::joinCopy(MachineInstr *CopyMI, bool &Again) {
// CopyMI has been erased by joinIntervals at this point. Remove it from
// ErasedInstrs since copyCoalesceWorkList() won't add a successful join back
// to the work list. This keeps ErasedInstrs from growing needlessly.
ErasedInstrs.erase(CopyMI);
if (ErasedInstrs.erase(CopyMI))
// But we may encounter the instruction again in this iteration.
CurrentErasedInstrs.insert(CopyMI);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do we end up with the same copy instruction twice in the worklist?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From the debugging results, it looks like removePartialRedundancy changes the contents of the work list.


// Rewrite all SrcReg operands to DstReg.
// Also update DstReg operands to include DstIdx if it is set.
Expand Down Expand Up @@ -3982,17 +3987,18 @@ void RegisterCoalescer::lateLiveIntervalUpdate() {
bool RegisterCoalescer::
copyCoalesceWorkList(MutableArrayRef<MachineInstr*> CurrList) {
bool Progress = false;
SmallPtrSet<MachineInstr *, 4> CurrentErasedInstrs;
for (MachineInstr *&MI : CurrList) {
if (!MI)
continue;
// Skip instruction pointers that have already been erased, for example by
// dead code elimination.
if (ErasedInstrs.count(MI)) {
if (ErasedInstrs.count(MI) || CurrentErasedInstrs.count(MI)) {
MI = nullptr;
continue;
}
bool Again = false;
bool Success = joinCopy(MI, Again);
bool Success = joinCopy(MI, Again, CurrentErasedInstrs);
Progress |= Success;
if (Success || !Again)
MI = nullptr;
Expand Down
213 changes: 213 additions & 0 deletions llvm/test/CodeGen/LoongArch/register-coalescer-crash-pr79718.mir
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,213 @@
# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 4
# RUN: llc -o - %s -mtriple=loongarch64 \
# RUN: -run-pass=register-coalescer -join-liveintervals=1 -join-splitedges=0 | FileCheck %s

---
name: foo
tracksRegLiveness: true
body: |
; CHECK-LABEL: name: foo
; CHECK: bb.0:
; CHECK-NEXT: successors: %bb.1(0x80000000)
; CHECK-NEXT: liveins: $r4, $r5, $r6, $r7, $r8
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: [[COPY:%[0-9]+]]:gpr = COPY $r8
; CHECK-NEXT: [[COPY1:%[0-9]+]]:gpr = COPY $r7
; CHECK-NEXT: [[COPY2:%[0-9]+]]:gpr = COPY $r6
; CHECK-NEXT: [[COPY3:%[0-9]+]]:gpr = COPY $r5
; CHECK-NEXT: [[COPY4:%[0-9]+]]:gpr = COPY $r4
; CHECK-NEXT: [[ANDI:%[0-9]+]]:gpr = ANDI [[COPY3]], 1
; CHECK-NEXT: [[ORI:%[0-9]+]]:gpr = ORI $r0, 1
; CHECK-NEXT: [[ANDI1:%[0-9]+]]:gpr = ANDI [[COPY2]], 1
; CHECK-NEXT: [[ANDI2:%[0-9]+]]:gpr = ANDI [[COPY1]], 1
; CHECK-NEXT: [[ANDI3:%[0-9]+]]:gpr = ANDI [[COPY]], 1
; CHECK-NEXT: [[COPY5:%[0-9]+]]:gpr = COPY $r0
; CHECK-NEXT: [[COPY6:%[0-9]+]]:gpr = COPY $r0
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: bb.1:
; CHECK-NEXT: successors: %bb.2(0x80000000)
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: [[COPY7:%[0-9]+]]:gpr = COPY [[COPY5]]
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: bb.2:
; CHECK-NEXT: successors: %bb.3(0x40000000), %bb.4(0x40000000)
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: BEQZ [[ANDI]], %bb.4
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: bb.3:
; CHECK-NEXT: successors: %bb.9(0x80000000)
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: PseudoBR %bb.9
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: bb.4:
; CHECK-NEXT: successors: %bb.5(0x80000000)
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: bb.5:
; CHECK-NEXT: successors: %bb.7(0x7c000000), %bb.6(0x04000000)
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: dead [[LD_D:%[0-9]+]]:gpr = LD_D $r0, 8
; CHECK-NEXT: dead [[LD_D1:%[0-9]+]]:gpr = LD_D $r0, 0
; CHECK-NEXT: BNEZ [[ANDI1]], %bb.7
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: bb.6:
; CHECK-NEXT: successors: %bb.11(0x80000000)
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: [[COPY6:%[0-9]+]]:gpr = COPY $r0
; CHECK-NEXT: PseudoBR %bb.11
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: bb.7:
; CHECK-NEXT: successors: %bb.8(0x7c000000), %bb.10(0x04000000)
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: BEQZ [[ANDI2]], %bb.10
; CHECK-NEXT: PseudoBR %bb.8
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: bb.8:
; CHECK-NEXT: successors: %bb.9(0x04000000), %bb.5(0x7c000000)
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: [[COPY6:%[0-9]+]]:gpr = ADDI_D [[COPY6]], 1
; CHECK-NEXT: BEQZ [[ANDI3]], %bb.5
; CHECK-NEXT: PseudoBR %bb.9
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: bb.9:
; CHECK-NEXT: successors: %bb.12(0x80000000)
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: ST_B $r0, [[COPY4]], 0
; CHECK-NEXT: PseudoBR %bb.12
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: bb.10:
; CHECK-NEXT: successors: %bb.11(0x80000000)
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: [[COPY5:%[0-9]+]]:gpr = ADDI_D [[COPY6]], 1
; CHECK-NEXT: [[COPY6:%[0-9]+]]:gpr = COPY [[ORI]]
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: bb.11:
; CHECK-NEXT: successors: %bb.12(0x80000000)
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: ST_D $r0, [[COPY4]], 0
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: bb.12:
; CHECK-NEXT: successors: %bb.2(0x7c000000), %bb.1(0x04000000)
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: BEQ [[COPY7]], [[ORI]], %bb.2
; CHECK-NEXT: PseudoBR %bb.1
bb.0:
liveins: $r4, $r5, $r6, $r7, $r8

%22:gpr = COPY killed $r8
%21:gpr = COPY killed $r7
%20:gpr = COPY killed $r6
%19:gpr = COPY killed $r5
%18:gpr = COPY killed $r4
%29:gpr = COPY $r0
%27:gpr = COPY killed %29
%30:gpr = ANDI killed %19, 1
%41:gpr = ORI $r0, 1
%35:gpr = ANDI killed %20, 1
%36:gpr = ANDI killed %21, 1
%39:gpr = ANDI killed %22, 1
%43:gpr = COPY %27
%44:gpr = COPY killed %27
%45:gpr = IMPLICIT_DEF

bb.1:
%2:gpr = COPY killed %45
%1:gpr = COPY killed %44
%0:gpr = COPY killed %43
%46:gpr = COPY %0
%47:gpr = COPY %1
%48:gpr = COPY killed %1
%49:gpr = COPY killed %2

bb.2:
successors: %bb.3, %bb.4

%6:gpr = COPY killed %49
%5:gpr = COPY killed %48
%4:gpr = COPY killed %47
%3:gpr = COPY killed %46
BEQZ %30, %bb.4

bb.3:
%51:gpr = COPY killed %4
%52:gpr = COPY killed %5
PseudoBR %bb.9

bb.4:
%50:gpr = COPY killed %5

bb.5:
successors: %bb.7(0x7c000000), %bb.6(0x04000000)

%7:gpr = COPY killed %50
dead %33:gpr = LD_D $r0, 8
dead %34:gpr = LD_D $r0, 0
BNEZ %35, %bb.7

bb.6:
%32:gpr = COPY $r0
%31:gpr = COPY killed %32
%53:gpr = COPY killed %31
%54:gpr = COPY killed %6
PseudoBR %bb.11

bb.7:
successors: %bb.8(0x7c000000), %bb.10(0x04000000)

BEQZ %36, %bb.10
PseudoBR %bb.8

bb.8:
successors: %bb.9(0x04000000), %bb.5(0x7c000000)

%8:gpr = ADDI_D killed %7, 1
%50:gpr = COPY %8
%51:gpr = COPY %8
%52:gpr = COPY killed %8
BEQZ %39, %bb.5
PseudoBR %bb.9

bb.9:
%10:gpr = COPY killed %52
%9:gpr = COPY killed %51
%40:gpr = COPY $r0
ST_B killed %40, %18, 0
%55:gpr = COPY killed %3
%56:gpr = COPY killed %9
%57:gpr = COPY killed %10
%58:gpr = COPY killed %6
PseudoBR %bb.12

bb.10:
%42:gpr = ADDI_D killed %7, 1
%53:gpr = COPY %41
%54:gpr = COPY killed %42

bb.11:
%13:gpr = COPY killed %54
%12:gpr = COPY killed %53
%38:gpr = COPY $r0
ST_D killed %38, %18, 0
%55:gpr = COPY %13
%56:gpr = COPY %12
%57:gpr = COPY killed %12
%58:gpr = COPY killed %13

bb.12:
successors: %bb.2(0x7c000000), %bb.1(0x04000000)

%17:gpr = COPY killed %58
%16:gpr = COPY killed %57
%15:gpr = COPY killed %56
%14:gpr = COPY killed %55
%43:gpr = COPY %14
%44:gpr = COPY %15
%45:gpr = COPY %17
%46:gpr = COPY killed %14
%47:gpr = COPY killed %15
%48:gpr = COPY killed %16
%49:gpr = COPY killed %17
BEQ %0, %41, %bb.2
PseudoBR %bb.1

...