Skip to content

Reapply "[AArch64][NFC] Switch to LiveRegUnits (#87313)" #96840

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

Closed
wants to merge 1 commit into from

Conversation

AZero13
Copy link
Contributor

@AZero13 AZero13 commented Jun 27, 2024

This reverts commit 84314d0.

@llvmbot
Copy link
Member

llvmbot commented Jun 27, 2024

@llvm/pr-subscribers-backend-aarch64

Author: AtariDreams (AtariDreams)

Changes

This reverts commit 84314d0.


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

1 Files Affected:

  • (modified) llvm/lib/Target/AArch64/AArch64FrameLowering.cpp (+16-16)
diff --git a/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp b/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
index 8216fa7db822c..22023eb7f9e69 100644
--- a/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
+++ b/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
@@ -197,6 +197,7 @@
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/Statistic.h"
 #include "llvm/CodeGen/LivePhysRegs.h"
+#include "llvm/CodeGen/LiveRegUnits.h"
 #include "llvm/CodeGen/MachineBasicBlock.h"
 #include "llvm/CodeGen/MachineFrameInfo.h"
 #include "llvm/CodeGen/MachineFunction.h"
@@ -1011,7 +1012,7 @@ void AArch64FrameLowering::emitZeroCallUsedRegs(BitVector RegsToZero,
   }
 }
 
-static void getLiveRegsForEntryMBB(LivePhysRegs &LiveRegs,
+static void getLiveRegsForEntryMBB(LiveRegUnits &LiveRegs,
                                    const MachineBasicBlock &MBB) {
   const MachineFunction *MF = MBB.getParent();
   LiveRegs.addLiveIns(MBB);
@@ -1044,16 +1045,18 @@ static Register findScratchNonCalleeSaveRegister(MachineBasicBlock *MBB) {
 
   const AArch64Subtarget &Subtarget = MF->getSubtarget<AArch64Subtarget>();
   const AArch64RegisterInfo &TRI = *Subtarget.getRegisterInfo();
-  LivePhysRegs LiveRegs(TRI);
+  LiveRegUnits LiveRegs(TRI);
   getLiveRegsForEntryMBB(LiveRegs, *MBB);
 
   // Prefer X9 since it was historically used for the prologue scratch reg.
-  const MachineRegisterInfo &MRI = MF->getRegInfo();
-  if (LiveRegs.available(MRI, AArch64::X9))
+  if (LiveRegs.available(AArch64::X9))
     return AArch64::X9;
 
-  for (unsigned Reg : AArch64::GPR64RegClass) {
-    if (LiveRegs.available(MRI, Reg))
+  BitVector Allocatable =
+      TRI.getAllocatableSet(*MF, TRI.getRegClass(AArch64::GPR64RegClassID));
+
+  for (unsigned Reg : Allocatable.set_bits()) {
+    if (LiveRegs.available(Reg))
       return Reg;
   }
   return AArch64::NoRegister;
@@ -1069,14 +1072,11 @@ bool AArch64FrameLowering::canUseAsPrologue(
   const AArch64FunctionInfo *AFI = MF->getInfo<AArch64FunctionInfo>();
 
   if (AFI->hasSwiftAsyncContext()) {
-    const AArch64RegisterInfo &TRI = *Subtarget.getRegisterInfo();
-    const MachineRegisterInfo &MRI = MF->getRegInfo();
-    LivePhysRegs LiveRegs(TRI);
+    LiveRegUnits LiveRegs(*RegInfo);
     getLiveRegsForEntryMBB(LiveRegs, MBB);
     // The StoreSwiftAsyncContext clobbers X16 and X17. Make sure they are
     // available.
-    if (!LiveRegs.available(MRI, AArch64::X16) ||
-        !LiveRegs.available(MRI, AArch64::X17))
+    if (!LiveRegs.available(AArch64::X16) || !LiveRegs.available(AArch64::X17))
       return false;
   }
 
@@ -1668,7 +1668,7 @@ static void emitDefineCFAWithFP(MachineFunction &MF, MachineBasicBlock &MBB,
 /// Collect live registers from the end of \p MI's parent up to (including) \p
 /// MI in \p LiveRegs.
 static void getLivePhysRegsUpTo(MachineInstr &MI, const TargetRegisterInfo &TRI,
-                                LivePhysRegs &LiveRegs) {
+                                LiveRegUnits &LiveRegs) {
 
   MachineBasicBlock &MBB = *MI.getParent();
   LiveRegs.addLiveOuts(MBB);
@@ -1706,7 +1706,7 @@ void AArch64FrameLowering::emitPrologue(MachineFunction &MF,
          NonFrameStart->getFlag(MachineInstr::FrameSetup))
     ++NonFrameStart;
 
-  LivePhysRegs LiveRegs(*TRI);
+  LiveRegUnits LiveRegs(*TRI);
   if (NonFrameStart != MBB.end()) {
     getLivePhysRegsUpTo(*NonFrameStart, *TRI, LiveRegs);
     // Ignore registers used for stack management for now.
@@ -1730,7 +1730,7 @@ void AArch64FrameLowering::emitPrologue(MachineFunction &MF,
          make_range(MBB.instr_begin(), NonFrameStart->getIterator())) {
       for (auto &Op : MI.operands())
         if (Op.isReg() && Op.isDef())
-          assert(!LiveRegs.contains(Op.getReg()) &&
+          assert(LiveRegs.available(Op.getReg()) &&
                  "live register clobbered by inserted prologue instructions");
     }
   });
@@ -4324,7 +4324,7 @@ MachineBasicBlock::iterator tryMergeAdjacentSTG(MachineBasicBlock::iterator II,
   // FIXME : This approach of bailing out from merge is conservative in
   // some ways like even if stg loops are not present after merge the
   // insert list, this liveness check is done (which is not needed).
-  LivePhysRegs LiveRegs(*(MBB->getParent()->getSubtarget().getRegisterInfo()));
+  LiveRegUnits LiveRegs(*(MBB->getParent()->getSubtarget().getRegisterInfo()));
   LiveRegs.addLiveOuts(*MBB);
   for (auto I = MBB->rbegin();; ++I) {
     MachineInstr &MI = *I;
@@ -4333,7 +4333,7 @@ MachineBasicBlock::iterator tryMergeAdjacentSTG(MachineBasicBlock::iterator II,
     LiveRegs.stepBackward(*I);
   }
   InsertI++;
-  if (LiveRegs.contains(AArch64::NZCV))
+  if (!LiveRegs.available(AArch64::NZCV))
     return InsertI;
 
   llvm::stable_sort(Instrs,

@AZero13 AZero13 changed the title Reapply "[AArch64][NFC] Switch to LiveRegUnits (#87313)" [AArch64] Switch to LiveRegUnits Jun 27, 2024
@AZero13 AZero13 changed the title [AArch64] Switch to LiveRegUnits Reapply "[AArch64][NFC] Switch to LiveRegUnits (#87313)" Jun 28, 2024
@AZero13
Copy link
Contributor Author

AZero13 commented Jul 15, 2024

@davemgreen Is this PR good?

@davemgreen
Copy link
Collaborator

I'm not sure. We recently found problems in RegUnits because they were not representing the upper D register correctly (see #96146). But we might not see the same problems if this just uses available(..).

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.

3 participants