Skip to content
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.

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

Refactor recomputeLiveIns to operate on whole CFG #79498

Merged
merged 1 commit into from
Jan 26, 2024

Conversation

oskarwirga
Copy link
Contributor

Currently, the way that recomputeLiveIns works is that it will recompute the livein registers for that MachineBasicBlock but it matters what order you call recomputeLiveIn which can result in incorrect register allocations down the line.

This PR fixes that by simply recomputing the liveins for the entire CFG until convergence is achieved. This makes it harder to introduce subtle bugs which alter liveness.

@llvmbot
Copy link
Collaborator

llvmbot commented Jan 25, 2024

@llvm/pr-subscribers-backend-arm

@llvm/pr-subscribers-backend-aarch64

Author: Oskar Wirga (oskarwirga)

Changes

Currently, the way that recomputeLiveIns works is that it will recompute the livein registers for that MachineBasicBlock but it matters what order you call recomputeLiveIn which can result in incorrect register allocations down the line.

This PR fixes that by simply recomputing the liveins for the entire CFG until convergence is achieved. This makes it harder to introduce subtle bugs which alter liveness.


Patch is 297.53 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/79498.diff

30 Files Affected:

  • (modified) llvm/include/llvm/CodeGen/LivePhysRegs.h (+23-2)
  • (modified) llvm/include/llvm/CodeGen/MachineBasicBlock.h (+6)
  • (modified) llvm/lib/CodeGen/BranchFolding.cpp (+1-2)
  • (modified) llvm/lib/Target/AArch64/AArch64FrameLowering.cpp (+1-2)
  • (modified) llvm/lib/Target/AArch64/AArch64InstrInfo.cpp (+1-3)
  • (modified) llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp (+1-6)
  • (modified) llvm/lib/Target/PowerPC/PPCExpandAtomicPseudoInsts.cpp (+2-5)
  • (modified) llvm/lib/Target/PowerPC/PPCFrameLowering.cpp (+2-4)
  • (modified) llvm/lib/Target/SystemZ/SystemZFrameLowering.cpp (+2-4)
  • (modified) llvm/lib/Target/X86/X86FrameLowering.cpp (+2-6)
  • (modified) llvm/test/CodeGen/AArch64/stack-probing-last-in-block.mir (+1-1)
  • (modified) llvm/test/CodeGen/SystemZ/branch-folder-hoist-livein.mir (+25-11)
  • (modified) llvm/test/CodeGen/Thumb2/LowOverheadLoops/biquad-cascade-default.mir (+102-90)
  • (modified) llvm/test/CodeGen/Thumb2/LowOverheadLoops/biquad-cascade-optsize-strd-lr.mir (+89-78)
  • (modified) llvm/test/CodeGen/Thumb2/LowOverheadLoops/biquad-cascade-optsize.mir (+96-87)
  • (modified) llvm/test/CodeGen/Thumb2/LowOverheadLoops/it-block-mov.mir (+87-74)
  • (modified) llvm/test/CodeGen/Thumb2/LowOverheadLoops/loop-dec-copy-chain.mir (+138-120)
  • (modified) llvm/test/CodeGen/Thumb2/LowOverheadLoops/loop-dec-copy-prev-iteration.mir (+155-130)
  • (modified) llvm/test/CodeGen/Thumb2/LowOverheadLoops/loop-dec-liveout.mir (+154-129)
  • (modified) llvm/test/CodeGen/Thumb2/LowOverheadLoops/matrix-debug.mir (+80-70)
  • (modified) llvm/test/CodeGen/Thumb2/LowOverheadLoops/matrix.mir (+173-144)
  • (modified) llvm/test/CodeGen/Thumb2/LowOverheadLoops/mov-after-dlstp.mir (+58-56)
  • (modified) llvm/test/CodeGen/Thumb2/LowOverheadLoops/multi-block-cond-iter-count.mir (+132-112)
  • (modified) llvm/test/CodeGen/Thumb2/LowOverheadLoops/multiple-do-loops.mir (+233-193)
  • (modified) llvm/test/CodeGen/Thumb2/LowOverheadLoops/predicated-liveout.mir (+38-29)
  • (modified) llvm/test/CodeGen/Thumb2/LowOverheadLoops/remove-elem-moves.mir (+97-79)
  • (modified) llvm/test/CodeGen/Thumb2/LowOverheadLoops/skip-debug.mir (+65-56)
  • (modified) llvm/test/CodeGen/Thumb2/LowOverheadLoops/skip-vpt-debug.mir (+71-68)
  • (modified) llvm/test/CodeGen/Thumb2/LowOverheadLoops/spillingmove.mir (+7-7)
  • (modified) llvm/test/CodeGen/Thumb2/LowOverheadLoops/unrolled-and-vector.mir (+154-130)
diff --git a/llvm/include/llvm/CodeGen/LivePhysRegs.h b/llvm/include/llvm/CodeGen/LivePhysRegs.h
index 76bb34d270a26dc..1e0ee9eb9eb3203 100644
--- a/llvm/include/llvm/CodeGen/LivePhysRegs.h
+++ b/llvm/include/llvm/CodeGen/LivePhysRegs.h
@@ -31,6 +31,7 @@
 
 #include "llvm/ADT/SparseSet.h"
 #include "llvm/CodeGen/MachineBasicBlock.h"
+#include "llvm/CodeGen/MachineFunction.h"
 #include "llvm/CodeGen/TargetRegisterInfo.h"
 #include "llvm/MC/MCRegister.h"
 #include "llvm/MC/MCRegisterInfo.h"
@@ -193,11 +194,31 @@ void addLiveIns(MachineBasicBlock &MBB, const LivePhysRegs &LiveRegs);
 void computeAndAddLiveIns(LivePhysRegs &LiveRegs,
                           MachineBasicBlock &MBB);
 
-/// Convenience function for recomputing live-in's for \p MBB.
-static inline void recomputeLiveIns(MachineBasicBlock &MBB) {
+/// Function to update the live-in's for a basic block and return whether any
+/// changes were made.
+static inline bool updateBlockLiveInfo(MachineBasicBlock &MBB) {
   LivePhysRegs LPR;
+  auto oldLiveIns = MBB.getLiveIns();
+
   MBB.clearLiveIns();
   computeAndAddLiveIns(LPR, MBB);
+  MBB.sortUniqueLiveIns();
+
+  auto newLiveIns = MBB.getLiveIns();
+  return oldLiveIns != newLiveIns;
+}
+
+/// Convenience function for recomputing live-in's for the entire CFG until
+/// convergence is reached.
+static inline void recomputeLiveIns(MachineFunction &MF) {
+  bool anyChanged;
+  do {
+    anyChanged = false;
+    for (auto MFI = MF.rbegin(), MFE = MF.rend(); MFI != MFE; ++MFI) {
+      MachineBasicBlock &MBB = *MFI;
+      anyChanged |= updateBlockLiveInfo(MBB);
+    }
+  } while (anyChanged);
 }
 
 } // end namespace llvm
diff --git a/llvm/include/llvm/CodeGen/MachineBasicBlock.h b/llvm/include/llvm/CodeGen/MachineBasicBlock.h
index c84fd281c6a549b..dc2035fa598c46d 100644
--- a/llvm/include/llvm/CodeGen/MachineBasicBlock.h
+++ b/llvm/include/llvm/CodeGen/MachineBasicBlock.h
@@ -111,6 +111,10 @@ class MachineBasicBlock
 
     RegisterMaskPair(MCPhysReg PhysReg, LaneBitmask LaneMask)
         : PhysReg(PhysReg), LaneMask(LaneMask) {}
+
+    bool operator==(const RegisterMaskPair &other) const {
+      return PhysReg == other.PhysReg && LaneMask == other.LaneMask;
+    }
   };
 
 private:
@@ -473,6 +477,8 @@ class MachineBasicBlock
   /// Remove entry from the livein set and return iterator to the next.
   livein_iterator removeLiveIn(livein_iterator I);
 
+  std::vector<RegisterMaskPair> getLiveIns() const { return LiveIns; }
+
   class liveout_iterator {
   public:
     using iterator_category = std::input_iterator_tag;
diff --git a/llvm/lib/CodeGen/BranchFolding.cpp b/llvm/lib/CodeGen/BranchFolding.cpp
index a9f78358e57b929..9075fa436da58cb 100644
--- a/llvm/lib/CodeGen/BranchFolding.cpp
+++ b/llvm/lib/CodeGen/BranchFolding.cpp
@@ -2048,8 +2048,7 @@ bool BranchFolder::HoistCommonCodeInSuccs(MachineBasicBlock *MBB) {
   FBB->erase(FBB->begin(), FIB);
 
   if (UpdateLiveIns) {
-    recomputeLiveIns(*TBB);
-    recomputeLiveIns(*FBB);
+    recomputeLiveIns(*MBB->getParent());
   }
 
   ++NumHoist;
diff --git a/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp b/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
index cffd414221c30cf..785f563faf4bb1a 100644
--- a/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
+++ b/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
@@ -4339,8 +4339,7 @@ AArch64FrameLowering::inlineStackProbeLoopExactMultiple(
   ExitMBB->transferSuccessorsAndUpdatePHIs(&MBB);
   MBB.addSuccessor(LoopMBB);
   // Update liveins.
-  recomputeLiveIns(*LoopMBB);
-  recomputeLiveIns(*ExitMBB);
+  recomputeLiveIns(MF);
 
   return ExitMBB->begin();
 }
diff --git a/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp b/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
index 2e8d8c63d6bec24..72722aa95b846e3 100644
--- a/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
+++ b/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
@@ -9597,9 +9597,7 @@ AArch64InstrInfo::probedStackAlloc(MachineBasicBlock::iterator MBBI,
 
   // Update liveins.
   if (MF.getRegInfo().reservedRegsFrozen()) {
-    recomputeLiveIns(*LoopTestMBB);
-    recomputeLiveIns(*LoopBodyMBB);
-    recomputeLiveIns(*ExitMBB);
+    recomputeLiveIns(MF);
   }
 
   return ExitMBB->begin();
diff --git a/llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp b/llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp
index 5c1c7046fdbff09..e199a21da500a17 100644
--- a/llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp
+++ b/llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp
@@ -1806,12 +1806,7 @@ void ARMLowOverheadLoops::Expand(LowOverheadLoop &LoLoop) {
   PostOrderLoopTraversal DFS(LoLoop.ML, *MLI);
   DFS.ProcessLoop();
   const SmallVectorImpl<MachineBasicBlock*> &PostOrder = DFS.getOrder();
-  for (auto *MBB : PostOrder) {
-    recomputeLiveIns(*MBB);
-    // FIXME: For some reason, the live-in print order is non-deterministic for
-    // our tests and I can't out why... So just sort them.
-    MBB->sortUniqueLiveIns();
-  }
+  recomputeLiveIns(*LoLoop.MF);
 
   for (auto *MBB : reverse(PostOrder))
     recomputeLivenessFlags(*MBB);
diff --git a/llvm/lib/Target/PowerPC/PPCExpandAtomicPseudoInsts.cpp b/llvm/lib/Target/PowerPC/PPCExpandAtomicPseudoInsts.cpp
index aee57a5075ff719..c170bafe9b184c9 100644
--- a/llvm/lib/Target/PowerPC/PPCExpandAtomicPseudoInsts.cpp
+++ b/llvm/lib/Target/PowerPC/PPCExpandAtomicPseudoInsts.cpp
@@ -208,8 +208,7 @@ bool PPCExpandAtomicPseudo::expandAtomicRMW128(
       .addMBB(LoopMBB);
   CurrentMBB->addSuccessor(LoopMBB);
   CurrentMBB->addSuccessor(ExitMBB);
-  recomputeLiveIns(*LoopMBB);
-  recomputeLiveIns(*ExitMBB);
+  recomputeLiveIns(*MF);
   NMBBI = MBB.end();
   MI.eraseFromParent();
   return true;
@@ -286,9 +285,7 @@ bool PPCExpandAtomicPseudo::expandAtomicCmpSwap128(
   CurrentMBB->addSuccessor(LoopCmpMBB);
   CurrentMBB->addSuccessor(ExitMBB);
 
-  recomputeLiveIns(*LoopCmpMBB);
-  recomputeLiveIns(*CmpSuccMBB);
-  recomputeLiveIns(*ExitMBB);
+  recomputeLiveIns(*MF);
   NMBBI = MBB.end();
   MI.eraseFromParent();
   return true;
diff --git a/llvm/lib/Target/PowerPC/PPCFrameLowering.cpp b/llvm/lib/Target/PowerPC/PPCFrameLowering.cpp
index 245e78641ed6544..f67658302958c3b 100644
--- a/llvm/lib/Target/PowerPC/PPCFrameLowering.cpp
+++ b/llvm/lib/Target/PowerPC/PPCFrameLowering.cpp
@@ -1441,8 +1441,7 @@ void PPCFrameLowering::inlineStackProbe(MachineFunction &MF,
       ProbeLoopBodyMBB->addSuccessor(ProbeLoopBodyMBB);
     }
     // Update liveins.
-    recomputeLiveIns(*ProbeLoopBodyMBB);
-    recomputeLiveIns(*ProbeExitMBB);
+    recomputeLiveIns(MF);
     return ProbeExitMBB;
   };
   // For case HasBP && MaxAlign > 1, we have to realign the SP by performing
@@ -1534,8 +1533,7 @@ void PPCFrameLowering::inlineStackProbe(MachineFunction &MF,
         buildDefCFAReg(*ExitMBB, ExitMBB->begin(), SPReg);
       }
       // Update liveins.
-      recomputeLiveIns(*LoopMBB);
-      recomputeLiveIns(*ExitMBB);
+      recomputeLiveIns(MF);
     }
   }
   ++NumPrologProbed;
diff --git a/llvm/lib/Target/SystemZ/SystemZFrameLowering.cpp b/llvm/lib/Target/SystemZ/SystemZFrameLowering.cpp
index db19c8881c685a5..965688dec69b252 100644
--- a/llvm/lib/Target/SystemZ/SystemZFrameLowering.cpp
+++ b/llvm/lib/Target/SystemZ/SystemZFrameLowering.cpp
@@ -840,8 +840,7 @@ void SystemZELFFrameLowering::inlineStackProbe(
   StackAllocMI->eraseFromParent();
   if (DoneMBB != nullptr) {
     // Compute the live-in lists for the new blocks.
-    recomputeLiveIns(*DoneMBB);
-    recomputeLiveIns(*LoopMBB);
+    recomputeLiveIns(MF);
   }
 }
 
@@ -1439,8 +1438,7 @@ void SystemZXPLINKFrameLowering::inlineStackProbe(
   StackAllocMI->eraseFromParent();
 
   // Compute the live-in lists for the new blocks.
-  recomputeLiveIns(*NextMBB);
-  recomputeLiveIns(*StackExtMBB);
+  recomputeLiveIns(MF);
 }
 
 bool SystemZXPLINKFrameLowering::hasFP(const MachineFunction &MF) const {
diff --git a/llvm/lib/Target/X86/X86FrameLowering.cpp b/llvm/lib/Target/X86/X86FrameLowering.cpp
index c0d358ead2787b2..4aa27be07913141 100644
--- a/llvm/lib/Target/X86/X86FrameLowering.cpp
+++ b/llvm/lib/Target/X86/X86FrameLowering.cpp
@@ -885,8 +885,7 @@ void X86FrameLowering::emitStackProbeInlineGenericLoop(
   }
 
   // Update Live In information
-  recomputeLiveIns(*testMBB);
-  recomputeLiveIns(*tailMBB);
+  recomputeLiveIns(MF);
 }
 
 void X86FrameLowering::emitStackProbeInlineWindowsCoreCLR64(
@@ -1378,10 +1377,7 @@ void X86FrameLowering::BuildStackAlignAND(MachineBasicBlock &MBB,
         footMBB->addSuccessor(&MBB);
       }
 
-      recomputeLiveIns(*headMBB);
-      recomputeLiveIns(*bodyMBB);
-      recomputeLiveIns(*footMBB);
-      recomputeLiveIns(MBB);
+      recomputeLiveIns(MF);
     }
   } else {
     MachineInstr *MI = BuildMI(MBB, MBBI, DL, TII.get(AndOp), Reg)
diff --git a/llvm/test/CodeGen/AArch64/stack-probing-last-in-block.mir b/llvm/test/CodeGen/AArch64/stack-probing-last-in-block.mir
index 6c8ec7e4c4fa924..8d7fab4cc39c660 100644
--- a/llvm/test/CodeGen/AArch64/stack-probing-last-in-block.mir
+++ b/llvm/test/CodeGen/AArch64/stack-probing-last-in-block.mir
@@ -82,7 +82,7 @@ body:             |
   ; CHECK-LABEL: name: f
   ; CHECK: bb.0.entry:
   ; CHECK-NEXT:   successors: %bb.3(0x80000000)
-  ; CHECK-NEXT:   liveins: $lr, $fp
+  ; CHECK-NEXT:   liveins: $fp, $lr
   ; CHECK-NEXT: {{  $}}
   ; CHECK-NEXT:   early-clobber $sp = frame-setup STPXpre killed $fp, killed $lr, $sp, -2 :: (store (s64) into %stack.2), (store (s64) into %stack.1)
   ; CHECK-NEXT:   frame-setup CFI_INSTRUCTION def_cfa_offset 16
diff --git a/llvm/test/CodeGen/SystemZ/branch-folder-hoist-livein.mir b/llvm/test/CodeGen/SystemZ/branch-folder-hoist-livein.mir
index 5e100b88ead300f..82e3bae97ec0c48 100644
--- a/llvm/test/CodeGen/SystemZ/branch-folder-hoist-livein.mir
+++ b/llvm/test/CodeGen/SystemZ/branch-folder-hoist-livein.mir
@@ -1,3 +1,4 @@
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 4
 # RUN: llc -verify-machineinstrs -O1 -mtriple=s390x-ibm-linux -o - %s -run-pass=branch-folder | FileCheck %s
 --- |
   target datalayout = "E-m:e-i1:8:16-i8:8:16-i64:64-f128:64-a:8:16-n32:64"
@@ -15,6 +16,30 @@
 name:            f1
 tracksRegLiveness: true
 body:             |
+  ; CHECK-LABEL: name: f1
+  ; CHECK: bb.0:
+  ; CHECK-NEXT:   successors: %bb.2(0x7fffffff), %bb.1(0x00000001)
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   renamable $r1d = LGRL @b :: (load (s32) from got, align 8)
+  ; CHECK-NEXT:   renamable $r1l = LH killed renamable $r1d, 0, $noreg, implicit-def $r1d :: (dereferenceable load (s8) from @b)
+  ; CHECK-NEXT:   renamable $r2l = LHI 0
+  ; CHECK-NEXT:   renamable $r3d = LGRL @d :: (load (s32) from got, align 8)
+  ; CHECK-NEXT:   renamable $r4d = LLILL 0, implicit-def $r4q
+  ; CHECK-NEXT:   renamable $r4d = COPY killed renamable $r4d, implicit killed $r4q
+  ; CHECK-NEXT:   CHI killed renamable $r2l, 0, implicit-def $cc
+  ; CHECK-NEXT:   BRC 14, 6, %bb.2, implicit killed $cc
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.1:
+  ; CHECK-NEXT:   successors:
+  ; CHECK-NEXT:   liveins: $r3d, $r4d, $r1l
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   STH renamable $r1l, killed renamable $r3d, 0, $noreg, implicit killed $r4d :: (store (s8) into @d)
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.2:
+  ; CHECK-NEXT:   liveins: $r3d, $r4d, $r1l
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   STH renamable $r1l, killed renamable $r3d, 0, $noreg, implicit killed $r4d :: (store (s8) into @d)
+  ; CHECK-NEXT:   Return
   bb.0:
     successors: %bb.2(0x7fffffff), %bb.1(0x00000001)
     liveins:
@@ -44,14 +69,3 @@ body:             |
     Return
 
 ...
-
-# CHECK: renamable $r4d = COPY killed renamable $r4d, implicit killed $r4q
-# CHECK-NEXT: CHI killed renamable $r2l, 0, implicit-def $cc
-# CHECK-NEXT: BRC 14, 6, %bb.2, implicit killed $cc
-# CHECK-NEXT: {{^  $}}
-# CHECK-NEXT: bb.1:
-# CHECK-NEXT: successors:
-# CHECK-NEXT: liveins: $r1l, $r3d, $r4d
-
-# CHECK: bb.2:
-# CHECK-NEXT: liveins: $r1l, $r3d, $r4d
diff --git a/llvm/test/CodeGen/Thumb2/LowOverheadLoops/biquad-cascade-default.mir b/llvm/test/CodeGen/Thumb2/LowOverheadLoops/biquad-cascade-default.mir
index dabebf4aeb77a81..d9d3fd571ab8de1 100644
--- a/llvm/test/CodeGen/Thumb2/LowOverheadLoops/biquad-cascade-default.mir
+++ b/llvm/test/CodeGen/Thumb2/LowOverheadLoops/biquad-cascade-default.mir
@@ -200,96 +200,108 @@ machineFunctionInfo: {}
 body:             |
   ; CHECK-LABEL: name: arm_biquad_cascade_df1_q31
   ; CHECK: bb.0.bb:
-  ; CHECK:   successors: %bb.2(0x80000000)
-  ; CHECK:   liveins: $r0, $r1, $r2, $r3, $r4, $r5, $r6, $r7, $r8, $r9, $r10, $r11, $lr
-  ; CHECK:   $sp = frame-setup t2STMDB_UPD $sp, 14 /* CC::al */, $noreg, killed $r4, killed $r5, killed $r6, killed $r7, killed $r8, killed $r9, killed $r10, killed $r11, killed $lr
-  ; CHECK:   frame-setup CFI_INSTRUCTION def_cfa_offset 36
-  ; CHECK:   frame-setup CFI_INSTRUCTION offset $lr, -4
-  ; CHECK:   frame-setup CFI_INSTRUCTION offset $r11, -8
-  ; CHECK:   frame-setup CFI_INSTRUCTION offset $r10, -12
-  ; CHECK:   frame-setup CFI_INSTRUCTION offset $r9, -16
-  ; CHECK:   frame-setup CFI_INSTRUCTION offset $r8, -20
-  ; CHECK:   frame-setup CFI_INSTRUCTION offset $r7, -24
-  ; CHECK:   frame-setup CFI_INSTRUCTION offset $r6, -28
-  ; CHECK:   frame-setup CFI_INSTRUCTION offset $r5, -32
-  ; CHECK:   frame-setup CFI_INSTRUCTION offset $r4, -36
-  ; CHECK:   $sp = frame-setup tSUBspi $sp, 10, 14 /* CC::al */, $noreg
-  ; CHECK:   frame-setup CFI_INSTRUCTION def_cfa_offset 76
-  ; CHECK:   $r6, $r5 = t2LDRDi8 $r0, 8, 14 /* CC::al */, $noreg :: (load (s32) from %ir.i7), (load (s32) from %ir.i10)
-  ; CHECK:   $r8 = tMOVr killed $r3, 14 /* CC::al */, $noreg
-  ; CHECK:   $r3, $r7 = t2LDRDi8 killed $r0, 0, 14 /* CC::al */, $noreg :: (load (s32) from %ir.i), (load (s32) from %ir.i5)
-  ; CHECK:   renamable $r0 = t2RSBri killed renamable $r6, 31, 14 /* CC::al */, $noreg, $noreg
-  ; CHECK:   t2STMIA $sp, 14 /* CC::al */, $noreg, killed $r0, $r2, $r8 :: (store (s32) into %stack.9), (store (s32) into %stack.8), (store (s32) into %stack.7)
-  ; CHECK:   $r12 = tMOVr killed $r2, 14 /* CC::al */, $noreg
-  ; CHECK:   renamable $r2 = tLDRspi $sp, 0, 14 /* CC::al */, $noreg :: (load (s32) from %stack.9)
-  ; CHECK:   tB %bb.2, 14 /* CC::al */, $noreg
-  ; CHECK: bb.1.bb74 (align 4):
-  ; CHECK:   successors: %bb.6(0x04000000), %bb.2(0x7c000000)
-  ; CHECK:   liveins: $r0, $r3, $r4, $r5, $r6, $r7, $r8, $r9, $r12, $r2
-  ; CHECK:   renamable $r7, dead $cpsr = nuw tADDi8 killed renamable $r7, 20, 14 /* CC::al */, $noreg
-  ; CHECK:   t2STRDi8 killed $r9, killed $r4, $r3, 0, 14 /* CC::al */, $noreg :: (store (s32) into %ir.i14), (store (s32) into %ir.i81)
-  ; CHECK:   t2STRDi8 killed $r6, killed $r0, $r3, 8, 14 /* CC::al */, $noreg :: (store (s32) into %ir.i84), (store (s32) into %ir.i88)
-  ; CHECK:   renamable $r3, dead $cpsr = nuw tADDi8 killed renamable $r3, 16, 14 /* CC::al */, $noreg
-  ; CHECK:   renamable $r5, $cpsr = tSUBi8 killed renamable $r5, 1, 14 /* CC::al */, $noreg
-  ; CHECK:   $r1 = tMOVr $r12, 14 /* CC::al */, $noreg
-  ; CHECK:   tBcc %bb.6, 0 /* CC::eq */, killed $cpsr
-  ; CHECK: bb.2.bb12:
-  ; CHECK:   successors: %bb.3(0x40000000), %bb.1(0x40000000)
-  ; CHECK:   liveins: $r1, $r2, $r3, $r5, $r7, $r8, $r12
-  ; CHECK:   $r9, $r4 = t2LDRDi8 $r3, 0, 14 /* CC::al */, $noreg :: (load (s32) from %ir.i14), (load (s32) from %ir.i20)
-  ; CHECK:   $r6, $r0 = t2LDRDi8 $r3, 8, 14 /* CC::al */, $noreg :: (load (s32) from %ir.i22), (load (s32) from %ir.i24)
-  ; CHECK:   dead $lr = t2SUBri renamable $r8, 0, 14 /* CC::al */, $noreg, def $cpsr
-  ; CHECK:   tBcc %bb.1, 0 /* CC::eq */, killed $cpsr
-  ; CHECK:   tB %bb.3, 14 /* CC::al */, $noreg
-  ; CHECK: bb.3.bb27:
-  ; CHECK:   successors: %bb.4(0x80000000)
-  ; CHECK:   liveins: $r0, $r1, $r2, $r3, $r4, $r5, $r6, $r7, $r8, $r9, $r12
-  ; CHECK:   t2STRDi8 killed $r3, killed $r5, $sp, 12, 14 /* CC::al */, $noreg :: (store (s32) into %stack.6), (store (s32) into %stack.5)
-  ; CHECK:   renamable $r3 = tLDRi renamable $r7, 0, 14 /* CC::al */, $noreg :: (load (s32) from %ir.i13)
-  ; CHECK:   tSTRspi killed renamable $r3, $sp, 9, 14 /* CC::al */, $noreg :: (store (s32) into %stack.0)
-  ; CHECK:   renamable $r3 = tLDRi renamable $r7, 1, 14 /* CC::al */, $noreg :: (load (s32) from %ir.i34)
-  ; CHECK:   tSTRspi killed renamable $r3, $sp, 8, 14 /* CC::al */, $noreg :: (store (s32) into %stack.1)
-  ; CHECK:   renamable $r3 = tLDRi renamable $r7, 2, 14 /* CC::al */, $noreg :: (load (s32) from %ir.i32)
-  ; CHECK:   tSTRspi killed renamable $r3, $sp, 7, 14 /* CC::al */, $noreg :: (store (s32) into %stack.2)
-  ; CHECK:   renamable $r3 = tLDRi renamable $r7, 3, 14 /* CC::al */, $noreg :: (load (s32) from %ir.i30)
-  ; CHECK:   t2STRDi8 $r7, killed $r3, $sp, 20, 14 /* CC::al */, $noreg :: (store (s32) into %stack.4), (store (s32) into %stack.3)
-  ; CHECK:   renamable $r10 = t2LDRi12 killed renamable $r7, 16, 14 /* CC::al */, $noreg :: (load (s32) from %ir.i28)
-  ; CHECK: bb.4.bb37 (align 4):
-  ; CHECK:   successors: %bb.4(0x7c000000), %bb.5(0x04000000)
-  ; CHECK:   liveins: $r0, $r1, $r2, $r4, $r6, $r8, $r9, $r10, $r12
-  ; CHECK:   $r7 = tMOVr killed $r6, 14 /* CC::al */, $noreg
-  ; CHECK:   renamable $r6 = tLDRspi $sp, 8, 14 /* CC::al */, $noreg :: (load (s32) from %stack.1)
-  ; CHECK:   renamable $r3 = tLDRspi $sp, 7, 14 /* CC::al */, $noreg :: (load (s32) from %stack.2)
-  ; CHECK:   renamable $r6, renamable $r11 = t2SMULL $r9, killed renamable $r6, 14 /* CC::al */, $noreg
-  ; CHECK:   renamable $r6, renamable $r11 = t2SMLAL killed renamable $r4, killed renamable $r3, killed renamable $r6, killed renamable $r11, 14 /* CC::al */, $noreg
-  ; CHECK:   renamable $r3 = tLDRspi $sp, 6, 14 /* CC::al */, $noreg :: (load (s32) from %stack.3)
-  ; CHECK:   $r5 = tMOVr killed $r9, 14 /* CC::al */, $noreg
-  ; CHECK:   renamable $r6, renamable $r11 = t2SMLAL renamable $r7, killed renamable $r3, killed renamable $r6, killed renamable $r11, 14 /* CC::al */, $noreg
-  ; CHECK:   renamable $r9, renamable $r1 = t2LDR_POST killed renamable $r1, 4, 14 /* CC::al */, $noreg :: (load (s32) from %ir.i38)
-  ; CHECK:   renamable $r6, renamable $r11 = t2SMLAL killed renamable $r0, renamable $r10, killed renamable $r6, killed renamable $r11, 14 /* CC::al */, $noreg
-  ; CHECK:   renamable $r0 = tLDRspi $sp, 9, 14 /* CC::al */, $noreg :: (load (s32) from %stack.0)
-  ; CHECK:   $lr = tMOVr $r8, 14 /* CC::al */, $noreg
-  ; CHECK:   renamable $r6, renamable $r11 = t2SMLAL renamable $r9, killed renamable $r0, killed renamable $r6, killed renamable $r11, 14 /* CC::al */, $noreg
-  ; CHECK:   early-clobber renamable $r6, dead early-clobber renamable $r11 = MVE_ASRLr killed renamable $r6, killed renamable $r11, renamable $r2, 14 /* CC::al */, $noreg
-  ; CHECK:   early-clobber renamable $r12 = t2STR_POST renamable $r6, killed renamable $r12, 4, 14 /* CC::al */, $noreg :: (store (s32) into %ir.i39)
-  ; CHECK:   dead renamable $lr = t2SUBri killed renamable $lr, 1, 14 /* CC::al */, $noreg, def $cpsr
-  ; CHECK:   renamable $r8 = t2SUBri killed renamable $r8, 1, 14 /* CC::al */, $noreg, $noreg
-  ; CHECK:   $r0 = tMOVr $r7, 14 /* CC::al */, $noreg
-  ; CHECK:   $r4 = tMOVr $r5, 14 /* CC::al */, $noreg
-  ; CHECK:   tBcc %bb.4, 1 /* CC::ne */, killed $cpsr
-  ; CHECK:   tB %bb.5, 14 /* CC::al */, $noreg
-  ; CHECK: bb.5.bb72:
-  ; CHECK:   successors: %bb.1(0x80000000)
-  ; CHECK:   liveins: $r2, $r5, $r6, $r7, $r9
-  ; CHECK:   $r0 = tMOVr killed $r7, 14 /* CC::al */, $noreg
-  ; CHECK:   $r7 = tADDrSPi $sp, 3, 14 /* CC::al */, $noreg
-  ; CHECK:   $r4 = tMOVr killed $r5, 14 /* CC::al */, $noreg
-  ; CHECK:   $r12, $r8 = t2LDRDi8 $sp, 4, 14 /* CC::al */, $noreg :: (load (s32) from %stack.8), (load (s32) from %stack.7)
-  ; CHECK:   tLDMIA killed $r7, 14 /* CC::al */, $noreg, def $r3, def $r5, def $r7 :: (load (s32) from %stack.6), (load (s32) from %stack.5), (load (s32) from %stack.4)
-  ; CHECK:   tB %bb.1, 14 /* CC::al */, $noreg
-  ; CHECK: bb.6.bb91:
-  ; CHECK:   $sp = frame-destroy tADDspi $sp, 10, 14 /* CC::al */, $noreg
-  ; CHECK:   $sp = frame-destroy t2LDMIA_RET $sp, 14 /* CC::al */, $noreg, def $r4, def $r5, def $r6, def $r7, def $r8, def $r9, def $r10, def $r11, def $pc
+  ; CHECK-NEXT:   successors: %bb.2(0x80000000)
+  ; CHECK-NEXT:   liveins: $lr, $r0, $r1, $r2, $r3, $r4, $r5, $r6, $r7, $r8, $r9, $r10, $r11
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   $sp = frame-setup t2STMDB_UPD $sp, 14 /* CC::al */, $noreg, kill...
[truncated]

@llvmbot
Copy link
Collaborator

llvmbot commented Jan 25, 2024

@llvm/pr-subscribers-backend-x86

Author: Oskar Wirga (oskarwirga)

Changes

Currently, the way that recomputeLiveIns works is that it will recompute the livein registers for that MachineBasicBlock but it matters what order you call recomputeLiveIn which can result in incorrect register allocations down the line.

This PR fixes that by simply recomputing the liveins for the entire CFG until convergence is achieved. This makes it harder to introduce subtle bugs which alter liveness.


Patch is 297.53 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/79498.diff

30 Files Affected:

  • (modified) llvm/include/llvm/CodeGen/LivePhysRegs.h (+23-2)
  • (modified) llvm/include/llvm/CodeGen/MachineBasicBlock.h (+6)
  • (modified) llvm/lib/CodeGen/BranchFolding.cpp (+1-2)
  • (modified) llvm/lib/Target/AArch64/AArch64FrameLowering.cpp (+1-2)
  • (modified) llvm/lib/Target/AArch64/AArch64InstrInfo.cpp (+1-3)
  • (modified) llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp (+1-6)
  • (modified) llvm/lib/Target/PowerPC/PPCExpandAtomicPseudoInsts.cpp (+2-5)
  • (modified) llvm/lib/Target/PowerPC/PPCFrameLowering.cpp (+2-4)
  • (modified) llvm/lib/Target/SystemZ/SystemZFrameLowering.cpp (+2-4)
  • (modified) llvm/lib/Target/X86/X86FrameLowering.cpp (+2-6)
  • (modified) llvm/test/CodeGen/AArch64/stack-probing-last-in-block.mir (+1-1)
  • (modified) llvm/test/CodeGen/SystemZ/branch-folder-hoist-livein.mir (+25-11)
  • (modified) llvm/test/CodeGen/Thumb2/LowOverheadLoops/biquad-cascade-default.mir (+102-90)
  • (modified) llvm/test/CodeGen/Thumb2/LowOverheadLoops/biquad-cascade-optsize-strd-lr.mir (+89-78)
  • (modified) llvm/test/CodeGen/Thumb2/LowOverheadLoops/biquad-cascade-optsize.mir (+96-87)
  • (modified) llvm/test/CodeGen/Thumb2/LowOverheadLoops/it-block-mov.mir (+87-74)
  • (modified) llvm/test/CodeGen/Thumb2/LowOverheadLoops/loop-dec-copy-chain.mir (+138-120)
  • (modified) llvm/test/CodeGen/Thumb2/LowOverheadLoops/loop-dec-copy-prev-iteration.mir (+155-130)
  • (modified) llvm/test/CodeGen/Thumb2/LowOverheadLoops/loop-dec-liveout.mir (+154-129)
  • (modified) llvm/test/CodeGen/Thumb2/LowOverheadLoops/matrix-debug.mir (+80-70)
  • (modified) llvm/test/CodeGen/Thumb2/LowOverheadLoops/matrix.mir (+173-144)
  • (modified) llvm/test/CodeGen/Thumb2/LowOverheadLoops/mov-after-dlstp.mir (+58-56)
  • (modified) llvm/test/CodeGen/Thumb2/LowOverheadLoops/multi-block-cond-iter-count.mir (+132-112)
  • (modified) llvm/test/CodeGen/Thumb2/LowOverheadLoops/multiple-do-loops.mir (+233-193)
  • (modified) llvm/test/CodeGen/Thumb2/LowOverheadLoops/predicated-liveout.mir (+38-29)
  • (modified) llvm/test/CodeGen/Thumb2/LowOverheadLoops/remove-elem-moves.mir (+97-79)
  • (modified) llvm/test/CodeGen/Thumb2/LowOverheadLoops/skip-debug.mir (+65-56)
  • (modified) llvm/test/CodeGen/Thumb2/LowOverheadLoops/skip-vpt-debug.mir (+71-68)
  • (modified) llvm/test/CodeGen/Thumb2/LowOverheadLoops/spillingmove.mir (+7-7)
  • (modified) llvm/test/CodeGen/Thumb2/LowOverheadLoops/unrolled-and-vector.mir (+154-130)
diff --git a/llvm/include/llvm/CodeGen/LivePhysRegs.h b/llvm/include/llvm/CodeGen/LivePhysRegs.h
index 76bb34d270a26dc..1e0ee9eb9eb3203 100644
--- a/llvm/include/llvm/CodeGen/LivePhysRegs.h
+++ b/llvm/include/llvm/CodeGen/LivePhysRegs.h
@@ -31,6 +31,7 @@
 
 #include "llvm/ADT/SparseSet.h"
 #include "llvm/CodeGen/MachineBasicBlock.h"
+#include "llvm/CodeGen/MachineFunction.h"
 #include "llvm/CodeGen/TargetRegisterInfo.h"
 #include "llvm/MC/MCRegister.h"
 #include "llvm/MC/MCRegisterInfo.h"
@@ -193,11 +194,31 @@ void addLiveIns(MachineBasicBlock &MBB, const LivePhysRegs &LiveRegs);
 void computeAndAddLiveIns(LivePhysRegs &LiveRegs,
                           MachineBasicBlock &MBB);
 
-/// Convenience function for recomputing live-in's for \p MBB.
-static inline void recomputeLiveIns(MachineBasicBlock &MBB) {
+/// Function to update the live-in's for a basic block and return whether any
+/// changes were made.
+static inline bool updateBlockLiveInfo(MachineBasicBlock &MBB) {
   LivePhysRegs LPR;
+  auto oldLiveIns = MBB.getLiveIns();
+
   MBB.clearLiveIns();
   computeAndAddLiveIns(LPR, MBB);
+  MBB.sortUniqueLiveIns();
+
+  auto newLiveIns = MBB.getLiveIns();
+  return oldLiveIns != newLiveIns;
+}
+
+/// Convenience function for recomputing live-in's for the entire CFG until
+/// convergence is reached.
+static inline void recomputeLiveIns(MachineFunction &MF) {
+  bool anyChanged;
+  do {
+    anyChanged = false;
+    for (auto MFI = MF.rbegin(), MFE = MF.rend(); MFI != MFE; ++MFI) {
+      MachineBasicBlock &MBB = *MFI;
+      anyChanged |= updateBlockLiveInfo(MBB);
+    }
+  } while (anyChanged);
 }
 
 } // end namespace llvm
diff --git a/llvm/include/llvm/CodeGen/MachineBasicBlock.h b/llvm/include/llvm/CodeGen/MachineBasicBlock.h
index c84fd281c6a549b..dc2035fa598c46d 100644
--- a/llvm/include/llvm/CodeGen/MachineBasicBlock.h
+++ b/llvm/include/llvm/CodeGen/MachineBasicBlock.h
@@ -111,6 +111,10 @@ class MachineBasicBlock
 
     RegisterMaskPair(MCPhysReg PhysReg, LaneBitmask LaneMask)
         : PhysReg(PhysReg), LaneMask(LaneMask) {}
+
+    bool operator==(const RegisterMaskPair &other) const {
+      return PhysReg == other.PhysReg && LaneMask == other.LaneMask;
+    }
   };
 
 private:
@@ -473,6 +477,8 @@ class MachineBasicBlock
   /// Remove entry from the livein set and return iterator to the next.
   livein_iterator removeLiveIn(livein_iterator I);
 
+  std::vector<RegisterMaskPair> getLiveIns() const { return LiveIns; }
+
   class liveout_iterator {
   public:
     using iterator_category = std::input_iterator_tag;
diff --git a/llvm/lib/CodeGen/BranchFolding.cpp b/llvm/lib/CodeGen/BranchFolding.cpp
index a9f78358e57b929..9075fa436da58cb 100644
--- a/llvm/lib/CodeGen/BranchFolding.cpp
+++ b/llvm/lib/CodeGen/BranchFolding.cpp
@@ -2048,8 +2048,7 @@ bool BranchFolder::HoistCommonCodeInSuccs(MachineBasicBlock *MBB) {
   FBB->erase(FBB->begin(), FIB);
 
   if (UpdateLiveIns) {
-    recomputeLiveIns(*TBB);
-    recomputeLiveIns(*FBB);
+    recomputeLiveIns(*MBB->getParent());
   }
 
   ++NumHoist;
diff --git a/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp b/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
index cffd414221c30cf..785f563faf4bb1a 100644
--- a/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
+++ b/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
@@ -4339,8 +4339,7 @@ AArch64FrameLowering::inlineStackProbeLoopExactMultiple(
   ExitMBB->transferSuccessorsAndUpdatePHIs(&MBB);
   MBB.addSuccessor(LoopMBB);
   // Update liveins.
-  recomputeLiveIns(*LoopMBB);
-  recomputeLiveIns(*ExitMBB);
+  recomputeLiveIns(MF);
 
   return ExitMBB->begin();
 }
diff --git a/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp b/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
index 2e8d8c63d6bec24..72722aa95b846e3 100644
--- a/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
+++ b/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
@@ -9597,9 +9597,7 @@ AArch64InstrInfo::probedStackAlloc(MachineBasicBlock::iterator MBBI,
 
   // Update liveins.
   if (MF.getRegInfo().reservedRegsFrozen()) {
-    recomputeLiveIns(*LoopTestMBB);
-    recomputeLiveIns(*LoopBodyMBB);
-    recomputeLiveIns(*ExitMBB);
+    recomputeLiveIns(MF);
   }
 
   return ExitMBB->begin();
diff --git a/llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp b/llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp
index 5c1c7046fdbff09..e199a21da500a17 100644
--- a/llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp
+++ b/llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp
@@ -1806,12 +1806,7 @@ void ARMLowOverheadLoops::Expand(LowOverheadLoop &LoLoop) {
   PostOrderLoopTraversal DFS(LoLoop.ML, *MLI);
   DFS.ProcessLoop();
   const SmallVectorImpl<MachineBasicBlock*> &PostOrder = DFS.getOrder();
-  for (auto *MBB : PostOrder) {
-    recomputeLiveIns(*MBB);
-    // FIXME: For some reason, the live-in print order is non-deterministic for
-    // our tests and I can't out why... So just sort them.
-    MBB->sortUniqueLiveIns();
-  }
+  recomputeLiveIns(*LoLoop.MF);
 
   for (auto *MBB : reverse(PostOrder))
     recomputeLivenessFlags(*MBB);
diff --git a/llvm/lib/Target/PowerPC/PPCExpandAtomicPseudoInsts.cpp b/llvm/lib/Target/PowerPC/PPCExpandAtomicPseudoInsts.cpp
index aee57a5075ff719..c170bafe9b184c9 100644
--- a/llvm/lib/Target/PowerPC/PPCExpandAtomicPseudoInsts.cpp
+++ b/llvm/lib/Target/PowerPC/PPCExpandAtomicPseudoInsts.cpp
@@ -208,8 +208,7 @@ bool PPCExpandAtomicPseudo::expandAtomicRMW128(
       .addMBB(LoopMBB);
   CurrentMBB->addSuccessor(LoopMBB);
   CurrentMBB->addSuccessor(ExitMBB);
-  recomputeLiveIns(*LoopMBB);
-  recomputeLiveIns(*ExitMBB);
+  recomputeLiveIns(*MF);
   NMBBI = MBB.end();
   MI.eraseFromParent();
   return true;
@@ -286,9 +285,7 @@ bool PPCExpandAtomicPseudo::expandAtomicCmpSwap128(
   CurrentMBB->addSuccessor(LoopCmpMBB);
   CurrentMBB->addSuccessor(ExitMBB);
 
-  recomputeLiveIns(*LoopCmpMBB);
-  recomputeLiveIns(*CmpSuccMBB);
-  recomputeLiveIns(*ExitMBB);
+  recomputeLiveIns(*MF);
   NMBBI = MBB.end();
   MI.eraseFromParent();
   return true;
diff --git a/llvm/lib/Target/PowerPC/PPCFrameLowering.cpp b/llvm/lib/Target/PowerPC/PPCFrameLowering.cpp
index 245e78641ed6544..f67658302958c3b 100644
--- a/llvm/lib/Target/PowerPC/PPCFrameLowering.cpp
+++ b/llvm/lib/Target/PowerPC/PPCFrameLowering.cpp
@@ -1441,8 +1441,7 @@ void PPCFrameLowering::inlineStackProbe(MachineFunction &MF,
       ProbeLoopBodyMBB->addSuccessor(ProbeLoopBodyMBB);
     }
     // Update liveins.
-    recomputeLiveIns(*ProbeLoopBodyMBB);
-    recomputeLiveIns(*ProbeExitMBB);
+    recomputeLiveIns(MF);
     return ProbeExitMBB;
   };
   // For case HasBP && MaxAlign > 1, we have to realign the SP by performing
@@ -1534,8 +1533,7 @@ void PPCFrameLowering::inlineStackProbe(MachineFunction &MF,
         buildDefCFAReg(*ExitMBB, ExitMBB->begin(), SPReg);
       }
       // Update liveins.
-      recomputeLiveIns(*LoopMBB);
-      recomputeLiveIns(*ExitMBB);
+      recomputeLiveIns(MF);
     }
   }
   ++NumPrologProbed;
diff --git a/llvm/lib/Target/SystemZ/SystemZFrameLowering.cpp b/llvm/lib/Target/SystemZ/SystemZFrameLowering.cpp
index db19c8881c685a5..965688dec69b252 100644
--- a/llvm/lib/Target/SystemZ/SystemZFrameLowering.cpp
+++ b/llvm/lib/Target/SystemZ/SystemZFrameLowering.cpp
@@ -840,8 +840,7 @@ void SystemZELFFrameLowering::inlineStackProbe(
   StackAllocMI->eraseFromParent();
   if (DoneMBB != nullptr) {
     // Compute the live-in lists for the new blocks.
-    recomputeLiveIns(*DoneMBB);
-    recomputeLiveIns(*LoopMBB);
+    recomputeLiveIns(MF);
   }
 }
 
@@ -1439,8 +1438,7 @@ void SystemZXPLINKFrameLowering::inlineStackProbe(
   StackAllocMI->eraseFromParent();
 
   // Compute the live-in lists for the new blocks.
-  recomputeLiveIns(*NextMBB);
-  recomputeLiveIns(*StackExtMBB);
+  recomputeLiveIns(MF);
 }
 
 bool SystemZXPLINKFrameLowering::hasFP(const MachineFunction &MF) const {
diff --git a/llvm/lib/Target/X86/X86FrameLowering.cpp b/llvm/lib/Target/X86/X86FrameLowering.cpp
index c0d358ead2787b2..4aa27be07913141 100644
--- a/llvm/lib/Target/X86/X86FrameLowering.cpp
+++ b/llvm/lib/Target/X86/X86FrameLowering.cpp
@@ -885,8 +885,7 @@ void X86FrameLowering::emitStackProbeInlineGenericLoop(
   }
 
   // Update Live In information
-  recomputeLiveIns(*testMBB);
-  recomputeLiveIns(*tailMBB);
+  recomputeLiveIns(MF);
 }
 
 void X86FrameLowering::emitStackProbeInlineWindowsCoreCLR64(
@@ -1378,10 +1377,7 @@ void X86FrameLowering::BuildStackAlignAND(MachineBasicBlock &MBB,
         footMBB->addSuccessor(&MBB);
       }
 
-      recomputeLiveIns(*headMBB);
-      recomputeLiveIns(*bodyMBB);
-      recomputeLiveIns(*footMBB);
-      recomputeLiveIns(MBB);
+      recomputeLiveIns(MF);
     }
   } else {
     MachineInstr *MI = BuildMI(MBB, MBBI, DL, TII.get(AndOp), Reg)
diff --git a/llvm/test/CodeGen/AArch64/stack-probing-last-in-block.mir b/llvm/test/CodeGen/AArch64/stack-probing-last-in-block.mir
index 6c8ec7e4c4fa924..8d7fab4cc39c660 100644
--- a/llvm/test/CodeGen/AArch64/stack-probing-last-in-block.mir
+++ b/llvm/test/CodeGen/AArch64/stack-probing-last-in-block.mir
@@ -82,7 +82,7 @@ body:             |
   ; CHECK-LABEL: name: f
   ; CHECK: bb.0.entry:
   ; CHECK-NEXT:   successors: %bb.3(0x80000000)
-  ; CHECK-NEXT:   liveins: $lr, $fp
+  ; CHECK-NEXT:   liveins: $fp, $lr
   ; CHECK-NEXT: {{  $}}
   ; CHECK-NEXT:   early-clobber $sp = frame-setup STPXpre killed $fp, killed $lr, $sp, -2 :: (store (s64) into %stack.2), (store (s64) into %stack.1)
   ; CHECK-NEXT:   frame-setup CFI_INSTRUCTION def_cfa_offset 16
diff --git a/llvm/test/CodeGen/SystemZ/branch-folder-hoist-livein.mir b/llvm/test/CodeGen/SystemZ/branch-folder-hoist-livein.mir
index 5e100b88ead300f..82e3bae97ec0c48 100644
--- a/llvm/test/CodeGen/SystemZ/branch-folder-hoist-livein.mir
+++ b/llvm/test/CodeGen/SystemZ/branch-folder-hoist-livein.mir
@@ -1,3 +1,4 @@
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 4
 # RUN: llc -verify-machineinstrs -O1 -mtriple=s390x-ibm-linux -o - %s -run-pass=branch-folder | FileCheck %s
 --- |
   target datalayout = "E-m:e-i1:8:16-i8:8:16-i64:64-f128:64-a:8:16-n32:64"
@@ -15,6 +16,30 @@
 name:            f1
 tracksRegLiveness: true
 body:             |
+  ; CHECK-LABEL: name: f1
+  ; CHECK: bb.0:
+  ; CHECK-NEXT:   successors: %bb.2(0x7fffffff), %bb.1(0x00000001)
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   renamable $r1d = LGRL @b :: (load (s32) from got, align 8)
+  ; CHECK-NEXT:   renamable $r1l = LH killed renamable $r1d, 0, $noreg, implicit-def $r1d :: (dereferenceable load (s8) from @b)
+  ; CHECK-NEXT:   renamable $r2l = LHI 0
+  ; CHECK-NEXT:   renamable $r3d = LGRL @d :: (load (s32) from got, align 8)
+  ; CHECK-NEXT:   renamable $r4d = LLILL 0, implicit-def $r4q
+  ; CHECK-NEXT:   renamable $r4d = COPY killed renamable $r4d, implicit killed $r4q
+  ; CHECK-NEXT:   CHI killed renamable $r2l, 0, implicit-def $cc
+  ; CHECK-NEXT:   BRC 14, 6, %bb.2, implicit killed $cc
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.1:
+  ; CHECK-NEXT:   successors:
+  ; CHECK-NEXT:   liveins: $r3d, $r4d, $r1l
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   STH renamable $r1l, killed renamable $r3d, 0, $noreg, implicit killed $r4d :: (store (s8) into @d)
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.2:
+  ; CHECK-NEXT:   liveins: $r3d, $r4d, $r1l
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   STH renamable $r1l, killed renamable $r3d, 0, $noreg, implicit killed $r4d :: (store (s8) into @d)
+  ; CHECK-NEXT:   Return
   bb.0:
     successors: %bb.2(0x7fffffff), %bb.1(0x00000001)
     liveins:
@@ -44,14 +69,3 @@ body:             |
     Return
 
 ...
-
-# CHECK: renamable $r4d = COPY killed renamable $r4d, implicit killed $r4q
-# CHECK-NEXT: CHI killed renamable $r2l, 0, implicit-def $cc
-# CHECK-NEXT: BRC 14, 6, %bb.2, implicit killed $cc
-# CHECK-NEXT: {{^  $}}
-# CHECK-NEXT: bb.1:
-# CHECK-NEXT: successors:
-# CHECK-NEXT: liveins: $r1l, $r3d, $r4d
-
-# CHECK: bb.2:
-# CHECK-NEXT: liveins: $r1l, $r3d, $r4d
diff --git a/llvm/test/CodeGen/Thumb2/LowOverheadLoops/biquad-cascade-default.mir b/llvm/test/CodeGen/Thumb2/LowOverheadLoops/biquad-cascade-default.mir
index dabebf4aeb77a81..d9d3fd571ab8de1 100644
--- a/llvm/test/CodeGen/Thumb2/LowOverheadLoops/biquad-cascade-default.mir
+++ b/llvm/test/CodeGen/Thumb2/LowOverheadLoops/biquad-cascade-default.mir
@@ -200,96 +200,108 @@ machineFunctionInfo: {}
 body:             |
   ; CHECK-LABEL: name: arm_biquad_cascade_df1_q31
   ; CHECK: bb.0.bb:
-  ; CHECK:   successors: %bb.2(0x80000000)
-  ; CHECK:   liveins: $r0, $r1, $r2, $r3, $r4, $r5, $r6, $r7, $r8, $r9, $r10, $r11, $lr
-  ; CHECK:   $sp = frame-setup t2STMDB_UPD $sp, 14 /* CC::al */, $noreg, killed $r4, killed $r5, killed $r6, killed $r7, killed $r8, killed $r9, killed $r10, killed $r11, killed $lr
-  ; CHECK:   frame-setup CFI_INSTRUCTION def_cfa_offset 36
-  ; CHECK:   frame-setup CFI_INSTRUCTION offset $lr, -4
-  ; CHECK:   frame-setup CFI_INSTRUCTION offset $r11, -8
-  ; CHECK:   frame-setup CFI_INSTRUCTION offset $r10, -12
-  ; CHECK:   frame-setup CFI_INSTRUCTION offset $r9, -16
-  ; CHECK:   frame-setup CFI_INSTRUCTION offset $r8, -20
-  ; CHECK:   frame-setup CFI_INSTRUCTION offset $r7, -24
-  ; CHECK:   frame-setup CFI_INSTRUCTION offset $r6, -28
-  ; CHECK:   frame-setup CFI_INSTRUCTION offset $r5, -32
-  ; CHECK:   frame-setup CFI_INSTRUCTION offset $r4, -36
-  ; CHECK:   $sp = frame-setup tSUBspi $sp, 10, 14 /* CC::al */, $noreg
-  ; CHECK:   frame-setup CFI_INSTRUCTION def_cfa_offset 76
-  ; CHECK:   $r6, $r5 = t2LDRDi8 $r0, 8, 14 /* CC::al */, $noreg :: (load (s32) from %ir.i7), (load (s32) from %ir.i10)
-  ; CHECK:   $r8 = tMOVr killed $r3, 14 /* CC::al */, $noreg
-  ; CHECK:   $r3, $r7 = t2LDRDi8 killed $r0, 0, 14 /* CC::al */, $noreg :: (load (s32) from %ir.i), (load (s32) from %ir.i5)
-  ; CHECK:   renamable $r0 = t2RSBri killed renamable $r6, 31, 14 /* CC::al */, $noreg, $noreg
-  ; CHECK:   t2STMIA $sp, 14 /* CC::al */, $noreg, killed $r0, $r2, $r8 :: (store (s32) into %stack.9), (store (s32) into %stack.8), (store (s32) into %stack.7)
-  ; CHECK:   $r12 = tMOVr killed $r2, 14 /* CC::al */, $noreg
-  ; CHECK:   renamable $r2 = tLDRspi $sp, 0, 14 /* CC::al */, $noreg :: (load (s32) from %stack.9)
-  ; CHECK:   tB %bb.2, 14 /* CC::al */, $noreg
-  ; CHECK: bb.1.bb74 (align 4):
-  ; CHECK:   successors: %bb.6(0x04000000), %bb.2(0x7c000000)
-  ; CHECK:   liveins: $r0, $r3, $r4, $r5, $r6, $r7, $r8, $r9, $r12, $r2
-  ; CHECK:   renamable $r7, dead $cpsr = nuw tADDi8 killed renamable $r7, 20, 14 /* CC::al */, $noreg
-  ; CHECK:   t2STRDi8 killed $r9, killed $r4, $r3, 0, 14 /* CC::al */, $noreg :: (store (s32) into %ir.i14), (store (s32) into %ir.i81)
-  ; CHECK:   t2STRDi8 killed $r6, killed $r0, $r3, 8, 14 /* CC::al */, $noreg :: (store (s32) into %ir.i84), (store (s32) into %ir.i88)
-  ; CHECK:   renamable $r3, dead $cpsr = nuw tADDi8 killed renamable $r3, 16, 14 /* CC::al */, $noreg
-  ; CHECK:   renamable $r5, $cpsr = tSUBi8 killed renamable $r5, 1, 14 /* CC::al */, $noreg
-  ; CHECK:   $r1 = tMOVr $r12, 14 /* CC::al */, $noreg
-  ; CHECK:   tBcc %bb.6, 0 /* CC::eq */, killed $cpsr
-  ; CHECK: bb.2.bb12:
-  ; CHECK:   successors: %bb.3(0x40000000), %bb.1(0x40000000)
-  ; CHECK:   liveins: $r1, $r2, $r3, $r5, $r7, $r8, $r12
-  ; CHECK:   $r9, $r4 = t2LDRDi8 $r3, 0, 14 /* CC::al */, $noreg :: (load (s32) from %ir.i14), (load (s32) from %ir.i20)
-  ; CHECK:   $r6, $r0 = t2LDRDi8 $r3, 8, 14 /* CC::al */, $noreg :: (load (s32) from %ir.i22), (load (s32) from %ir.i24)
-  ; CHECK:   dead $lr = t2SUBri renamable $r8, 0, 14 /* CC::al */, $noreg, def $cpsr
-  ; CHECK:   tBcc %bb.1, 0 /* CC::eq */, killed $cpsr
-  ; CHECK:   tB %bb.3, 14 /* CC::al */, $noreg
-  ; CHECK: bb.3.bb27:
-  ; CHECK:   successors: %bb.4(0x80000000)
-  ; CHECK:   liveins: $r0, $r1, $r2, $r3, $r4, $r5, $r6, $r7, $r8, $r9, $r12
-  ; CHECK:   t2STRDi8 killed $r3, killed $r5, $sp, 12, 14 /* CC::al */, $noreg :: (store (s32) into %stack.6), (store (s32) into %stack.5)
-  ; CHECK:   renamable $r3 = tLDRi renamable $r7, 0, 14 /* CC::al */, $noreg :: (load (s32) from %ir.i13)
-  ; CHECK:   tSTRspi killed renamable $r3, $sp, 9, 14 /* CC::al */, $noreg :: (store (s32) into %stack.0)
-  ; CHECK:   renamable $r3 = tLDRi renamable $r7, 1, 14 /* CC::al */, $noreg :: (load (s32) from %ir.i34)
-  ; CHECK:   tSTRspi killed renamable $r3, $sp, 8, 14 /* CC::al */, $noreg :: (store (s32) into %stack.1)
-  ; CHECK:   renamable $r3 = tLDRi renamable $r7, 2, 14 /* CC::al */, $noreg :: (load (s32) from %ir.i32)
-  ; CHECK:   tSTRspi killed renamable $r3, $sp, 7, 14 /* CC::al */, $noreg :: (store (s32) into %stack.2)
-  ; CHECK:   renamable $r3 = tLDRi renamable $r7, 3, 14 /* CC::al */, $noreg :: (load (s32) from %ir.i30)
-  ; CHECK:   t2STRDi8 $r7, killed $r3, $sp, 20, 14 /* CC::al */, $noreg :: (store (s32) into %stack.4), (store (s32) into %stack.3)
-  ; CHECK:   renamable $r10 = t2LDRi12 killed renamable $r7, 16, 14 /* CC::al */, $noreg :: (load (s32) from %ir.i28)
-  ; CHECK: bb.4.bb37 (align 4):
-  ; CHECK:   successors: %bb.4(0x7c000000), %bb.5(0x04000000)
-  ; CHECK:   liveins: $r0, $r1, $r2, $r4, $r6, $r8, $r9, $r10, $r12
-  ; CHECK:   $r7 = tMOVr killed $r6, 14 /* CC::al */, $noreg
-  ; CHECK:   renamable $r6 = tLDRspi $sp, 8, 14 /* CC::al */, $noreg :: (load (s32) from %stack.1)
-  ; CHECK:   renamable $r3 = tLDRspi $sp, 7, 14 /* CC::al */, $noreg :: (load (s32) from %stack.2)
-  ; CHECK:   renamable $r6, renamable $r11 = t2SMULL $r9, killed renamable $r6, 14 /* CC::al */, $noreg
-  ; CHECK:   renamable $r6, renamable $r11 = t2SMLAL killed renamable $r4, killed renamable $r3, killed renamable $r6, killed renamable $r11, 14 /* CC::al */, $noreg
-  ; CHECK:   renamable $r3 = tLDRspi $sp, 6, 14 /* CC::al */, $noreg :: (load (s32) from %stack.3)
-  ; CHECK:   $r5 = tMOVr killed $r9, 14 /* CC::al */, $noreg
-  ; CHECK:   renamable $r6, renamable $r11 = t2SMLAL renamable $r7, killed renamable $r3, killed renamable $r6, killed renamable $r11, 14 /* CC::al */, $noreg
-  ; CHECK:   renamable $r9, renamable $r1 = t2LDR_POST killed renamable $r1, 4, 14 /* CC::al */, $noreg :: (load (s32) from %ir.i38)
-  ; CHECK:   renamable $r6, renamable $r11 = t2SMLAL killed renamable $r0, renamable $r10, killed renamable $r6, killed renamable $r11, 14 /* CC::al */, $noreg
-  ; CHECK:   renamable $r0 = tLDRspi $sp, 9, 14 /* CC::al */, $noreg :: (load (s32) from %stack.0)
-  ; CHECK:   $lr = tMOVr $r8, 14 /* CC::al */, $noreg
-  ; CHECK:   renamable $r6, renamable $r11 = t2SMLAL renamable $r9, killed renamable $r0, killed renamable $r6, killed renamable $r11, 14 /* CC::al */, $noreg
-  ; CHECK:   early-clobber renamable $r6, dead early-clobber renamable $r11 = MVE_ASRLr killed renamable $r6, killed renamable $r11, renamable $r2, 14 /* CC::al */, $noreg
-  ; CHECK:   early-clobber renamable $r12 = t2STR_POST renamable $r6, killed renamable $r12, 4, 14 /* CC::al */, $noreg :: (store (s32) into %ir.i39)
-  ; CHECK:   dead renamable $lr = t2SUBri killed renamable $lr, 1, 14 /* CC::al */, $noreg, def $cpsr
-  ; CHECK:   renamable $r8 = t2SUBri killed renamable $r8, 1, 14 /* CC::al */, $noreg, $noreg
-  ; CHECK:   $r0 = tMOVr $r7, 14 /* CC::al */, $noreg
-  ; CHECK:   $r4 = tMOVr $r5, 14 /* CC::al */, $noreg
-  ; CHECK:   tBcc %bb.4, 1 /* CC::ne */, killed $cpsr
-  ; CHECK:   tB %bb.5, 14 /* CC::al */, $noreg
-  ; CHECK: bb.5.bb72:
-  ; CHECK:   successors: %bb.1(0x80000000)
-  ; CHECK:   liveins: $r2, $r5, $r6, $r7, $r9
-  ; CHECK:   $r0 = tMOVr killed $r7, 14 /* CC::al */, $noreg
-  ; CHECK:   $r7 = tADDrSPi $sp, 3, 14 /* CC::al */, $noreg
-  ; CHECK:   $r4 = tMOVr killed $r5, 14 /* CC::al */, $noreg
-  ; CHECK:   $r12, $r8 = t2LDRDi8 $sp, 4, 14 /* CC::al */, $noreg :: (load (s32) from %stack.8), (load (s32) from %stack.7)
-  ; CHECK:   tLDMIA killed $r7, 14 /* CC::al */, $noreg, def $r3, def $r5, def $r7 :: (load (s32) from %stack.6), (load (s32) from %stack.5), (load (s32) from %stack.4)
-  ; CHECK:   tB %bb.1, 14 /* CC::al */, $noreg
-  ; CHECK: bb.6.bb91:
-  ; CHECK:   $sp = frame-destroy tADDspi $sp, 10, 14 /* CC::al */, $noreg
-  ; CHECK:   $sp = frame-destroy t2LDMIA_RET $sp, 14 /* CC::al */, $noreg, def $r4, def $r5, def $r6, def $r7, def $r8, def $r9, def $r10, def $r11, def $pc
+  ; CHECK-NEXT:   successors: %bb.2(0x80000000)
+  ; CHECK-NEXT:   liveins: $lr, $r0, $r1, $r2, $r3, $r4, $r5, $r6, $r7, $r8, $r9, $r10, $r11
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   $sp = frame-setup t2STMDB_UPD $sp, 14 /* CC::al */, $noreg, kill...
[truncated]

@oskarwirga oskarwirga merged commit 59bf605 into llvm:main Jan 26, 2024
4 checks passed
oskarwirga added a commit to oskarwirga/llvm-project that referenced this pull request Jan 26, 2024
Currently, the way that recomputeLiveIns works is that it will recompute
the livein registers for that MachineBasicBlock but it matters what
order you call recomputeLiveIn which can result in incorrect register
allocations down the line.

This PR fixes that by simply recomputing the liveins for the entire CFG
until convergence is achieved. This makes it harder to introduce subtle
bugs which alter liveness.

(cherry picked from commit 59bf605)
nikic added a commit that referenced this pull request Jan 26, 2024
This reverts commit 59bf605.

Introduces a major compile-time regression.
@nikic
Copy link
Contributor

nikic commented Jan 26, 2024

oskarwirga added a commit that referenced this pull request Jan 31, 2024
…9940)

This is a fix for the regression seen in
#79498

> Currently, the way that recomputeLiveIns works is that it will
recompute the livein registers for that MachineBasicBlock but it matters
what order you call recomputeLiveIn which can result in incorrect
register allocations down the line.

Now we do not recompute the entire CFG but we do ensure that the newly
added MBB do reach convergence.
oskarwirga added a commit to oskarwirga/llvm-project that referenced this pull request Jan 31, 2024
…vm#79940)

This is a fix for the regression seen in
llvm#79498

> Currently, the way that recomputeLiveIns works is that it will
recompute the livein registers for that MachineBasicBlock but it matters
what order you call recomputeLiveIn which can result in incorrect
register allocations down the line.

Now we do not recompute the entire CFG but we do ensure that the newly
added MBB do reach convergence.

(cherry picked from commit ff4636a)
tstellar pushed a commit that referenced this pull request Feb 7, 2024
Currently, the way that recomputeLiveIns works is that it will recompute
the livein registers for that MachineBasicBlock but it matters what
order you call recomputeLiveIn which can result in incorrect register
allocations down the line.

Now we do not recompute the entire CFG but we do ensure that the newly
added MBB do reach convergence. This fixes a register allocation bug
introduced in AArch64 stack probing.

(cherry picked from commit ff4636a)
tstellar pushed a commit to tstellar/llvm-project that referenced this pull request Feb 14, 2024
…79641)

Currently, the way that recomputeLiveIns works is that it will recompute
the livein registers for that MachineBasicBlock but it matters what
order you call recomputeLiveIn which can result in incorrect register
allocations down the line.

Now we do not recompute the entire CFG but we do ensure that the newly
added MBB do reach convergence. This fixes a register allocation bug
introduced in AArch64 stack probing.

(cherry picked from commit ff4636a)
tstellar pushed a commit to tstellar/llvm-project that referenced this pull request Feb 14, 2024
…79641)

Currently, the way that recomputeLiveIns works is that it will recompute
the livein registers for that MachineBasicBlock but it matters what
order you call recomputeLiveIn which can result in incorrect register
allocations down the line.

Now we do not recompute the entire CFG but we do ensure that the newly
added MBB do reach convergence. This fixes a register allocation bug
introduced in AArch64 stack probing.

(cherry picked from commit ff4636a)
tstellar pushed a commit to tstellar/llvm-project that referenced this pull request Feb 14, 2024
…79641)

Currently, the way that recomputeLiveIns works is that it will recompute
the livein registers for that MachineBasicBlock but it matters what
order you call recomputeLiveIn which can result in incorrect register
allocations down the line.

Now we do not recompute the entire CFG but we do ensure that the newly
added MBB do reach convergence. This fixes a register allocation bug
introduced in AArch64 stack probing.

(cherry picked from commit ff4636a)
tstellar pushed a commit to tstellar/llvm-project that referenced this pull request Feb 14, 2024
…79641)

Currently, the way that recomputeLiveIns works is that it will recompute
the livein registers for that MachineBasicBlock but it matters what
order you call recomputeLiveIn which can result in incorrect register
allocations down the line.

Now we do not recompute the entire CFG but we do ensure that the newly
added MBB do reach convergence. This fixes a register allocation bug
introduced in AArch64 stack probing.

(cherry picked from commit ff4636a)
@pointhex pointhex mentioned this pull request May 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants