[PATCH] Add a shrink-wrapping pass to improve the placement of prologue and epilogue.

[Kristof Beyls]

REPOSITORY
  rL LLVM

================
Comment at: lib/CodeGen/MachineFunction.cpp:626-627
@@ +625,4 @@
+
+  // Starting from MBB, check if there is a path leading to Save than do
+  // not cross Restore.
+  SmallPtrSet<const MachineBasicBlock *, 8> Visited;
----------------
qcolombet wrote:
> qcolombet wrote:
> > kristof.beyls wrote:
> > > I don't understand what "... than do not cross Restore." means here.
> > > Should the comment just be "Starting from MBB, check if there is a patch leading to Save"?
> > That is a typo indeed, this is “that do not cross Restore” :).
> > This is the important part in fact. Restore is the kill point of the region that needs CSR to be saved and past the kill point, there is nothing to track.
> Do I still need to do something about the comment?
If the comment reads "Starting from MBB, check if there is a path leading to Save that does not cross Restore", that makes sense to me.

================
Comment at: lib/CodeGen/PrologEpilogInserter.cpp:389-392
@@ +388,6 @@
+  MachineBasicBlock *Entry = &MF.front();
+  MachineBasicBlock *Save = MFI->getSavePoint();
+
+  if (!Save)
+    Save = Entry;
+
----------------
qcolombet wrote:
> qcolombet wrote:
> > kristof.beyls wrote:
> > > I'm wondering if the logic overall would get simpler if MFI->getSavePoint() would always return a MachineBasicBlock,
> > > i.e. returning Entry if shrink wrapping didn't change the default save point?
> > > That way, the logic in most functions doesn't need to handle the case separately of whether or not the save block
> > > and entry block are the same - generalizing the logic a bit more.
> > > But maybe there's a good reason to do it like this that I haven't noticed?
> > The rational was that this is homogenous with the way we handle the Restore point. I.e., current shrink-wrapping just give Restore point, whereas PEI can have multiple of them. So I did not want to unbalanced this.
> > 
> > I guess I can also push the list of exits blocks in MFI if you think that is better.
> What do you think?
> Should I:
> 1. Keep Restore and Save as they are?
> 2. Create an asymmetry between their handling? (Don’t like that.)
> 3. Push everything into MachineFrameInfo?
Looking further into the details of how PEI is implemented at the moment, I'm starting to think that the best option probably is:
* Get rid of the EntryBlock variable in class PEI, and replace it/change its name to "SaveBlock".
* Get rid of the ReturnBlocks variable in class PEI, and replace it/change its name to "RestoreBlocks".

That way, the names of the variables reflect the intent better, i.e. the block where saves and restores need to be inserted.
This can be done as a separate NFC-patch.
After that, the shrink wrapping pass "just" optimizes the SaveBlock and RestoreBlocks values. With the current implementation, if the shrink wrapping actually does any optimization, RestoreBlocks will be a single-element vector.

Doing it this way, I think PEI code should not have to special case (i.e. have if statements) based on whether or not the shrink wrapping optimization happened or not. I hope.

I guess this corresponds to option 3, "push everything into MachineFrameInfo"?

http://reviews.llvm.org/D9210

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/