[llvm] r321014 - [MachineOutliner] Recommit r320229

Jessica Paquette via llvm-commits llvm-commits at lists.llvm.org
Mon Dec 18 11:33:21 PST 2017


Author: paquette
Date: Mon Dec 18 11:33:21 2017
New Revision: 321014

URL: http://llvm.org/viewvc/llvm-project?rev=321014&view=rev
Log:
[MachineOutliner] Recommit r320229

LR was undefined entering outlined functions that contain calls. This made the
machine verifier unhappy when expensive checks were enabled. This fixes that.


Modified:
    llvm/trunk/lib/Target/AArch64/AArch64InstrInfo.cpp
    llvm/trunk/test/CodeGen/AArch64/machine-outliner.mir

Modified: llvm/trunk/lib/Target/AArch64/AArch64InstrInfo.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AArch64/AArch64InstrInfo.cpp?rev=321014&r1=321013&r2=321014&view=diff
==============================================================================
--- llvm/trunk/lib/Target/AArch64/AArch64InstrInfo.cpp (original)
+++ llvm/trunk/lib/Target/AArch64/AArch64InstrInfo.cpp Mon Dec 18 11:33:21 2017
@@ -28,6 +28,7 @@
 #include "llvm/CodeGen/MachineMemOperand.h"
 #include "llvm/CodeGen/MachineOperand.h"
 #include "llvm/CodeGen/MachineRegisterInfo.h"
+#include "llvm/CodeGen/MachineModuleInfo.h"
 #include "llvm/CodeGen/StackMaps.h"
 #include "llvm/CodeGen/TargetRegisterInfo.h"
 #include "llvm/CodeGen/TargetSubtargetInfo.h"
@@ -4640,55 +4641,55 @@ AArch64InstrInfo::getSerializableMachine
   return makeArrayRef(TargetFlags);
 }
 
-/// Constants defining how certain sequences should be outlined.
-/// This encompasses how an outlined function should be called, and what kind of
-/// frame should be emitted for that outlined function.
-///
-/// \p MachineOutlinerDefault implies that the function should be called with
-/// a save and restore of LR to the stack.
-///
-/// That is,
-///
-/// I1     Save LR                    OUTLINED_FUNCTION:
-/// I2 --> BL OUTLINED_FUNCTION       I1
-/// I3     Restore LR                 I2
-///                                   I3
-///                                   RET
-///
-/// * Call construction overhead: 3 (save + BL + restore)
-/// * Frame construction overhead: 1 (ret)
-/// * Requires stack fixups? Yes
-///
-/// \p MachineOutlinerTailCall implies that the function is being created from
-/// a sequence of instructions ending in a return.
-///
-/// That is,
-///
-/// I1                             OUTLINED_FUNCTION:
-/// I2 --> B OUTLINED_FUNCTION     I1
-/// RET                            I2
-///                                RET
-///
-/// * Call construction overhead: 1 (B)
-/// * Frame construction overhead: 0 (Return included in sequence)
-/// * Requires stack fixups? No
-///
-/// \p MachineOutlinerNoLRSave implies that the function should be called using
-/// a BL instruction, but doesn't require LR to be saved and restored. This
-/// happens when LR is known to be dead.
-///
-/// That is,
-///
-/// I1                                OUTLINED_FUNCTION:
-/// I2 --> BL OUTLINED_FUNCTION       I1
-/// I3                                I2
-///                                   I3
-///                                   RET
-///
-/// * Call construction overhead: 1 (BL)
-/// * Frame construction overhead: 1 (RET)
-/// * Requires stack fixups? No
-///
+  /// Constants defining how certain sequences should be outlined.
+  /// This encompasses how an outlined function should be called, and what kind of
+  /// frame should be emitted for that outlined function.
+  ///
+  /// \p MachineOutlinerDefault implies that the function should be called with
+  /// a save and restore of LR to the stack.
+  ///
+  /// That is,
+  ///
+  /// I1     Save LR                    OUTLINED_FUNCTION:
+  /// I2 --> BL OUTLINED_FUNCTION       I1
+  /// I3     Restore LR                 I2
+  ///                                   I3
+  ///                                   RET
+  ///
+  /// * Call construction overhead: 3 (save + BL + restore)
+  /// * Frame construction overhead: 1 (ret)
+  /// * Requires stack fixups? Yes
+  ///
+  /// \p MachineOutlinerTailCall implies that the function is being created from
+  /// a sequence of instructions ending in a return.
+  ///
+  /// That is,
+  ///
+  /// I1                             OUTLINED_FUNCTION:
+  /// I2 --> B OUTLINED_FUNCTION     I1
+  /// RET                            I2
+  ///                                RET
+  ///
+  /// * Call construction overhead: 1 (B)
+  /// * Frame construction overhead: 0 (Return included in sequence)
+  /// * Requires stack fixups? No
+  ///
+  /// \p MachineOutlinerNoLRSave implies that the function should be called using
+  /// a BL instruction, but doesn't require LR to be saved and restored. This
+  /// happens when LR is known to be dead.
+  ///
+  /// That is,
+  ///
+  /// I1                                OUTLINED_FUNCTION:
+  /// I2 --> BL OUTLINED_FUNCTION       I1
+  /// I3                                I2
+  ///                                   I3
+  ///                                   RET
+  ///
+  /// * Call construction overhead: 1 (BL)
+  /// * Frame construction overhead: 1 (RET)
+  /// * Requires stack fixups? No
+  ///
 enum MachineOutlinerClass {
   MachineOutlinerDefault,  /// Emit a save, restore, call, and return.
   MachineOutlinerTailCall, /// Only emit a branch.
@@ -4705,9 +4706,8 @@ bool AArch64InstrInfo::canOutlineWithout
   // Get liveness information from the end of the block to the end of the
   // prospective outlined region.
   std::for_each(MBB.rbegin(),
-               (MachineBasicBlock::reverse_iterator)CallInsertionPt,
-               [&LRU](MachineInstr &MI) {LRU.stepBackward(MI);}
-               );
+                (MachineBasicBlock::reverse_iterator)CallInsertionPt,
+                [&LRU](MachineInstr &MI) { LRU.stepBackward(MI); });
 
   // If the link register is available at this point, then we can safely outline
   // the region without saving/restoring LR. Otherwise, we must emit a save and
@@ -4747,12 +4747,26 @@ AArch64InstrInfo::getOutlininingCandidat
     NumInstrsToCreateFrame = 1;
   }
 
+  // Check if the range contains a call. These require a save + restore of the
+  // link register.
+  if (std::any_of(RepeatedSequenceLocs[0].first, RepeatedSequenceLocs[0].second,
+                  [](const MachineInstr &MI) { return MI.isCall(); }))
+    NumInstrsToCreateFrame += 2; // Save + restore the link register.
+
+  // Handle the last instruction separately. If this is a tail call, then the
+  // last instruction is a call. We don't want to save + restore in this case.
+  // However, it could be possible that the last instruction is a call without
+  // it being valid to tail call this sequence. We should consider this as well.
+  else if (RepeatedSequenceLocs[0].second->isCall() &&
+           FrameID != MachineOutlinerTailCall)
+    NumInstrsToCreateFrame += 2;
+
   return MachineOutlinerInfo(NumInstrsForCall, NumInstrsToCreateFrame, CallID,
                              FrameID);
 }
 
-bool AArch64InstrInfo::isFunctionSafeToOutlineFrom(MachineFunction &MF,
-                                           bool OutlineFromLinkOnceODRs) const {
+bool AArch64InstrInfo::isFunctionSafeToOutlineFrom(
+    MachineFunction &MF, bool OutlineFromLinkOnceODRs) const {
   const Function &F = MF.getFunction();
 
   // If F uses a redzone, then don't outline from it because it might mess up
@@ -4796,6 +4810,69 @@ AArch64InstrInfo::getOutliningType(Machi
     return MachineOutlinerInstrType::Illegal;
   }
 
+  // Outline calls without stack parameters or aggregate parameters.
+  if (MI.isCall()) {
+    const Module *M = MF->getFunction().getParent();
+    assert(M && "No module?");
+
+    // Get the function associated with the call. Look at each operand and find
+    // the one that represents the callee and get its name.
+    Function *Callee = nullptr;
+    for (const MachineOperand &MOP : MI.operands()) {
+      if (MOP.isSymbol()) {
+        Callee = M->getFunction(MOP.getSymbolName());
+        break;
+      }
+
+      else if (MOP.isGlobal()) {
+        Callee = M->getFunction(MOP.getGlobal()->getGlobalIdentifier());
+        break;
+      }
+    }
+
+    // Only handle functions that we have information about.
+    if (!Callee)
+      return MachineOutlinerInstrType::Illegal;
+
+    // We have a function we have information about. Check it if it's something
+    // can safely outline.
+
+    // If the callee is vararg, it passes parameters on the stack. Don't touch
+    // it.
+    // FIXME: Functions like printf are very common and we should be able to
+    // outline them.
+    if (Callee->isVarArg())
+      return MachineOutlinerInstrType::Illegal;
+
+    // Check if any of the arguments are a pointer to a struct. We don't want
+    // to outline these since they might be loaded in two instructions.
+    for (Argument &Arg : Callee->args()) {
+      if (Arg.getType()->isPointerTy() &&
+          Arg.getType()->getPointerElementType()->isAggregateType())
+        return MachineOutlinerInstrType::Illegal;
+    }
+
+    // If the thing we're calling doesn't access memory at all, then we're good
+    // to go.
+    if (Callee->doesNotAccessMemory())
+      return MachineOutlinerInstrType::Legal;
+
+    // It accesses memory. Get the machine function for the callee to see if
+    // it's safe to outline.
+    MachineFunction *CalleeMF = MF->getMMI().getMachineFunction(*Callee);
+
+    // We don't know what's going on with the callee at all. Don't touch it.
+    if (!CalleeMF)
+      return MachineOutlinerInstrType::Illegal;
+
+    // Does it pass anything on the stack? If it does, don't outline it.
+    if (CalleeMF->getInfo<AArch64FunctionInfo>()->getBytesInStackArgArea() != 0)
+      return MachineOutlinerInstrType::Illegal;
+
+    // It doesn't, so it's safe to outline and we're done.
+    return MachineOutlinerInstrType::Legal;
+  }
+
   // Don't outline positions.
   if (MI.isPosition())
     return MachineOutlinerInstrType::Illegal;
@@ -4820,7 +4897,6 @@ AArch64InstrInfo::getOutliningType(Machi
   if (MI.modifiesRegister(AArch64::SP, &RI) ||
       MI.readsRegister(AArch64::SP, &RI)) {
 
-    // Is it a memory operation?
     if (MI.mayLoadOrStore()) {
       unsigned Base;  // Filled with the base regiser of MI.
       int64_t Offset; // Filled with the offset of MI.
@@ -4834,14 +4910,12 @@ AArch64InstrInfo::getOutliningType(Machi
       // Find the minimum/maximum offset for this instruction and check if
       // fixing it up would be in range.
       int64_t MinOffset, MaxOffset; // Unscaled offsets for the instruction.
-      unsigned Scale; // The scale to multiply the offsets by.
-      getMemOpInfo(MI.getOpcode(), Scale, DummyWidth, MinOffset,
-                   MaxOffset);
+      unsigned Scale;               // The scale to multiply the offsets by.
+      getMemOpInfo(MI.getOpcode(), Scale, DummyWidth, MinOffset, MaxOffset);
 
       // TODO: We should really test what happens if an instruction overflows.
       // This is tricky to test with IR tests, but when the outliner is moved
       // to a MIR test, it really ought to be checked.
-
       Offset += 16; // Update the offset to what it would be if we outlined.
       if (Offset < MinOffset * Scale || Offset > MaxOffset * Scale)
         return MachineOutlinerInstrType::Illegal;
@@ -4889,6 +4963,46 @@ void AArch64InstrInfo::insertOutlinerEpi
     MachineBasicBlock &MBB, MachineFunction &MF,
     const MachineOutlinerInfo &MInfo) const {
 
+  bool ContainsCalls = false;
+
+  for (MachineInstr &MI : MBB) {
+    if (MI.isCall()) {
+      ContainsCalls = true;
+      break;
+    }
+  }
+
+  if (ContainsCalls) {
+    // Fix up the instructions in the range, since we're going to modify the
+    // stack.
+    fixupPostOutline(MBB);
+
+    // LR has to be a live in so that we can save it.
+    MBB.addLiveIn(AArch64::LR);
+
+    MachineBasicBlock::iterator It = MBB.begin();
+    MachineBasicBlock::iterator Et = MBB.end();
+
+    if (MInfo.FrameConstructionID == MachineOutlinerTailCall)
+      Et = std::prev(MBB.end());
+
+    // Insert a save before the outlined region
+    MachineInstr *STRXpre = BuildMI(MF, DebugLoc(), get(AArch64::STRXpre))
+                                .addReg(AArch64::SP, RegState::Define)
+                                .addReg(AArch64::LR)
+                                .addReg(AArch64::SP)
+                                .addImm(-16);
+    It = MBB.insert(It, STRXpre);
+
+    // Insert a restore before the terminator for the function.
+    MachineInstr *LDRXpost = BuildMI(MF, DebugLoc(), get(AArch64::LDRXpost))
+                                 .addReg(AArch64::SP, RegState::Define)
+                                 .addReg(AArch64::LR, RegState::Define)
+                                 .addReg(AArch64::SP)
+                                 .addImm(16);
+    Et = MBB.insert(Et, LDRXpost);
+  }
+
   // If this is a tail call outlined function, then there's already a return.
   if (MInfo.FrameConstructionID == MachineOutlinerTailCall)
     return;
@@ -4955,4 +5069,4 @@ MachineBasicBlock::iterator AArch64Instr
   It = MBB.insert(It, LDRXpost);
 
   return It;
-}
+}
\ No newline at end of file

Modified: llvm/trunk/test/CodeGen/AArch64/machine-outliner.mir
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/AArch64/machine-outliner.mir?rev=321014&r1=321013&r2=321014&view=diff
==============================================================================
--- llvm/trunk/test/CodeGen/AArch64/machine-outliner.mir (original)
+++ llvm/trunk/test/CodeGen/AArch64/machine-outliner.mir Mon Dec 18 11:33:21 2017
@@ -1,6 +1,10 @@
 # RUN: llc -mtriple=aarch64--- -run-pass=machine-outliner %s -o - | FileCheck %s
 --- |
 
+  define void @baz() #0 {
+    ret void
+  }
+
   define i32 @main() #0 {
     ret i32 0
   }
@@ -80,6 +84,7 @@ body:             |
 ---
 # This test ensures that we can avoid saving LR when it's available.
 # CHECK-LABEL: bb.1:
+# CHECK-NOT: BL @baz, implicit-def dead %lr, implicit %sp
 # CHECK: BL @OUTLINED_FUNCTION_[[F1:[0-9]+]], implicit-def %lr, implicit %sp
 # CHECK-NEXT: %w17 = ORRWri %wzr, 2
 # CHECK-NEXT: BL @OUTLINED_FUNCTION_[[F1]], implicit-def %lr, implicit %sp
@@ -93,15 +98,19 @@ body:             |
     %fp = frame-setup ADDXri %sp, 16, 0
 
   bb.1:
+    BL @baz, implicit-def dead %lr, implicit %sp
     %w17 = ORRWri %wzr, 1
     %w17 = ORRWri %wzr, 1
     %w17 = ORRWri %wzr, 1
     %w17 = ORRWri %wzr, 1
+    BL @baz, implicit-def dead %lr, implicit %sp
     %w17 = ORRWri %wzr, 2
+    BL @baz, implicit-def dead %lr, implicit %sp
     %w17 = ORRWri %wzr, 1
     %w17 = ORRWri %wzr, 1
     %w17 = ORRWri %wzr, 1
     %w17 = ORRWri %wzr, 1
+    BL @baz, implicit-def dead %lr, implicit %sp
     %w8 = ORRWri %wzr, 0
   
   bb.2:
@@ -110,6 +119,13 @@ body:             |
     RET undef %lr
 
 ...
+---
+name:            baz
+tracksRegLiveness: true
+body:             |
+  bb.0:
+    liveins: %w0, %lr, %w8
+    RET undef %lr
 
 # CHECK-LABEL: name:            OUTLINED_FUNCTION_{{[0-9]}}
 # CHECK=LABEL: name:            OUTLINED_FUNCTION_{{[1-9]}}




More information about the llvm-commits mailing list