[llvm] [RDA] Fix getGlobalUses() to account for redefinitions (PR #78178)

Dmitry Borisenkov via llvm-commits llvm-commits at lists.llvm.org
Mon Jan 15 07:57:22 PST 2024


https://github.com/akiramenai created https://github.com/llvm/llvm-project/pull/78178

Prior to the patch getGlobalUses() continued to look for uses in a successor BB if the register of interest is alive in the successor, but it didn't check whether the BB redefined the register. The patch adds this check, so getGlobalUses() only proceed with successors if the register is not redefined.

-----
I discovered the issue in a downstream project and it looks like LLVM only have a single call site of `getGlobalUses()` in a `ARM/ARMLowOverheadLoops.cpp` (that's probably why it remained unfixed for that long). I'm not very proficient in ARM, so I'd appreciate if somebody can suggest how to add a test for the patch.

>From a325f9acf33d901698db3b7ebea0cd69d0403658 Mon Sep 17 00:00:00 2001
From: Dmitry Borisenkov <dmitriy.borisenkov89 at gmail.com>
Date: Mon, 15 Jan 2024 17:39:56 +0200
Subject: [PATCH] [RDA] Fix getGlobalUses() to account for redefinitions

Prior to the patch getGlobalUses() continued to look for uses in
a successor BB if the register of interest is alive in the successor,
but it didn't check whether the BB redefined the register.
The patch adds this check, so getGlobalUses() only proceed with successors
if the register is not redefined.
---
 llvm/lib/CodeGen/ReachingDefAnalysis.cpp | 30 +++++++++++++-----------
 1 file changed, 16 insertions(+), 14 deletions(-)

diff --git a/llvm/lib/CodeGen/ReachingDefAnalysis.cpp b/llvm/lib/CodeGen/ReachingDefAnalysis.cpp
index 61a668907be77d..44a88c1711a6d7 100644
--- a/llvm/lib/CodeGen/ReachingDefAnalysis.cpp
+++ b/llvm/lib/CodeGen/ReachingDefAnalysis.cpp
@@ -378,21 +378,23 @@ void ReachingDefAnalysis::getGlobalUses(MachineInstr *MI, MCRegister PhysReg,
   // Collect the uses that each def touches within the block.
   getReachingLocalUses(MI, PhysReg, Uses);
 
-  // Handle live-out values.
-  if (auto *LiveOut = getLocalLiveOutMIDef(MI->getParent(), PhysReg)) {
-    if (LiveOut != MI)
-      return;
+  // If the definition doesn't live-out stop search.
+  if (getLocalLiveOutMIDef(MBB, PhysReg) != MI)
+    return;
 
-    SmallVector<MachineBasicBlock *, 4> ToVisit(MBB->successors());
-    SmallPtrSet<MachineBasicBlock*, 4>Visited;
-    while (!ToVisit.empty()) {
-      MachineBasicBlock *MBB = ToVisit.pop_back_val();
-      if (Visited.count(MBB) || !MBB->isLiveIn(PhysReg))
-        continue;
-      if (getLiveInUses(MBB, PhysReg, Uses))
-        llvm::append_range(ToVisit, MBB->successors());
-      Visited.insert(MBB);
-    }
+  SmallVector<MachineBasicBlock *, 4> ToVisit(MBB->successors());
+  SmallPtrSet<MachineBasicBlock *, 4> Visited;
+
+  // Otherwise visit successors until PhysReg is redefined.
+  while (!ToVisit.empty()) {
+    MachineBasicBlock *MBB = ToVisit.pop_back_val();
+    if (Visited.count(MBB) || !MBB->isLiveIn(PhysReg))
+      continue;
+    getLiveInUses(MBB, PhysReg, Uses);
+    // Check if any PhysReg definition exist in the current MBB.
+    if (!getLocalLiveOutMIDef(MBB, PhysReg))
+      llvm::append_range(ToVisit, MBB->successors());
+    Visited.insert(MBB);
   }
 }
 



More information about the llvm-commits mailing list