[AArch64] Resolve FIXME: Use scavengeRegisterBackwards to find the best unused register

[AtariDreams]

No description provided.

[llvmbot]

@llvm/pr-subscribers-llvm-regalloc
@llvm/pr-subscribers-backend-amdgpu

@llvm/pr-subscribers-backend-aarch64

Author: AtariDreams (AtariDreams)

Changes

---

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

1 Files Affected:

• (modified) llvm/lib/Target/AArch64/AArch64SpeculationHardening.cpp (+2-7)

diff --git a/llvm/lib/Target/AArch64/AArch64SpeculationHardening.cpp b/llvm/lib/Target/AArch64/AArch64SpeculationHardening.cpp
index a991d645eb6f405..86135ef39d82148 100644
--- a/llvm/lib/Target/AArch64/AArch64SpeculationHardening.cpp
+++ b/llvm/lib/Target/AArch64/AArch64SpeculationHardening.cpp
@@ -300,13 +300,8 @@ bool AArch64SpeculationHardening::instrumentControlFlow(
       RS.enterBasicBlock(MBB);
     else
       RS.backward(I);
-    // FIXME: The below just finds *a* unused register. Maybe code could be
-    // optimized more if this looks for the register that isn't used for the
-    // longest time around this place, to enable more scheduling freedom. Not
-    // sure if that would actually result in a big performance difference
-    // though. Maybe RegisterScavenger::findSurvivorBackwards has some logic
-    // already to do this - but it's unclear if that could easily be used here.
-    Register TmpReg = RS.FindUnusedReg(&AArch64::GPR64commonRegClass);
+    Register TmpReg = RS.scavengeRegisterBackwards(AArch64::GPR64commonRegClass,
+                                                   I, false, 0, false);
     LLVM_DEBUG(dbgs() << "RS finds "
                       << ((TmpReg == 0) ? "no register " : "register ");
                if (TmpReg != 0) dbgs() << printReg(TmpReg, TRI) << " ";

[github-actions]

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

[jayfoad]

I think you can call scavengeRegisterBackwards even at the start of a block - can't you?

    if (I == MBB.begin())
      RS.enterBasicBlock(MBB);
    else
      RS.backward(I);
    TmpReg = RS.scavengeRegisterBackwards(AArch64::GPR64commonRegClass, I,
                                          false, 0, false);

The only reason this code was special casing I == MBB.begin() in the first place is for efficiency, not for correctness. The assumption is that enterBasicBlock is more efficient than backward which might have to step over arbitrarily many instructions.

[qcolombet]

If you use the autogenerated test, make sure to clear the all check lines.
As is, there are labels that are repeated which may lead in weird failures with FileCheck. (e.g., the label nobranch_fallthrough appears twice.)

[qcolombet]

Could you add a comment of what needs to happen or must not happen in the comment?

The manual checks were conveying this in a more direct way (but broken in its own way :P).

[arsenm]

Previous comments

[arsenm]

This comment should probably be reserved, fundamentally the scavenge is still doing the same thing and not finding a "nicer" register