[PATCH] D63803: [PowerPC] Move TOC save to prologue when profitable

[Nemanja Ivanovic via Phabricator via llvm-commits]

nemanjai marked 3 inline comments as done.
nemanjai added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCFrameLowering.cpp:447
 
+static bool MustSaveTOC(const MachineFunction &MF) {
+  const PPCFunctionInfo *FI = MF.getInfo<PPCFunctionInfo>();
----------------
hfinkel wrote:
> You've introduced this function, which contains two lines of code, and then make a Boolean variable in several later places with the same name. I recommend just moving this code into its caller below and eliminating any potential confusion.
Will do. I was a little on the fence about the value of this function as well.


================
Comment at: llvm/lib/Target/PowerPC/PPCFrameLowering.cpp:1314
 
+      if (Reg == PPC::X2 || Reg == PPC::R2)
+        continue;
----------------
hfinkel wrote:
> Need a MustSaveTOC check here?
Yup, this is an omission. I'll fix it.


================
Comment at: llvm/lib/Target/PowerPC/PPCMIPeephole.cpp:229
+  // we can just do the save in the prologue.
+  if (!MFI.hasVarSizedObjects() &&
+      (CurrBlockFreq > EntryFreq || MPDT->dominates(MI->getParent(), Entry)))
----------------
hfinkel wrote:
> Why the MFI.hasVarSizedObjects() check?
It is actually not necessary as this function won't be called when processing functions that have dynamic objects.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63803