[PATCH] D11421: New interface to enable shrink wrapping

[Quentin Colombet]

qcolombet added a comment.

Hi Kit,

LGTM with Hal’s comments addressed.

Thanks,
Q.


================
Comment at: lib/CodeGen/Passes.cpp:539
@@ -541,4 +538,3 @@
   // Insert prolog/epilog code.  Eliminate abstract frame index references...
-  if (getEnableShrinkWrap())
-    addPass(&ShrinkWrapID);
+  addPass(createShrinkWrapPass());
   addPass(&PrologEpilogCodeInserterID);
----------------
hfinkel wrote:
> wschmidt wrote:
> > kbarton wrote:
> > > This will add shrink wrapping all the time. I think it makes more sense to guard this by (getOptLevel() != CodeGenOpt::None),  as is done below for addMachineLateOptimization().
> > > I will make this change in the next revision. 
> > Depending on the computational complexity of shrink wrapping, you may want to enable this at -O2 and up rather than -O1.  I haven't looked at the code, but I assume Quentin can advise.
> Two comments here:
> 
> The pass should always be added, but it should contain a check for:
> 
>   if (skipOptnoneFunction(*MF.getFunction()))
> 
> and, if it does not, we should fix that.
> 
> Second, regarding complexity, it does not seem as though it should be significantly more expensive than MachineLICM, etc. that are used at -O1, so I think that leaving it always enabled if (getOptLevel() != CodeGenOpt::None) should be fine.
Like Hal said, having it running at O1 should be fine.

================
Comment at: lib/CodeGen/ShrinkWrap.cpp:191
@@ +190,3 @@
+  /// \param[in] MF The MachineFunction to run shrink wrapping on
+  /// \return true if shrink wrapping should be run, false otherwise
+  std::function<bool(const MachineFunction &MF)> PredicateFtor;
----------------
Period at end of comments.


http://reviews.llvm.org/D11421