[PATCH] D150560: [AArch64] Avoid RegScavenger::forward in AArch64SpeculationHardening

Tue May 23 00:51:53 PDT 2023

arsenm accepted this revision.
arsenm added inline comments.
This revision is now accepted and ready to land.

================
Comment at: llvm/lib/Target/AArch64/AArch64SpeculationHardening.cpp:299-302
+    if (I == MBB.begin())
+      RS.enterBasicBlock(MBB);
+    else
+      RS.backward(std::prev(I));
----------------
foad wrote:
> arsenm wrote:
> > foad wrote:
> > > 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.
> > 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
> 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.
Yes, I think I had a full set of reverse_iterator APIs but it wasn't particularly usable with everything else expecting forward iterators 

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150560/new/

https://reviews.llvm.org/D150560