[llvm] r348414 - [MachineOutliner] Outline functions by order of benefit

Jessica Paquette via llvm-commits llvm-commits at lists.llvm.org
Wed Dec 5 13:36:05 PST 2018


Author: paquette
Date: Wed Dec  5 13:36:04 2018
New Revision: 348414

URL: http://llvm.org/viewvc/llvm-project?rev=348414&view=rev
Log:
[MachineOutliner] Outline functions by order of benefit

Mostly NFC, only change is the order of outlined function names.

Loop over the outlined functions instead of walking the candidate list.

This is a bit easier to understand. It's far more natural to create a function,
then replace all of its occurrences with calls than the other way around.

The functions outlined after this do not change, but their names will be
decided by their benefit. E.g, OUTLINED_FUNCTION_0 will now always be the
most beneficial function, rather than the first one seen.

This makes it easier to enforce an ordering on the outlined functions. So,
this also adds a test to make sure that the ordering works as expected.

Added:
    llvm/trunk/test/CodeGen/AArch64/machine-outliner-ordering.mir
Modified:
    llvm/trunk/include/llvm/CodeGen/MachineOutliner.h
    llvm/trunk/lib/CodeGen/MachineOutliner.cpp

Modified: llvm/trunk/include/llvm/CodeGen/MachineOutliner.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/MachineOutliner.h?rev=348414&r1=348413&r2=348414&view=diff
==============================================================================
--- llvm/trunk/include/llvm/CodeGen/MachineOutliner.h (original)
+++ llvm/trunk/include/llvm/CodeGen/MachineOutliner.h Wed Dec  5 13:36:04 2018
@@ -190,7 +190,7 @@ public:
   unsigned FrameConstructionID;
 
   /// Return the number of candidates for this \p OutlinedFunction.
-  unsigned getOccurrenceCount() const { return OccurrenceCount; }
+  unsigned getOccurrenceCount() const { return Candidates.size(); }
 
   /// Decrement the occurrence count of this OutlinedFunction and return the
   /// new count.

Modified: llvm/trunk/lib/CodeGen/MachineOutliner.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/MachineOutliner.cpp?rev=348414&r1=348413&r2=348414&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/MachineOutliner.cpp (original)
+++ llvm/trunk/lib/CodeGen/MachineOutliner.cpp Wed Dec  5 13:36:04 2018
@@ -1414,78 +1414,83 @@ bool MachineOutliner::outline(
   // Number to append to the current outlined function.
   unsigned OutlinedFunctionNum = 0;
 
-  // Replace the candidates with calls to their respective outlined functions.
-  for (const std::shared_ptr<Candidate> &Cptr : CandidateList) {
-    Candidate &C = *Cptr;
-    // Was the candidate removed during pruneOverlaps?
-    if (!C.InCandidateList)
-      continue;
-
-    // If not, then look at its OutlinedFunction.
-    OutlinedFunction &OF = FunctionList[C.FunctionIdx];
+  // Sort by benefit. The most beneficial functions should be outlined first.
+  std::stable_sort(
+      FunctionList.begin(), FunctionList.end(),
+      [](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.
+  for (OutlinedFunction &OF : FunctionList) {
+    // If we outlined something that overlapped with a candidate in a previous
+    // step, then we can't outline from it.
+    erase_if(OF.Candidates,
+             [](std::shared_ptr<Candidate> &C) { return !C->InCandidateList; });
 
-    // Was its OutlinedFunction made unbeneficial during pruneOverlaps?
+    // If we made it unbeneficial to outline this function, skip it.
     if (OF.getBenefit() < 1)
       continue;
 
-    // Does this candidate have a function yet?
-    if (!OF.MF) {
-      OF.MF = createOutlinedFunction(M, OF, Mapper, OutlinedFunctionNum);
-      emitOutlinedFunctionRemark(OF);
-      FunctionsCreated++;
-      OutlinedFunctionNum++; // Created a function, move to the next name.
-    }
-
+    // It's beneficial. Create the function and outline its sequence's
+    // occurrences.
+    OF.MF = createOutlinedFunction(M, OF, Mapper, OutlinedFunctionNum);
+    emitOutlinedFunctionRemark(OF);
+    FunctionsCreated++;
+    OutlinedFunctionNum++; // Created a function, move to the next name.
     MachineFunction *MF = OF.MF;
-    MachineBasicBlock &MBB = *C.getMBB();
-    MachineBasicBlock::iterator StartIt = C.front();
-    MachineBasicBlock::iterator EndIt = C.back();
-    assert(StartIt != C.getMBB()->end() && "StartIt out of bounds!");
-    assert(EndIt != C.getMBB()->end() && "EndIt out of bounds!");
-
     const TargetSubtargetInfo &STI = MF->getSubtarget();
     const TargetInstrInfo &TII = *STI.getInstrInfo();
 
-    // Insert a call to the new function and erase the old sequence.
-    auto CallInst = TII.insertOutlinedCall(M, MBB, StartIt, *OF.MF, C);
+    // Replace occurrences of the sequence with calls to the new function.
+    for (std::shared_ptr<Candidate> &Cptr : OF.Candidates) {
+      Candidate &C = *Cptr;
+      MachineBasicBlock &MBB = *C.getMBB();
+      MachineBasicBlock::iterator StartIt = C.front();
+      MachineBasicBlock::iterator EndIt = C.back();
+
+      // Insert the call.
+      auto CallInst = TII.insertOutlinedCall(M, MBB, StartIt, *MF, C);
+
+      // If the caller tracks liveness, then we need to make sure that
+      // anything we outline doesn't break liveness assumptions. The outlined
+      // functions themselves currently don't track liveness, but we should
+      // make sure that the ranges we yank things out of aren't wrong.
+      if (MBB.getParent()->getProperties().hasProperty(
+              MachineFunctionProperties::Property::TracksLiveness)) {
+        // Helper lambda for adding implicit def operands to the call
+        // instruction.
+        auto CopyDefs = [&CallInst](MachineInstr &MI) {
+          for (MachineOperand &MOP : MI.operands()) {
+            // Skip over anything that isn't a register.
+            if (!MOP.isReg())
+              continue;
+
+            // If it's a def, add it to the call instruction.
+            if (MOP.isDef())
+              CallInst->addOperand(MachineOperand::CreateReg(
+                  MOP.getReg(), true, /* isDef = true */
+                  true /* isImp = true */));
+          }
+        };
+        // 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.
+        std::for_each(CallInst, std::next(EndIt), CopyDefs);
+      }
+
+      // Erase from the point after where the call was inserted up to, and
+      // including, the final instruction in the sequence.
+      // Erase needs one past the end, so we need std::next there too.
+      MBB.erase(std::next(StartIt), std::next(EndIt));
+      OutlinedSomething = true;
 
-    // If the caller tracks liveness, then we need to make sure that anything
-    // we outline doesn't break liveness assumptions.
-    // The outlined functions themselves currently don't track liveness, but
-    // we should make sure that the ranges we yank things out of aren't
-    // wrong.
-    if (MBB.getParent()->getProperties().hasProperty(
-            MachineFunctionProperties::Property::TracksLiveness)) {
-      // Helper lambda for adding implicit def operands to the call instruction.
-      auto CopyDefs = [&CallInst](MachineInstr &MI) {
-        for (MachineOperand &MOP : MI.operands()) {
-          // Skip over anything that isn't a register.
-          if (!MOP.isReg())
-            continue;
-
-          // If it's a def, add it to the call instruction.
-          if (MOP.isDef())
-            CallInst->addOperand(
-                MachineOperand::CreateReg(MOP.getReg(), true, /* isDef = true */
-                                          true /* isImp = true */));
-        }
-      };
-
-      // 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.
-      std::for_each(CallInst, std::next(EndIt), CopyDefs);
+      // Statistics.
+      NumOutlined++;
     }
-
-    // Erase from the point after where the call was inserted up to, and
-    // including, the final instruction in the sequence.
-    // Erase needs one past the end, so we need std::next there too.
-    MBB.erase(std::next(StartIt), std::next(EndIt));
-    OutlinedSomething = true;
-
-    // Statistics.
-    NumOutlined++;
   }
 
   LLVM_DEBUG(dbgs() << "OutlinedSomething = " << OutlinedSomething << "\n";);

Added: llvm/trunk/test/CodeGen/AArch64/machine-outliner-ordering.mir
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/AArch64/machine-outliner-ordering.mir?rev=348414&view=auto
==============================================================================
--- llvm/trunk/test/CodeGen/AArch64/machine-outliner-ordering.mir (added)
+++ llvm/trunk/test/CodeGen/AArch64/machine-outliner-ordering.mir Wed Dec  5 13:36:04 2018
@@ -0,0 +1,106 @@
+# RUN: llc -mtriple=aarch64-unknown-unknown -run-pass=machine-outliner \
+# RUN: -verify-machineinstrs %s -o - | FileCheck %s
+
+# Ensure that outlined function names appear as expected. Currently, they are
+# output in order of benefit.
+
+--- |
+  define void @should_have_fn2() #0 { ret void }
+  define void @should_have_fn0() #0 { ret void }
+  define void @should_have_fn1() #0 { ret void }
+  attributes #0 = { noredzone optsize minsize }
+...
+---
+
+name: should_have_fn2
+tracksRegLiveness: true
+body:             |
+    bb.0:
+        ; CHECK-LABEL: name:        should_have_fn2
+        ; CHECK-NOT: OUTLINED_FUNCTION_1
+        ; CHECK-NOT: OUTLINED_FUNCTION_0
+        ; CHECK: OUTLINED_FUNCTION_2
+        $w0 = ORRWri $wzr, 1
+        $w0 = ORRWri $wzr, 1
+        $w0 = ORRWri $wzr, 1
+    bb.1:
+        ; CHECK-DAG: OUTLINED_FUNCTION_2
+        $w0 = ORRWri $wzr, 1
+        $w0 = ORRWri $wzr, 1
+        $w0 = ORRWri $wzr, 1
+    bb.2:
+        ; CHECK-DAG: OUTLINED_FUNCTION_2
+        $w0 = ORRWri $wzr, 1
+        $w0 = ORRWri $wzr, 1
+        $w0 = ORRWri $wzr, 1
+    bb.3:
+        ; CHECK-DAG: OUTLINED_FUNCTION_2
+        $w0 = ORRWri $wzr, 1
+        $w0 = ORRWri $wzr, 1
+        $w0 = ORRWri $wzr, 1
+    bb.4:
+        RET undef $lr
+
+...
+---
+
+name: should_have_fn0
+
+tracksRegLiveness: true
+body:             |
+    bb.0:
+        ; CHECK-LABEL: name: should_have_fn0
+        ; CHECK-NOT: OUTLINED_FUNCTION_1
+        ; CHECK-NOT: OUTLINED_FUNCTION_2
+        ; CHECK: OUTLINED_FUNCTION_0
+        $w1 = ORRWri $wzr, 1
+        $w1 = ORRWri $wzr, 1
+        $w1 = ORRWri $wzr, 1
+        $w1 = ORRWri $wzr, 1
+        $w1 = ORRWri $wzr, 1
+    bb.1:
+        ; CHECK-DAG: OUTLINED_FUNCTION_0
+        $w1 = ORRWri $wzr, 1
+        $w1 = ORRWri $wzr, 1
+        $w1 = ORRWri $wzr, 1
+        $w1 = ORRWri $wzr, 1
+        $w1 = ORRWri $wzr, 1
+    bb.3:
+        ; CHECK-DAG: OUTLINED_FUNCTION_0
+        $w1 = ORRWri $wzr, 1
+        $w1 = ORRWri $wzr, 1
+        $w1 = ORRWri $wzr, 1
+        $w1 = ORRWri $wzr, 1
+        $w1 = ORRWri $wzr, 1
+    bb.4:
+        RET undef $lr
+
+...
+---
+
+name: should_have_fn1
+tracksRegLiveness: true
+body:             |
+    bb.0:
+        ; CHECK-LABEL: name: should_have_fn1
+        ; CHECK-NOT: OUTLINED_FUNCTION_0
+        ; CHECK-NOT: OUTLINED_FUNCTION_2
+        ; CHECK: OUTLINED_FUNCTION_1
+        $w2 = ORRWri $wzr, 1
+        $w2 = ORRWri $wzr, 1
+        $w2 = ORRWri $wzr, 1
+        $w2 = ORRWri $wzr, 1
+    bb.1:
+        ; CHECK-DAG: OUTLINED_FUNCTION_1
+        $w2 = ORRWri $wzr, 1
+        $w2 = ORRWri $wzr, 1
+        $w2 = ORRWri $wzr, 1
+        $w2 = ORRWri $wzr, 1
+    bb.3:
+        ; CHECK-DAG: OUTLINED_FUNCTION_1
+        $w2 = ORRWri $wzr, 1
+        $w2 = ORRWri $wzr, 1
+        $w2 = ORRWri $wzr, 1
+        $w2 = ORRWri $wzr, 1
+    bb.4:
+        RET undef $lr




More information about the llvm-commits mailing list