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

Tue May 23 01:07:23 PDT 2023

barannikov88 added inline comments.

================
Comment at: llvm/lib/Target/AArch64/AArch64SpeculationHardening.cpp:299-302
+    if (I == MBB.begin())
+      RS.enterBasicBlock(MBB);
+    else
+      RS.backward(std::prev(I));
----------------
arsenm wrote:
> 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 
FWIW A reverse iterator can be converted to a normal iterator with `I.getReverse()`.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150560