[RegisterCoalescer] Skip CR_Replace pattern that has early-clobber in segment's edge

[BeMg]

#71023

Skip the situation that will trigger assertion.

The pruneValues can't handle the early-clobber in edge, but it will be mark as CR_Replace

LHS: %12 [32r,80r:0)[96r,240r:1) 0@32r 1@96r RHS: %17 [240e,288r:1) 0@48r 1@240e 

In result, it make LiveInterval try to join the overlap segments with different VNIInfo.

This patch find this pattern and skip in beginning.

[llvmbot]

@llvm/pr-subscribers-llvm-regalloc

Author: Piyou Chen (BeMg)

Changes

#71023

Skip the situation that will trigger assertion.

The pruneValues can't handle the early-clobber in edge, but it will be mark as CR_Replace

LHS: %12 [32r,80r:0)[96r,240r:1) 0@<!-- -->32r 1@<!-- -->96r RHS: %17 [240e,288r:1) 0@<!-- -->48r 1@<!-- -->240e 

In result, it make LiveInterval try to join the overlap segments with different VNIInfo.

This patch find this pattern and skip in beginning.

---

Full diff: https://github.com/llvm/llvm-project/pull/71024.diff

2 Files Affected:

• (modified) llvm/lib/CodeGen/RegisterCoalescer.cpp (+39)
• (added) llvm/test/CodeGen/RISCV/reg-coalescing-replace-not-resolvable.mir (+66)

diff --git a/llvm/lib/CodeGen/RegisterCoalescer.cpp b/llvm/lib/CodeGen/RegisterCoalescer.cpp index 9858482cd51b4a7..c962668d314c0fd 100644 --- a/llvm/lib/CodeGen/RegisterCoalescer.cpp +++ b/llvm/lib/CodeGen/RegisterCoalescer.cpp @@ -2612,6 +2612,8 @@ class JoinVals { ConflictResolution getResolution(unsigned Num) const { return Vals[Num].Resolution; } + + bool canCRReplaceBeResolved(JoinVals &Other); }; } // end anonymous namespace @@ -3481,6 +3483,39 @@ void JoinVals::eraseInstrs(SmallPtrSetImpl<MachineInstr*> &ErasedInstrs, } } +// If the overlapping segment all be mark as CR_Replace resolution,  +// but can't be resolved resolveConflicts/pruneValues function. +//  +// This function target this particular pattern and make joinVReg fail.  +// +// For example:  +// LHS: %12 [32r,80r:0)[96r,240r:1) 0@32r 1@96r  +// RHS: %17 [240e,288r:1) 0@48r 1@240e +// [96r,240r:2) and [240e,288r:1) both be mark as CR_Replace,  +// but does't be resolved after pruneValues. +//  +bool JoinVals::canCRReplaceBeResolved(JoinVals &Other) { + for (auto Seg : LR.segments) { + for (auto OtherSeg : Other.LR.segments) { + int NewVNInoSeg = getAssignments()[Seg.valno->id]; + int NewVNInoOtherSeg = Other.getAssignments()[OtherSeg.valno->id]; + + if (NewVNInoSeg == NewVNInoOtherSeg) + continue; + + if (!(getResolution(Seg.valno->id) == CR_Replace && + Other.getResolution(OtherSeg.valno->id) == CR_Replace)) + continue; + + if (Seg.contains(OtherSeg.start) && OtherSeg.start.isEarlyClobber() && + !Seg.end.isEarlyClobber() && + (Seg.end.getBaseIndex() == OtherSeg.start.getBaseIndex())) + return false; + } + } + return true; +} + void RegisterCoalescer::joinSubRegRanges(LiveRange &LRange, LiveRange &RRange, LaneBitmask LaneMask, const CoalescerPair &CP) { @@ -3599,6 +3634,10 @@ bool RegisterCoalescer::joinVirtRegs(CoalescerPair &CP) { if (!LHSVals.resolveConflicts(RHSVals) || !RHSVals.resolveConflicts(LHSVals)) return false; + // Some CR_Replace can't be solved by pruneValues. Early exit here. + if (!LHSVals.canCRReplaceBeResolved(RHSVals) || !RHSVals.canCRReplaceBeResolved(LHSVals)) + return false; + // All clear, the live ranges can be merged. if (RHS.hasSubRanges() || LHS.hasSubRanges()) { BumpPtrAllocator &Allocator = LIS->getVNInfoAllocator(); diff --git a/llvm/test/CodeGen/RISCV/reg-coalescing-replace-not-resolvable.mir b/llvm/test/CodeGen/RISCV/reg-coalescing-replace-not-resolvable.mir new file mode 100644 index 000000000000000..a3f1ac7f0015602 --- /dev/null +++ b/llvm/test/CodeGen/RISCV/reg-coalescing-replace-not-resolvable.mir @@ -0,0 +1,66 @@ +# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 3 +# RUN: llc %s -o - -mtriple=riscv64 -mattr=+v -run-pass=register-coalescer \ +# RUN: -verify-machineinstrs | FileCheck %s + +--- +name: main +tracksRegLiveness: true +body: | + ; CHECK-LABEL: name: main + ; CHECK: bb.0: + ; CHECK-NEXT: successors: %bb.3(0x40000000), %bb.1(0x40000000) + ; CHECK-NEXT: liveins: $x10, $v8, $v10 + ; CHECK-NEXT: {{ $}} + ; CHECK-NEXT: dead [[DEF:%[0-9]+]]:gpr = IMPLICIT_DEF + ; CHECK-NEXT: undef [[PseudoVMV_V_I_M1_:%[0-9]+]].sub_vrm1_0:vrn6m1nov0 = PseudoVMV_V_I_M1 undef [[PseudoVMV_V_I_M1_]].sub_vrm1_0, 0, -1, 3 /* e8 */, 0 /* tu, mu */, implicit $vl, implicit $vtype + ; CHECK-NEXT: undef [[DEF1:%[0-9]+]].sub_vrm1_5:vrn6m1 = IMPLICIT_DEF + ; CHECK-NEXT: [[PseudoVMV_V_I_M1_:%[0-9]+]].sub_vrm1_3:vrn6m1nov0 = COPY [[DEF1]].sub_vrm1_5 + ; CHECK-NEXT: [[PseudoVMV_V_I_M1_:%[0-9]+]].sub_vrm1_4:vrn6m1nov0 = COPY undef [[PseudoVMV_V_I_M1_]].sub_vrm1_0 + ; CHECK-NEXT: BNE undef [[DEF]], $x0, %bb.3 + ; CHECK-NEXT: PseudoBR %bb.1 + ; CHECK-NEXT: {{ $}} + ; CHECK-NEXT: bb.1: + ; CHECK-NEXT: successors: %bb.3(0x40000000), %bb.2(0x40000000) + ; CHECK-NEXT: {{ $}} + ; CHECK-NEXT: BNE undef [[DEF]], $x0, %bb.3 + ; CHECK-NEXT: PseudoBR %bb.2 + ; CHECK-NEXT: {{ $}} + ; CHECK-NEXT: bb.2: + ; CHECK-NEXT: successors: %bb.3(0x80000000) + ; CHECK-NEXT: {{ $}} + ; CHECK-NEXT: {{ $}} + ; CHECK-NEXT: bb.3: + ; CHECK-NEXT: dead [[DEF2:%[0-9]+]]:vr = IMPLICIT_DEF + ; CHECK-NEXT: early-clobber [[DEF1]].sub_vrm1_0:vrn6m1 = PseudoVRGATHER_VI_M1 undef [[DEF1]].sub_vrm1_0, [[PseudoVMV_V_I_M1_]].sub_vrm1_0, 0, 0, 3 /* e8 */, 0 /* tu, mu */, implicit $vl, implicit $vtype + ; CHECK-NEXT: PseudoVSSEG6E8_V_M1_MASK [[DEF1]], undef [[DEF]], killed undef $v0, 0, 3 /* e8 */, implicit $vl, implicit $vtype :: (store unknown-size, align 1) + ; CHECK-NEXT: PseudoRET + bb.0: + successors: %bb.5(0x40000000), %bb.3(0x40000000) + liveins: $x10, $v8, $v10 + + %32:gpr = IMPLICIT_DEF + %10:vrnov0 = PseudoVMV_V_I_M1 undef %10, 0, -1, 3 /* e8 */, 0 /* tu, mu */, implicit $vl, implicit $vtype + %11:vrnov0 = IMPLICIT_DEF + undef %12.sub_vrm1_0:vrn6m1nov0 = COPY undef %10 + %12.sub_vrm1_3:vrn6m1nov0 = COPY %11 + %12.sub_vrm1_4:vrn6m1nov0 = COPY undef %10 + BNE undef %32, $x0, %bb.5 + PseudoBR %bb.3 + + bb.3: + successors: %bb.5(0x40000000), %bb.4(0x40000000) + + BNE killed undef %32, $x0, %bb.5 + PseudoBR %bb.4 + + bb.4: + successors: %bb.5(0x80000000) + + bb.5: + %1:vr = IMPLICIT_DEF + early-clobber %1:vr = PseudoVRGATHER_VI_M1 undef %1, killed %10, 0, 0, 3 /* e8 */, 0 /* tu, mu */, implicit $vl, implicit $vtype + undef %17.sub_vrm1_0:vrn6m1 = COPY killed %1 + %17.sub_vrm1_5:vrn6m1 = COPY killed %11 + PseudoVSSEG6E8_V_M1_MASK killed %17, undef %32, killed undef $v0, 0, 3 /* e8 */, implicit $vl, implicit $vtype :: (store unknown-size, align 1) + PseudoRET +... 

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[arsenm]

Title should also be revised to be more descriptive of the issue

[arsenm]

Should compact the register numbers by using -run-pass=none

[arsenm]

Instead of fixing this up after the fact, you should avoid reporting CR_Replace in the first place

[MatzeB]

Does that mean you encounter generic COPY or SUBREG_TO_REG instructions with an early-clobber destination (the coalescer only handles instructions with MachineInstr::isCopyLike() no?) ? Is that intentional? My first instinct would we be to just not allow early-clobber flags for these generic opcodes...

[BeMg]

Does that mean you encounter generic COPY or SUBREG_TO_REG instructions with an early-clobber destination (the coalescer only handles instructions with MachineInstr::isCopyLike() no?) ? Is that intentional? My first instinct would we be to just not allow early-clobber flags for these generic opcodes...

The early-clobber comes from the RISC-V pseudo PseudoVRGATHER_VI_M1. And all VReg coalescing is triggered by COPY without early-clobber itself.

[BeMg]

Found JoinVals::analyzeValue already handle this pattern but early exit by if ((V.WriteLanes & OtherV.ValidLanes).none()) case.

Reorder the condition to resolved it.

[BeMg]

This coalescing should be valid but somehow trigger the assertion accidentally.

The current approach is a workaround to avoid the assertion fail. I will investigate how to correctly handle this pattern in the register coalescing pass.