Rework of the interface to enable shrink wrapping

[kbarton]

Summary

Based on comments from Hal (http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20150810/292978.html), I've changed the interface to add a callback mechanism to the TargetFrameLowering class to query whether the specific target supports shrink wrapping. By default, shrink wrapping is disabled by default. Each target can override the default behaviour using the TargetFrameLowering::targetSupportsShrinkWrapping() method. Shrink wrapping can still be explicitly enabled or disabled from the command line, using the existing -enable-shrink-wrap=<true|false> option.

[kbarton]

Pass the MachineFunction to the targetSupportsShrinkWrapping method, to allow function-specific decisions to be made about whether to shrink-wrap it. PPC needs to make this decision based on the ABI and 32/64 bit.

[qcolombet]

Hi Kit,

Small nitpicks inlined but LGTM otherwise.

Thanks,
-Quentin

include/llvm/Target/TargetFrameLowering.h

132

Do not repeat the method name in the comment.

134

Maybe enableShrinkWrapping?
“Supports" is kind of misleading because for instance X86 supports it, but it does not enabled it by default yet.

(Personally, I would not repeat target in the method name, but I see that are precedence :).)

lib/CodeGen/ShrinkWrap.cpp

155

Period at the end of the comment.

156

Could be a static method.

332

The formatting seems weird here.

332

Maybe put this check with the previous condition, i.e., if MF.empty || !isShrinkWrapEnabled(MF).

[hfinkel]

Please take care of Quentin's "nitpicks", otherwise, LGTM too. Thanks!

include/llvm/Target/TargetFrameLowering.h

134

I agree, enableShrinkWrapping seems good.

[kbarton]

Addressed Quentin's comments. I"ll commit this shortly.

lib/CodeGen/ShrinkWrap.cpp

156

I've made it static, and dropped the const qualifier as a result.

[kbarton]

Committed revision 246463.

include/llvm/CodeGen/Passes.h

protected:
   // Target Pass Options
   // Targets provide a default setting, user flags override.
   //
   bool DisableVerify;
 
   /// Default setting for -enable-tail-merge on this target.
   bool EnableTailMerge;
 
-  /// Default setting for -enable-shrink-wrap on this target.
-  bool EnableShrinkWrap;
-
 public:
   TargetPassConfig(TargetMachine *tm, PassManagerBase &pm);
   // Dummy constructor.
   TargetPassConfig();
 
   ~TargetPassConfig() override;
 
   static char ID;

include/llvm/Target/TargetFrameLowering.h

public:
 
   /// targetHandlesStackFrameRounding - Returns true if the target is
   /// responsible for rounding up the stack frame (probably at emitPrologue
   /// time).
   virtual bool targetHandlesStackFrameRounding() const {
     return false;
   }
 
+  /// targetHandlesShrinkWrapping - Returns true if the target will correctly
+  /// handle shrink wrapping.
+  virtual bool targetSupportsShrinkWrapping() const {
+    return false;
+  }
+  
   /// emitProlog/emitEpilog - These methods insert prolog and epilog code into
   /// the function.
   virtual void emitPrologue(MachineFunction &MF,
                             MachineBasicBlock &MBB) const = 0;
   virtual void emitEpilogue(MachineFunction &MF,
                             MachineBasicBlock &MBB) const = 0;
 
   /// Adjust the prologue to have the function use segmented stacks. This works

lib/CodeGen/Passes.cpp

 static cl::opt<bool> DisableMachineDCE("disable-machine-dce", cl::Hidden,
     cl::desc("Disable Machine Dead Code Elimination"));
 static cl::opt<bool> DisableEarlyIfConversion("disable-early-ifcvt", cl::Hidden,
     cl::desc("Disable Early If-conversion"));
 static cl::opt<bool> DisableMachineLICM("disable-machine-licm", cl::Hidden,
     cl::desc("Disable Machine LICM"));
 static cl::opt<bool> DisableMachineCSE("disable-machine-cse", cl::Hidden,
     cl::desc("Disable Machine Common Subexpression Elimination"));
-static cl::opt<cl::boolOrDefault>
-    EnableShrinkWrapOpt("enable-shrink-wrap", cl::Hidden,
-                        cl::desc("enable the shrink-wrapping pass"));
 static cl::opt<cl::boolOrDefault> OptimizeRegAlloc(
     "optimize-regalloc", cl::Hidden,
     cl::desc("Enable optimized register allocation compilation path."));
 static cl::opt<bool> DisablePostRAMachineLICM("disable-postra-machine-licm",
     cl::Hidden,
     cl::desc("Disable Machine LICM"));
 static cl::opt<bool> DisableMachineSink("disable-machine-sink", cl::Hidden,
     cl::desc("Disable Machine Sinking"));

 }
 
 // Out of line constructor provides default values for pass options and
 // registers all common codegen passes.
 TargetPassConfig::TargetPassConfig(TargetMachine *tm, PassManagerBase &pm)
     : ImmutablePass(ID), PM(&pm), StartBefore(nullptr), StartAfter(nullptr),
       StopAfter(nullptr), Started(true), Stopped(false),
       AddingMachinePasses(false), TM(tm), Impl(nullptr), Initialized(false),
-      DisableVerify(false), EnableTailMerge(true), EnableShrinkWrap(false) {
+      DisableVerify(false), EnableTailMerge(true) { 
 
   Impl = new PassConfigImpl();
 
   // Register all target independent codegen passes to activate their PassIDs,
   // including this pass itself.
   initializeCodeGen(*PassRegistry::getPassRegistry());
 
   // Substitute Pseudo Pass IDs for real ones.
if (getOptimizeRegAlloc())
     addOptimizedRegAlloc(createRegAllocPass(true));
   else
     addFastRegAlloc(createRegAllocPass(false));
 
   // Run post-ra passes.
   addPostRegAlloc();
 
   // Insert prolog/epilog code.  Eliminate abstract frame index references...
-  if (getEnableShrinkWrap())
+  if (getOptLevel() != CodeGenOpt::None) 
     addPass(&ShrinkWrapID);
+
   addPass(&PrologEpilogCodeInserterID);
 
   /// Add passes that optimize machine instructions after register allocation.
   if (getOptLevel() != CodeGenOpt::None)
     addMachineLateOptimization();
 
   // Expand pseudo instructions before second scheduling pass.
   addPass(&ExpandPostRAPseudosID);
void TargetPassConfig::addMachineSSAOptimization() {
   addPass(&MachineSinkingID);
 
   addPass(&PeepholeOptimizerID, false);
   // Clean-up the dead code that may have been generated by peephole
   // rewriting.
   addPass(&DeadMachineInstructionElimID);
 }
 
-bool TargetPassConfig::getEnableShrinkWrap() const {
-  switch (EnableShrinkWrapOpt) {
-  case cl::BOU_UNSET:
-    return EnableShrinkWrap && getOptLevel() != CodeGenOpt::None;
-  // If EnableShrinkWrap is set, it takes precedence on whatever the
-  // target sets. The rational is that we assume we want to test
-  // something related to shrink-wrapping.
-  case cl::BOU_TRUE:
-    return true;
-  case cl::BOU_FALSE:
-    return false;
-  }
-  llvm_unreachable("Invalid shrink-wrapping state");
-}
-
 //===---------------------------------------------------------------------===//
 /// Register Allocation Pass Configuration
 //===---------------------------------------------------------------------===//
 
 bool TargetPassConfig::getOptimizeRegAlloc() const {
   switch (OptimizeRegAlloc) {
   case cl::BOU_UNSET: return getOptLevel() != CodeGenOpt::None;
   case cl::BOU_TRUE:  return true;

lib/CodeGen/ShrinkWrap.cpp

 
 using namespace llvm;
 
 STATISTIC(NumFunc, "Number of functions");
 STATISTIC(NumCandidates, "Number of shrink-wrapping candidates");
 STATISTIC(NumCandidatesDropped,
           "Number of shrink-wrapping candidates dropped because of frequency");
 
+static cl::opt<cl::boolOrDefault>
+    EnableShrinkWrapOpt("enable-shrink-wrap", cl::Hidden,
+                        cl::desc("enable the shrink-wrapping pass"));
+
 namespace {
 /// \brief Class to determine where the safe point to insert the
 /// prologue and epilogue are.
 /// Unlike the paper from Fred C. Chow, PLDI'88, that introduces the
 /// shrink-wrapping term for prologue/epilogue placement, this pass
 /// does not rely on expensive data-flow analysis. Instead we use the
 /// dominance properties and loop information to decide which point
 /// are safe for such insertion.
void init(MachineFunction &MF) {
 
     ++NumFunc;
   }
 
   /// Check whether or not Save and Restore points are still interesting for
   /// shrink-wrapping.
   bool ArePointsInteresting() const { return Save != Entry && Save && Restore; }
 
+  /// \brief Check if shrink wrapping is enabled for this target and function
+  bool isShrinkWrapEnabled(const TargetFrameLowering *TFI) const;
+  
 public:
   static char ID;
 
   ShrinkWrap() : MachineFunctionPass(ID) {
     initializeShrinkWrapPass(*PassRegistry::getPassRegistry());
   }
 
   void getAnalysisUsage(AnalysisUsage &AU) const override {
if (Save && Restore && Save != Restore &&
       }      
     }
   }
 }
 
 bool ShrinkWrap::runOnMachineFunction(MachineFunction &MF) {
   if (MF.empty())
     return false;
+
+  const TargetFrameLowering *TFI = MF.getSubtarget().getFrameLowering();
+
+  if (! isShrinkWrapEnabled(TFI))
+    return false;
+  
   DEBUG(dbgs() << "**** Analysing " << MF.getName() << '\n');
 
   init(MF);
 
   for (MachineBasicBlock &MBB : MF) {
     DEBUG(dbgs() << "Look into: " << MBB.getNumber() << ' ' << MBB.getName()
                  << '\n');
 
if (!ArePointsInteresting()) {
     assert(!Save && !Restore && "We miss a shrink-wrap opportunity?!");
     DEBUG(dbgs() << "Nothing to shrink-wrap\n");
     return false;
   }
 
   DEBUG(dbgs() << "\n ** Results **\nFrequency of the Entry: " << EntryFreq
                << '\n');
 
-  const TargetFrameLowering *TFI = MF.getSubtarget().getFrameLowering();
   do {
     DEBUG(dbgs() << "Shrink wrap candidates (#, Name, Freq):\nSave: "
                  << Save->getNumber() << ' ' << Save->getName() << ' '
                  << MBFI->getBlockFreq(Save).getFrequency() << "\nRestore: "
                  << Restore->getNumber() << ' ' << Restore->getName() << ' '
                  << MBFI->getBlockFreq(Restore).getFrequency() << '\n');
 
     bool IsSaveCheap, TargetCanUseSaveAsPrologue = false;
DEBUG(dbgs() << "Final shrink wrap candidates:\nSave: " << Save->getNumber()
                << Restore->getNumber() << ' ' << Restore->getName() << '\n');
 
   MachineFrameInfo *MFI = MF.getFrameInfo();
   MFI->setSavePoint(Save);
   MFI->setRestorePoint(Restore);
   ++NumCandidates;
   return false;
 }
+
+bool ShrinkWrap::isShrinkWrapEnabled(const TargetFrameLowering *TFI) const {
+  switch (EnableShrinkWrapOpt) {
+  case cl::BOU_UNSET:
+    return TFI->targetSupportsShrinkWrapping();
+  // If EnableShrinkWrap is set, it takes precedence on whatever the
+  // target sets. The rational is that we assume we want to test
+  // something related to shrink-wrapping.
+  case cl::BOU_TRUE:
+    return true;
+  case cl::BOU_FALSE:
+    return false;
+  }
+  llvm_unreachable("Invalid shrink-wrapping state");
+}