[NFCi] Iterative Outliner + clang-format refactoring.

[plotfi]

Summary

Prior to D69446 I had done some NFC cleanup to make landing an iterative outliner a cleaner more straight-forward patch. Since then, it seems that has landed but I noticed some ways it could be cleaned up. Specifically:

1. doOutline was meant to be the re-runable function, but instead runOnceOnModule was created that just calls doOutline.

2. In D69446 we discussed that the flag allowing the re-run of the outliner should be a flag to tell how many additional times to run the outliner again, not the total number of times. I don't think it makes sense to introduce a flag, but print an error if the flag is set to 0.

This is an NFCi, the i being that I get rid of the way that the machine-outline-runs flag could be used to tell the outliner to not run at all, and because I renamed the flag to '-machine-outliner-reruns'.

[thegameg]

Looks pretty straightforward. Thanks!

[paquette]

llvm/lib/CodeGen/MachineOutliner.cpp

100–106

• s/if/of/
• total number of runs

llvm/test/CodeGen/AArch64/machine-outliner-iterative-2.mir

21

noredzone can be passed in MachineModuleInfo now, so you shouldn't need any IR.

see llvm/test/CodeGen/AArch64/function-info-noredzone-present.ll for an example

26

Maybe it would be better to do this with a regex?
e.g.

BL @OUTLINED_FUNCTION_[[FUNC1:[0-9]+]]

42–50

can just delete this I think

51

can just delete this too I think

56

If you change everything this:

(store 8 into %stack.1)

into this:

(store 8)

you can remove some references to the IR. This, in combination with putting the redzone attribute in the MIR, should allow you to delete the IR entirely.

94–103

can probably just delete this

[plotfi]

Updated based on @paquette's feedback

[paquette]

LGTM

llvm/lib/CodeGen/MachineOutliner.cpp

 // functions. Since the outliner is confined to a single module (modulo LTO),
 // this is off by default. It should, however, be the default behaviour in
 // LTO.
 static cl::opt<bool> EnableLinkOnceODROutlining(
     "enable-linkonceodr-outlining", cl::Hidden,
     cl::desc("Enable the machine outliner on linkonceodr functions"),
     cl::init(false));
 
-// Set the number of times to repeatedly apply outlining.
-// Defaults to 1, but more repetitions can save additional size.
-static cl::opt<unsigned>
-    NumRepeat("machine-outline-runs", cl::Hidden,
-              cl::desc("The number of times to apply machine outlining"),
-              cl::init(1));
+/// Number of times to re-run the outliner. This is not the total number of runs
+/// as the outliner will run at least one time. The default value is set to 0,
+/// meaning the outliner will run one time and rerun zero times after that.
+static cl::opt<unsigned> OutlinerReruns(
+    "machine-outliner-reruns", cl::init(0), cl::Hidden,
+    cl::desc(
+        "Number of times to rerun the outliner after the initial outline"));
 
 namespace {
 
 /// Represents an undefined index in the suffix tree.
 const unsigned EmptyIdx = -1;
 
 /// A node in a suffix tree which represents a substring or suffix.
 ///
struct MachineOutliner : public ModulePass {
   bool outline(Module &M, std::vector<OutlinedFunction> &FunctionList,
                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 runOnceOnModule NumRepeat times
+  /// Calls 'doOutline()' 1 + OutlinerReruns times.
   bool runOnModule(Module &M) override;
 
-  /// Calls 'doOutline()'.
-  bool runOnceOnModule(Module &M, unsigned Iter);
-
   /// Construct a suffix tree on the instructions in \p M and outline repeated
   /// strings from that tree.
   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) {
     for (const Candidate &C : OF.Candidates)
for (Candidate &C : OF.Candidates) {
         // code.
         SmallSet<Register, 2> UseRegs, DefRegs;
         // Copy over the defs in the outlined range.
         // First inst in outlined range <-- Anything that's defined in this
         // ...                           .. range has to be added as an
         // implicit Last inst in outlined range  <-- def to the call
         // instruction. Also remove call site information for outlined block
         // of code. The exposed uses need to be copied in the outlined range.
-        for (MachineBasicBlock::reverse_iterator Iter = EndIt.getReverse(),
+        for (MachineBasicBlock::reverse_iterator
+                 Iter = EndIt.getReverse(),
                  Last = std::next(CallInst.getReverse());
              Iter != Last; Iter++) {
           MachineInstr *MI = &*Iter;
           for (MachineOperand &MOP : MI->operands()) {
             // Skip over anything that isn't a register.
             if (!MOP.isReg())
               continue;
 
             if (MOP.isDef()) {
for (Candidate &C : OF.Candidates) {
               UseRegs.insert(MOP.getReg());
             }
           }
           if (MI->isCandidateForCallSiteEntry())
             MI->getMF()->eraseCallSiteInfo(MI);
         }
 
         for (const Register &I : DefRegs)
-           // If it's a def, add it to the call instruction.
-          CallInst->addOperand(MachineOperand::CreateReg(
-                  I, true, /* isDef = true */
+          CallInst->addOperand(
+              MachineOperand::CreateReg(I, true, /* isDef = true */
                                         true /* isImp = true */));
 
         for (const Register &I : UseRegs)
           // If it's a exposed use, add it to the call instruction.
           CallInst->addOperand(
               MachineOperand::CreateReg(I, false, /* isDef = false */
                                         true /* isImp = true */));
       }
 
MORE.emit([&]() {
                                                     FnCountAfter)
         << "; Delta: "
         << DiagnosticInfoOptimizationBase::Argument("Delta", FnDelta);
       return R;
     });
   }
 }
 
-bool MachineOutliner::runOnceOnModule(Module &M, unsigned Iter) {
+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;
 
-  OutlineRepeatedNum = Iter;
-
   // Number to append to the current outlined function.
   unsigned OutlinedFunctionNum = 0;
 
+  OutlineRepeatedNum = 0;
   if (!doOutline(M, OutlinedFunctionNum))
     return false;
+
+  for (unsigned I = 0; I < OutlinerReruns; ++I) {
+    OutlinedFunctionNum = 0;
+    OutlineRepeatedNum++;
+    if (!doOutline(M, OutlinedFunctionNum)) {
+      LLVM_DEBUG({
+        dbgs() << "Did not outline on iteration " << I + 2 << " out of "
+               << OutlinerReruns + 1 << "\n";
+      });
+      break;
+    }
+  }
+
   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.
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;
-}
+  LLVM_DEBUG({
+    if (!OutlinedSomething)
+      dbgs() << "Stopped outlining at iteration " << OutlineRepeatedNum
+             << " because no changes were found.\n";
+  });
 
-// Apply machine outlining for NumRepeat times.
-bool MachineOutliner::runOnModule(Module &M) {
-  if (NumRepeat < 1)
-    report_fatal_error("Expect NumRepeat for machine outlining "
-                       "to be greater than or equal to 1!\n");
-
-  bool Changed = false;
-  for (unsigned I = 0; I < NumRepeat; I++) {
-    if (!runOnceOnModule(M, I)) {
-      LLVM_DEBUG(dbgs() << "Stopped outlining at iteration " << I
-                        << " because no changes were found.\n";);
-      return Changed;
-    }
-    Changed = true;
-  }
-  LLVM_DEBUG(dbgs() << "Stopped outlining because iteration is "
-                       "equal to " << NumRepeat << "\n";);
-  return Changed;
+  return OutlinedSomething;
 }

llvm/test/CodeGen/AArch64/machine-outliner-iterative-2.mir

+# RUN: llc -mtriple=aarch64-linux-gnu -run-pass=machine-outliner \
+# RUN:   -machine-outliner-reruns=0 -verify-machineinstrs %s -o - | FileCheck --check-prefix=ITER1 %s
+
+# RUN: llc -mtriple=aarch64-linux-gnu -run-pass=machine-outliner \
+# RUN:   -machine-outliner-reruns=1 -verify-machineinstrs %s -o - | FileCheck --check-prefix=ITER2 %s
+
+# RUN: llc -mtriple=aarch64-linux-gnu -run-pass=machine-outliner \
+# RUN:   -machine-outliner-reruns=2 -debug-only=machine-outliner \
+# RUN:   -verify-machineinstrs %s -o /dev/null 2>&1  | FileCheck --check-prefix=ITER3 %s
+
+--- |
+  declare void @z1(i32, i32)
+  declare void @z2(i32, i32, i32)
+  define void @a(i32 %p1) { unreachable }
+  define void @b(i32 %p1) { unreachable }
+...
+---
+#ITER1-LABEL: name: a
+#ITER1:      $w19 = ORRWrs $wzr, killed $w0, 0
+#ITER1-NEXT: BL @OUTLINED_FUNCTION_{{[0-9]+}}
+#ITER1-NEXT: $w1 = ORRWri $wzr, 1
+#ITER1-NEXT: BL @OUTLINED_FUNCTION_{{[0-9]+}}
+#ITER1-NEXT: $w1 = ORRWri $wzr, 1984
+#ITER1-NEXT: BL @OUTLINED_FUNCTION_{{[0-9]+}}
+#ITER1-NEXT: BL @OUTLINED_FUNCTION_{{[0-9]+}}
+
+#ITER2-LABEL: name: a
+#ITER2:      $w19 = ORRWrs $wzr, killed $w0, 0
+#ITER2-NEXT: BL @OUTLINED_FUNCTION_{{[0-9]+}}
+#ITER2-NEXT: $w1 = ORRWri $wzr, 1
+#ITER2-NEXT: BL @OUTLINED_FUNCTION_{{[0-9]+}}
+#ITER2-NEXT: BL @OUTLINED_FUNCTION_{{[0-9]+}}
+#ITER2-NEXT: BL @OUTLINED_FUNCTION_{{[0-9]+}}
+name:            a
+tracksRegLiveness: true
+liveins:
+  - { reg: '$w0', virtual-reg: '' }
+machineFunctionInfo:
+  hasRedZone:      false
+body:             |
+  bb.0:
+    liveins: $w0, $lr, $x19
+
+    early-clobber $sp = frame-setup STPXpre killed $x19, killed $lr, $sp, -2 :: (store 8), (store 8)
+    $w19 = ORRWrs $wzr, killed $w0, 0
+    $w0 = ORRWri $wzr, 0
+    $w1 = ORRWri $wzr, 1
+    BL @z1, csr_aarch64_aapcs, implicit-def dead $lr, implicit $sp, implicit killed $w0, implicit killed $w1, implicit-def $sp
+    $w1 = ORRWri $wzr, 1
+    $w2 = ORRWri $wzr, 1920
+    $w0 = ORRWrs $wzr, $w19, 0
+    BL @z2, csr_aarch64_aapcs, implicit-def dead $lr, implicit $sp, implicit killed $w0, implicit killed $w1, implicit killed $w2, implicit-def $sp
+    $w1 = ORRWri $wzr, 1984
+    $w2 = ORRWri $wzr, 1920
+    $w0 = ORRWrs $wzr, killed $w19, 0
+    BL @z2, csr_aarch64_aapcs, implicit-def dead $lr, implicit $sp, implicit killed $w0, implicit killed $w1, implicit killed $w2, implicit-def $sp
+    $w0 = ORRWri $wzr, 0
+    $w1 = ORRWri $wzr, 1
+    BL @z1, csr_aarch64_aapcs, implicit-def dead $lr, implicit $sp, implicit killed $w0, implicit killed $w1, implicit-def $sp
+    early-clobber $sp, $x19, $lr = frame-destroy LDPXpost $sp, 2 :: (load 8), (load 8)
+    RET undef $lr
+
+...
+---
+#ITER1-LABEL: name: b
+#ITER1:      $w19 = ORRWrs $wzr, killed $w0, 0
+#ITER1-NEXT: BL @OUTLINED_FUNCTION_{{[0-9]+}}
+#ITER1-NEXT: $w1 = ORRWri $wzr, 1984
+#ITER1-NEXT: BL @OUTLINED_FUNCTION_{{[0-9]+}}
+#ITER1-NEXT: $w1 = ORRWri $wzr, 1984
+#ITER1-NEXT: BL @OUTLINED_FUNCTION_{{[0-9]+}}
+#ITER1-NEXT: BL @OUTLINED_FUNCTION_{{[0-9]+}}
+
+#ITER2-LABEL: name: b
+#ITER2:      $w19 = ORRWrs $wzr, killed $w0, 0
+#ITER2-NEXT: BL @OUTLINED_FUNCTION_{{[0-9]+}}
+#ITER2-NEXT: BL @OUTLINED_FUNCTION_{{[0-9]+}}
+#ITER2-NEXT: BL @OUTLINED_FUNCTION_{{[0-9]+}}
+#ITER2-NEXT: BL @OUTLINED_FUNCTION_{{[0-9]+}}
+name:            b
+tracksRegLiveness: true
+liveins:
+  - { reg: '$w0', virtual-reg: '' }
+machineFunctionInfo:
+  hasRedZone:      false
+body:             |
+  bb.0:
+    liveins: $w0, $lr, $x19
+
+    early-clobber $sp = frame-setup STPXpre killed $x19, killed $lr, $sp, -2 :: (store 8), (store 8)
+    $w19 = ORRWrs $wzr, killed $w0, 0
+    $w0 = ORRWri $wzr, 0
+    $w1 = ORRWri $wzr, 1
+    BL @z1, csr_aarch64_aapcs, implicit-def dead $lr, implicit $sp, implicit killed $w0, implicit killed $w1, implicit-def $sp
+    $w1 = ORRWri $wzr, 1984
+    $w2 = ORRWri $wzr, 1920
+    $w0 = ORRWrs $wzr, $w19, 0
+    BL @z2, csr_aarch64_aapcs, implicit-def dead $lr, implicit $sp, implicit killed $w0, implicit killed $w1, implicit killed $w2, implicit-def $sp
+    $w1 = ORRWri $wzr, 1984
+    $w2 = ORRWri $wzr, 1920
+    $w0 = ORRWrs $wzr, killed $w19, 0
+    BL @z2, csr_aarch64_aapcs, implicit-def dead $lr, implicit $sp, implicit killed $w0, implicit killed $w1, implicit killed $w2, implicit-def $sp
+    $w0 = ORRWri $wzr, 0
+    $w1 = ORRWri $wzr, 1
+    BL @z1, csr_aarch64_aapcs, implicit-def dead $lr, implicit $sp, implicit killed $w0, implicit killed $w1, implicit-def $sp
+    early-clobber $sp, $x19, $lr = frame-destroy LDPXpost $sp, 2 :: (load 8), (load 8)
+    RET undef $lr
+...
+
+#ITER1-LABEL: name: OUTLINED_FUNCTION_{{[0-9]+}}
+#ITER1-LABEL: name: OUTLINED_FUNCTION_{{[0-9]+}}
+
+#ITER2-LABEL: name: OUTLINED_FUNCTION_{{[0-9]+}}
+#ITER2-LABEL: name: OUTLINED_FUNCTION_{{[0-9]+}}
+#ITER2-LABEL: name: OUTLINED_FUNCTION_{{[0-9]+}}
+
+# ITER3: Did not outline on iteration 3 out of 3

llvm/test/CodeGen/AArch64/machine-outliner-iterative.mir

-# RUN: llc -mtriple=aarch64 -run-pass=machine-outliner -machine-outline-runs=2 -verify-machineinstrs %s -o - | FileCheck %s --check-prefix TWO-RUNS
-# RUN: llc -mtriple=aarch64 -run-pass=machine-outliner -machine-outline-runs=1 -verify-machineinstrs %s -o - | FileCheck %s --check-prefix ONE-RUN
-# RUN: llc -mtriple=aarch64 -run-pass=machine-outliner -machine-outline-runs=4 -verify-machineinstrs %s -o - | FileCheck %s --check-prefix FOUR-RUNS
+# RUN: llc -mtriple=aarch64 -run-pass=machine-outliner -machine-outliner-reruns=1 -verify-machineinstrs %s -o - | FileCheck %s --check-prefix TWO-RUNS
+# RUN: llc -mtriple=aarch64 -run-pass=machine-outliner -machine-outliner-reruns=0 -verify-machineinstrs %s -o - | FileCheck %s --check-prefix ONE-RUN
+# RUN: llc -mtriple=aarch64 -run-pass=machine-outliner -machine-outliner-reruns=3 -verify-machineinstrs %s -o - | FileCheck %s --check-prefix FOUR-RUNS
 
 # Example of Repeated Instruction Sequence - Iterative Machine Outlining
 #
 #; define void @"$s12"(...) {         define i64 @"$s5” (...) {             define void @"$s13"(...) {
 #   ...                                ...                                   ...
 #   %8 = load i1, i1* %7                                                     %8 = load i1, i1* %7
 #   %9 = load i4, i4*, %6              %9 = load i4, i4*, %6                 %9 = load i4, i4*, %6
 #   store i4 %9, i4* %5                store i4 %9, i4* %5                   store i4 %9, i4* %5