[PATCH] D66935: [AArch64][DebugInfo] Do not recompute CalleeSavedStackSize

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added a comment.

In D66935#1651499 <https://reviews.llvm.org/D66935#1651499>, @efriedma wrote:

> Why are we calling determineCalleeSaves in LiveDebugValues, anyway?  Can't it just call getCalleeSavedInfo()?
>  Making the determineCalleeSaves computation lazy, like this patch does, is really confusing.


This method seems to be called by several passes:

- LiveDebugValues
- PrologEpilogInserter
- RegUsageInfoCollector
- RegisterScavenging
- ShrinkWrap

The problem with `determineCalleeSaves` is that it does much more than determining the callee-saves:

- It also creates a new StackObject for the emergency spill slot.
- It marks this slot as the scavenging slot in the RegisterScavenger.
- It saves the size of the callee-save area (in AArch64MachineFunctionInfo).

And this behaviour to change state seems somewhat by design, because `RegisterScavenger*` is passed as argument and MachineFunction is not a `const` reference. After PEI, `getCalleeSavedInfo` can be used as you suggest, but one could argue that `determineCalleeSaves` is still a valid interface to use. Perhaps I could add an assert in determineCalleeSaves that suggests using getCalleeSavedInfo if `MFI.isCalleeSavedInfoValid()` is true, but I think that is even more confusing. Perhaps the better way is to not let `determineCalleeSaves` change state at all, but that would require changes to the interface, which is likely to also require changes to other targets. From that point of view, letting `determineCalleeSaves` use the cached information from `getCalleeSavedInfo` seemed sensible.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66935