[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

• 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

     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 {
 

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

         // 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(),
-             Last = std::next(CallInst.getReverse());
+        for (MachineBasicBlock::reverse_iterator
+                 Iter = EndIt.getReverse(),
+                 Last = std::next(CallInst.getReverse());
              Iter != Last; Iter++) {
           MachineInstr *MI = &*Iter;
           for (MachineOperand &MOP : MI->operands()) {

         }
 
         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.

   }
 }
 
-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;
 }
 

   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
 #