[PATCH] D73692: [PowerPC] Fix spilling of vector registers in PEI of EH aware functions

[Jinsong Ji via Phabricator via llvm-commits]

jsji added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCInstrInfo.h:293
 
+  void storeRegToStackSlotNoUpd(MachineBasicBlock &MBB,
+                                MachineBasicBlock::iterator MBBI,
----------------
nemanjai wrote:
> nemanjai wrote:
> > jsji wrote:
> > > NeHuang wrote:
> > > > nit: IMHO it could be better to add descriptions/comments on `NoUpd` for the two new functions `storeRegToStackSlotNoUpd` and  `loadRegFromStackSlotNoUpd`? 
> > > Nit: `NoUpd` here is referring to `No updatedRC`? It might be confusing at first glance, as store with update may refer to store with index update?
> > > 
> > > Why we want to create a new interface here? Why not extending `storeRegToStackSlot` with a default arg?
> > > `storeRegToStackSlot(... , bool mustPreserveElementOrder=true)`
> > > `loadRegFromStackSlot(...., bool mustPreserveElementOrder=true)`
> > I can't really say that I'm up on my knowledge of how virtual function signatures are allowed or not allowed to change between the base class and the derived class. But I can try it and see if the compiler complains.
> OK, tried it and apparently you can't change the signature of virtual functions you override. Which I initially assumed without trying it, now I've confirmed it.
Yes, my original proposal is not to change it when override it. But to change the original virtual functions as well. But anyway, this can be a nice to have NFC.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73692