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

[Kristof Beyls]

Overall, it seems you've made a lot of changes in the Hexagon backend. I guess these are necessary to make the backend structured more like the other backends so that the SaveBlock/RestoreBlock changes in PEI can be done?
If so, are the changes basically refactoring, i.e. moving existing code to be placed behind certain interfaces, or did you need to write a lot of new code in the Hexagon backend that should be reviewed in detail?


================
Comment at: lib/CodeGen/PrologEpilogInserter.cpp:142-157
@@ -142,11 +141,18 @@
+
+  // Use the points found by shrink-wrapping, if any.
+  if (MFI->getSavePoint()) {
+    SaveBlock = MFI->getSavePoint();
+    assert(MFI->getRestorePoint() && "Both restore and save must be set");
+    RestoreBlocks.push_back(MFI->getRestorePoint());
     return;
+  }
 
   // Save refs to entry and return blocks.
-  EntryBlock = Fn.begin();
+  SaveBlock = Fn.begin();
   for (MachineFunction::iterator MBB = Fn.begin(), E = Fn.end();
        MBB != E; ++MBB)
     if (isReturnBlock(MBB))
-      ReturnBlocks.push_back(MBB);
+      RestoreBlocks.push_back(MBB);
 
   return;
 }
----------------
Just a minor comment, feel free to ignore this one:
I feel that it would be a slightly better separation of concerns if MFI->getSavePoint() would always be set, and PEI doesn't need to calculate a SavePoint itself when it isn't set.
For this to happen, the ShrinkWrap pass should probably always be run, and when it's "switched off be default", it would just set MFI->SavePoint and MFI->Restore to
be equal to FN.begin() and to the ReturnBlocks respectively.
I.o.w., The second half of the code above would be executed in the ShrinkWrap pass.
If you'd push ahead and implement it like this, the name of the pass also should be changed to "SaveRestorePointInserter/Calculator" or something similar. It's then a pass
which always calculates the most appropriate Save and Restore blocks and happens to implement a ShrinkWrapping optimization.
As I said, maybe this is pushing for too much separation of concerns - I'm not sure what the negative consequences would be to require the "SaveRestorePointInserterPass" to always be run, if any.

================
Comment at: lib/CodeGen/PrologEpilogInserter.cpp:784-785
@@ -732,1 +783,4 @@
+  // Add epilogue to restore the callee-save registers in each exiting block.
+  for (MachineBasicBlock *RetBlock : RestoreBlocks)
+    TFI.emitEpilogue(Fn, *RetBlock);
 
----------------
s/RetBlock/RestoreBlock/?
The basic blocks iterated through are "Restore blocks", not necessarily "Return blocks"?

================
Comment at: lib/Target/ARM/ARMFrameLowering.cpp:1692-1693
@@ -1690,4 +1691,4 @@
           unsigned Reg = UnspilledCS1GPRs[i];
           // Don't spill high register if the function is thumb
           if (!AFI->isThumbFunction() ||
               isARMLowRegister(Reg) || Reg == ARM::LR) {
----------------
It's unclear to me why this change is needed?
If only implementing the AArch64-backend-specific functionality, how come you need to make a change in the AArch32-backend?

http://reviews.llvm.org/D9210

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