[MachineOuliner][NFC] Refactoring code to make outline rerunning a cleaner diff.

[plotfi]

Summary

I want to add the ability to rerun the outliner in certain cases, and I thought this could be an NFC change that could make a subsequent change a cleaner diff.

[paquette]

I think the implementation of doOutline is missing here?

llvm/lib/CodeGen/MachineOutliner.cpp

899

I think that if you write this as doOutline(), Doxygen will automatically create a link to its entry on doOutline. That might be useful to people looking at the docs.

(Judging by http://www.doxygen.nl/manual/autolink.html)

[plotfi]

llvm/lib/CodeGen/MachineOutliner.cpp

1440

@paquette The implementation of MachineOutliner::doOutline is right here.

[plotfi]

adding 'doOutline()' for doxygen comments.

[paquette]

LGTM

[tellenbach]

Maybe I just don't see it but I don't get the reason for creating this new function doOutline. Doesn't it basically just call outline? Why can't you just add OutlinedFunctionNum to outline and then call outline in runOnModule?

Please be so kind to clarify this.

[plotfi]

In D69482#1723825, @tellenbach wrote:

Maybe I just don't see it but I don't get the reason for creating this new function doOutline. Doesn't it basically just call outline? Why can't you just add OutlinedFunctionNum to outline and then call outline in runOnModule?

Please be so kind to clarify this.

It’s possible to get greater size reduction through rerunning the outliner repeatedly. I and a couple other folks were looking into this and I noticed that much of the change could be refactored into an nfc change which is preferable.

[tellenbach]

In D69482#1723832, @plotfi wrote:

In D69482#1723825, @tellenbach wrote:

Maybe I just don't see it but I don't get the reason for creating this new function doOutline. Doesn't it basically just call outline? Why can't you just add OutlinedFunctionNum to outline and then call outline in runOnModule?

Please be so kind to clarify this.

It’s possible to get greater size reduction through rerunning the outliner repeatedly. I and a couple other folks were looking into this and I noticed that much of the change could be refactored into an nfc change which is preferable.

I see, thanks!

llvm/lib/CodeGen/MachineOutliner.cpp

struct MachineOutliner : public ModulePass {
 
   /// Replace the sequences of instructions represented by \p OutlinedFunctions
   /// with calls to functions.
   ///
   /// \param M The module we are outlining from.
   /// \param FunctionList A list of functions to be inserted into the module.
   /// \param Mapper Contains the instruction mappings for the module.
   bool outline(Module &M, std::vector<OutlinedFunction> &FunctionList,
-               InstructionMapper &Mapper);
+               InstructionMapper &Mapper,
+               unsigned &OutlinedFunctionNum);
 
   /// Creates a function for \p OF and inserts it into the module.
   MachineFunction *createOutlinedFunction(Module &M, OutlinedFunction &OF,
                                           InstructionMapper &Mapper,
                                           unsigned Name);
 
+  /// Calls doOutline.
+  bool runOnModule(Module &M) override;
+
   /// Construct a suffix tree on the instructions in \p M and outline repeated
   /// strings from that tree.
-  bool runOnModule(Module &M) override;
+  bool doOutline(Module &M, unsigned &OutlinedFunctionNum);
 
   /// Return a DISubprogram for OF if one exists, and null otherwise. Helper
   /// function for remark emission.
   DISubprogram *getSubprogramOrNull(const OutlinedFunction &OF) {
     DISubprogram *SP;
     for (const Candidate &C : OF.Candidates)
       if (C.getMF() && (SP = C.getMF()->getFunction().getSubprogram()))
         return SP;
if (DISubprogram *SP = getSubprogramOrNull(OF)) {
     DB.finalize();
   }
 
   return &MF;
 }
 
 bool MachineOutliner::outline(Module &M,
                               std::vector<OutlinedFunction> &FunctionList,
-                              InstructionMapper &Mapper) {
+                              InstructionMapper &Mapper,
+                              unsigned &OutlinedFunctionNum) {
 
   bool OutlinedSomething = false;
 
-  // Number to append to the current outlined function.
-  unsigned OutlinedFunctionNum = 0;
-
   // Sort by benefit. The most beneficial functions should be outlined first.
   llvm::stable_sort(FunctionList, [](const OutlinedFunction &LHS,
                                      const OutlinedFunction &RHS) {
     return LHS.getBenefit() > RHS.getBenefit();
   });
 
   // Walk over each function, outlining them as we go along. Functions are
   // outlined greedily, based off the sort above.

 }
 
 bool MachineOutliner::runOnModule(Module &M) {
   // Check if there's anything in the module. If it's empty, then there's
   // nothing to outline.
   if (M.empty())
     return false;
 
+  // Number to append to the current outlined function.
+  unsigned OutlinedFunctionNum = 0;
+
+  if (!doOutline(M, OutlinedFunctionNum))
+    return false;
+  return true;
+}
+
+bool MachineOutliner::doOutline(Module &M, unsigned &OutlinedFunctionNum) {
   MachineModuleInfo &MMI = getAnalysis<MachineModuleInfoWrapperPass>().getMMI();
 
   // If the user passed -enable-machine-outliner=always or
   // -enable-machine-outliner, the pass will run on all functions in the module.
   // Otherwise, if the target supports default outlining, it will run on all
   // functions deemed by the target to be worth outlining from by default. Tell
   // the user how the outliner is running.
   LLVM_DEBUG(
bool MachineOutliner::doOutline(Module &M, unsigned &OutlinedFunctionNum) {
 
   // Check if we want size remarks.
   bool ShouldEmitSizeRemarks = M.shouldEmitInstrCountChangedRemark();
   StringMap<unsigned> FunctionToInstrCount;
   if (ShouldEmitSizeRemarks)
     initSizeRemarkInfo(M, MMI, FunctionToInstrCount);
 
   // Outline each of the candidates and return true if something was outlined.
-  bool OutlinedSomething = outline(M, FunctionList, Mapper);
+  bool OutlinedSomething =
+      outline(M, FunctionList, Mapper, OutlinedFunctionNum);
 
   // If we outlined something, we definitely changed the MI count of the
   // module. If we've asked for size remarks, then output them.
   // FIXME: This should be in the pass manager.
   if (ShouldEmitSizeRemarks && OutlinedSomething)
     emitInstrCountChangedRemark(M, MMI, FunctionToInstrCount);
 
   return OutlinedSomething;
 }