[PATCH] D11421: New interface to enable shrink wrapping

hfinkel at anl.gov hfinkel at anl.gov
Sat Jul 25 12:49:26 PDT 2015

hfinkel added a comment.

> This is necessary for PowerPC, as the decision to run shrink wrapping is partially based on the ABI.

How do you imagine this working such that we can still turn off shrink wrapping on PowerPC using the command-line option?

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());
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.

Comment at: lib/CodeGen/ShrinkWrap.cpp:323
@@ -303,1 +322,3 @@
 bool ShrinkWrap::runOnMachineFunction(MachineFunction &MF) {
+  if (PredicateFtor && !PredicateFtor(MF)) 
+    return false;
kbarton wrote:
> nemanjai wrote:
> > Does this mean that shrink wrapping will run on a passed function if the PredicateFtor is not callable? Although as currently set-up, this cannot happen.
> Yes, that is true. 
> I could not remove the default constructor, as it is needed somewhere else.
> I could change this logic to disable Shrink Wrapping if PredicateFtor has not been set if there is a concern about this.
This seems reasonable. If you somehow were able to invoke the pass manually from the command line using the default constructor, you'd expect it to run.


More information about the llvm-commits mailing list