[RA] Don't split a register generated from another split

[weiguozhi]

Split a register generated from another split usually doesn't bring us too much benefit. It may also cause dead loop as pr67188 shows if the heuristic cost always satisfy the split condition. So prevent such splitting.

It fixed pr67188.

[llvmbot]

@llvm/pr-subscribers-backend-arm

Changes

Split a register generated from another split usually doesn't bring us too much benefit. It may also cause dead loop as pr67188 shows if the heuristic cost always satisfy the split condition. So prevent such splitting.

It fixed pr67188.

---

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

2 Files Affected:

• (modified) llvm/lib/CodeGen/RegAllocGreedy.cpp (+3)
• (added) llvm/test/CodeGen/ARM/split-deadloop.mir (+111)

diff --git a/llvm/lib/CodeGen/RegAllocGreedy.cpp b/llvm/lib/CodeGen/RegAllocGreedy.cpp
index 36625c4848c5349..ae5fafe77f0e944 100644
--- a/llvm/lib/CodeGen/RegAllocGreedy.cpp
+++ b/llvm/lib/CodeGen/RegAllocGreedy.cpp
@@ -1218,6 +1218,9 @@ bool RAGreedy::trySplitAroundHintReg(MCPhysReg Hint,
                                      const LiveInterval &VirtReg,
                                      SmallVectorImpl<Register> &NewVRegs,
                                      AllocationOrder &Order) {
+  if (ExtraInfo->getStage(VirtReg) >= RS_Split2)
+    return false;
+
   BlockFrequency Cost = 0;
   Register Reg = VirtReg.reg();
 
diff --git a/llvm/test/CodeGen/ARM/split-deadloop.mir b/llvm/test/CodeGen/ARM/split-deadloop.mir
new file mode 100644
index 000000000000000..d5f8303ab13ecb0
--- /dev/null
+++ b/llvm/test/CodeGen/ARM/split-deadloop.mir
@@ -0,0 +1,111 @@
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 3
+# RUN: llc -o - %s -run-pass=greedy | FileCheck %s
+#
+# Make sure we don't run into dead loop when split register with a hint.
+
+--- |
+  target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+  target triple = "aarch64-none-linux-gnu"
+
+  define void @foo(ptr %0, i32 %1) gc "statepoint-example" personality ptr null {
+  bci_0:
+    unreachable
+
+  bci_19:
+    unreachable
+
+  bci_10:
+    unreachable
+  }
+
+...
+---
+name:            foo
+alignment:       4
+exposesReturnsTwice: false
+legalized:       false
+regBankSelected: false
+selected:        false
+failedISel:      false
+tracksRegLiveness: true
+hasWinCFI:       false
+callsEHReturn:   false
+callsUnwindInit: false
+hasEHCatchret:   false
+hasEHScopes:     false
+hasEHFunclets:   false
+isOutlined:      false
+debugInstrRef:   false
+failsVerification: false
+tracksDebugUserValues: false
+registers:
+  - { id: 0, class: gpr64, preferred-register: '' }
+  - { id: 1, class: gpr32, preferred-register: '' }
+  - { id: 2, class: gpr32, preferred-register: '' }
+  - { id: 3, class: gpr64all, preferred-register: '' }
+  - { id: 4, class: gpr64all, preferred-register: '' }
+  - { id: 5, class: gpr32all, preferred-register: '' }
+liveins:
+  - { reg: '$w1', virtual-reg: '%1' }
+frameInfo:
+  isFrameAddressTaken: false
+  isReturnAddressTaken: false
+  hasStackMap:     false
+  hasPatchPoint:   false
+  stackSize:       0
+  offsetAdjustment: 0
+  maxAlignment:    1
+  adjustsStack:    true
+  hasCalls:        true
+  stackProtector:  ''
+  functionContext: ''
+  maxCallFrameSize: 0
+  cvBytesOfCalleeSavedRegisters: 0
+  hasOpaqueSPAdjustment: false
+  hasVAStart:      false
+  hasMustTailInVarArgFunc: false
+  hasTailCall:     false
+  localFrameSize:  0
+  savePoint:       ''
+  restorePoint:    ''
+fixedStack:      []
+stack:           []
+entry_values:    []
+callSites:       []
+debugValueSubstitutions: []
+constants:       []
+machineFunctionInfo: {}
+body:             |
+  ; CHECK-LABEL: name: foo
+  ; CHECK: bb.0.bci_0:
+  ; CHECK-NEXT:   successors: %bb.1(0x80000000)
+  ; CHECK-NEXT:   liveins: $w1
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   [[COPY:%[0-9]+]]:gpr32 = COPY $w1
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.1.bci_19:
+  ; CHECK-NEXT:   ADJCALLSTACKDOWN 0, 0, implicit-def dead $sp, implicit $sp
+  ; CHECK-NEXT:   $x0 = COPY $xzr
+  ; CHECK-NEXT:   STATEPOINT 0, 0, 1, $xzr, killed $x0, 2, 0, 2, 0, 2, 33, 2, 0, 2, 0, 2, 0, 2, 0, 2, 0, 2, 0, 2, 0, 2, 0, 2, 0, 2, 0, [[COPY]], 2, 0, 2, 0, 2, 0, 2, 0, 2, 0, 2, 0, 2, 0, 2, 0, 2, 0, 2, 0, 2, 0, 2, 0, 2, 0, 2, 0, 2, 0, 2, 0, 2, 0, 2, 0, 2, 0, 2, 0, 2, 0, 2, 0, 2, 1, 2, 0, 2, 0, 2, 1, 0, 0, csr_aarch64_aapcs, implicit-def $sp, implicit-def dead $x0, implicit-def dead early-clobber $lr
+  ; CHECK-NEXT:   ADJCALLSTACKUP 0, 0, implicit-def dead $sp, implicit $sp
+  ; CHECK-NEXT:   ADJCALLSTACKDOWN 0, 0, implicit-def dead $sp, implicit $sp
+  ; CHECK-NEXT:   $w0 = COPY $wzr
+  ; CHECK-NEXT:   STATEPOINT 0, 0, 1, $xzr, killed $w0, 2, 0, 2, 0, 2, 0, 2, 0, 2, 0, 2, 0, csr_aarch64_aapcs, implicit-def $sp, implicit-def dead early-clobber $lr
+  ; CHECK-NEXT:   ADJCALLSTACKUP 0, 0, implicit-def dead $sp, implicit $sp
+  bb.0.bci_0:
+    successors: %bb.1(0x80000000)
+    liveins: $w1
+
+    %1:gpr32 = COPY $w1
+
+  bb.1.bci_19:
+    ADJCALLSTACKDOWN 0, 0, implicit-def dead $sp, implicit $sp
+    $x0 = COPY $xzr
+    STATEPOINT 0, 0, 1, $xzr, killed $x0, 2, 0, 2, 0, 2, 33, 2, 0, 2, 0, 2, 0, 2, 0, 2, 0, 2, 0, 2, 0, 2, 0, 2, 0, 2, 0, %1, 2, 0, 2, 0, 2, 0, 2, 0, 2, 0, 2, 0, 2, 0, 2, 0, 2, 0, 2, 0, 2, 0, 2, 0, 2, 0, 2, 0, 2, 0, 2, 0, 2, 0, 2, 0, 2, 0, 2, 0, 2, 0, 2, 0, 2, 1, 2, 0, 2, 0, 2, 1, 0, 0, csr_aarch64_aapcs, implicit-def $sp, implicit-def dead $x0, implicit-def dead early-clobber $lr
+    ADJCALLSTACKUP 0, 0, implicit-def dead $sp, implicit $sp
+    ADJCALLSTACKDOWN 0, 0, implicit-def dead $sp, implicit $sp
+    $w0 = COPY $wzr
+    STATEPOINT 0, 0, 1, $xzr, killed $w0, 2, 0, 2, 0, 2, 0, 2, 0, 2, 0, 2, 0, csr_aarch64_aapcs, implicit-def $sp, implicit-def dead early-clobber $lr
+    ADJCALLSTACKUP 0, 0, implicit-def dead $sp, implicit $sp
+
+...

[danilaml]

Looks good to me. Maybe add a short comment to explain why we bail out early in this case.

[weiguozhi]

Looks good to me. Maybe add a short comment to explain why we bail out early in this case.

I copied the comment from splitAroundRegion which is for the same purpose.

[danilaml]

LGTM. Not an RA expert, but the sooner we fix the hang the better.

[qcolombet]

Nice catch!
The infinite looping was one of my fear for doing that outside of the regular splitting heuristic.
I should have caught this.

Anyhow thanks for the fix.