[AArch64] Avoid RegScavenger::forward in AArch64SpeculationHardening

[foad]

Summary

RegScavenger::backward is preferred because it does not rely on accurate
kill flags.

[foad]

llvm/lib/Target/AArch64/AArch64SpeculationHardening.cpp

299–303

It is unfortunate that RegScavenger::backward(MachineBasicBlock::iterator) does not provide a way to step back to the state right at the start of the MBB.

[arsenm]

llvm/lib/Target/AArch64/AArch64SpeculationHardening.cpp

299–303

Would it help to use reverse_iterator? When I first looked into doing this I think I got hung up on making reverse_iterator work just as well

[foad]

llvm/lib/Target/AArch64/AArch64SpeculationHardening.cpp

299–303

I don't think using reverse_iterator in AArch64SpeculationHardening would help. Did you mean also changing the RegScavenger API to take reverse_iterators? That might make sense, I'm not sure.

I think a proper fix would change both the RegScavenger API and the internals to track liveness into *MBBI instead of out of it. But I don't want to change any of that yet. The API lets me do what I need, just in an ugly way.

[arsenm]

llvm/lib/Target/AArch64/AArch64SpeculationHardening.cpp

299–303

Yes, I think I had a full set of reverse_iterator APIs but it wasn't particularly usable with everything else expecting forward iterators

[barannikov88]

llvm/lib/Target/AArch64/AArch64SpeculationHardening.cpp

299–303

FWIW A reverse iterator can be converted to a normal iterator with I.getReverse().

llvm/lib/Target/AArch64/AArch64SpeculationHardening.cpp

bool AArch64SpeculationHardening::instrumentControlFlow(
   SmallVector<std::pair<MachineInstr *, unsigned>, 4> CallInstructions;
   // if a temporary register is not available for at least one of the
   // instructions for which we need to transfer taint to the stack pointer, we
   // need to insert a full speculation barrier.
   // TmpRegisterNotAvailableEverywhere tracks that condition.
   bool TmpRegisterNotAvailableEverywhere = false;
 
   RegScavenger RS;
-  RS.enterBasicBlock(MBB);
+  RS.enterBasicBlockEnd(MBB);
 
-  for (MachineBasicBlock::iterator I = MBB.begin(); I != MBB.end(); I++) {
-    MachineInstr &MI = *I;
+  for (MachineBasicBlock::iterator I = MBB.end(); I != MBB.begin(); ) {
+    MachineInstr &MI = *--I;
     if (!MI.isReturn() && !MI.isCall())
       continue;
 
     // The RegScavenger represents registers available *after* the MI
     // instruction pointed to by RS.getCurrentPosition().
     // We need to have a register that is available *before* the MI is executed.
-    if (I != MBB.begin())
-      RS.forward(std::prev(I));
+    if (I == MBB.begin())
+      RS.enterBasicBlock(MBB);
+    else
+      RS.backward(std::prev(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);
     LLVM_DEBUG(dbgs() << "RS finds "
                       << ((TmpReg == 0) ? "no register " : "register ");