From: Jessica Paquette Date: Mon, 18 Dec 2017 19:33:21 +0000 (+0000) Subject: [MachineOutliner] Recommit r320229 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=d4a00a677300944a92ae9e687d7a8264280ff53e;p=llvm [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. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@321014 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/Target/AArch64/AArch64InstrInfo.cpp b/lib/Target/AArch64/AArch64InstrInfo.cpp index e26f15bedb7..c7c560a8132 100644 --- a/lib/Target/AArch64/AArch64InstrInfo.cpp +++ b/lib/Target/AArch64/AArch64InstrInfo.cpp @@ -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::getSerializableMachineMemOperandTargetFlags() const { 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::canOutlineWithoutLRSave( // 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::getOutlininingCandidateInfo( 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(MachineInstr &MI) const { 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()->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(MachineInstr &MI) const { 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(MachineInstr &MI) const { // 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::insertOutlinerEpilogue( 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 AArch64InstrInfo::insertOutlinedCall( It = MBB.insert(It, LDRXpost); return It; -} +} \ No newline at end of file diff --git a/test/CodeGen/AArch64/machine-outliner.mir b/test/CodeGen/AArch64/machine-outliner.mir index ee3daf11c9f..708e2e42880 100644 --- a/test/CodeGen/AArch64/machine-outliner.mir +++ b/test/CodeGen/AArch64/machine-outliner.mir @@ -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]}}