[llvm] r320229 - [MachineOutliner] Outline calls
Simon Pilgrim via llvm-commits
llvm-commits at lists.llvm.org
Sat Dec 9 11:51:33 PST 2017
This looks like its causing an error when EXPENSIVE_CHECKS are enabled:
http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-win/builds/6680/steps/test-check-all/logs/stdio
# After Machine Outliner
# Machine code for function OUTLINED_FUNCTION_2: IsSSA, TracksLiveness
%bb.0:
early-clobber %sp = STRXpre %lr, %sp, -16
BL <ga:@baz>, implicit-def dead %lr, implicit %sp
%w17 = ORRWri %wzr, 1
%w17 = ORRWri %wzr, 1
%w17 = ORRWri %wzr, 1
%w17 = ORRWri %wzr, 1
BL <ga:@baz>, implicit-def dead %lr, implicit %sp
early-clobber %sp, %lr = LDRXpost %sp, 16
RET undef %lr
# End machine code for function OUTLINED_FUNCTION_2.
*** Bad machine code: Using an undefined physical register ***
- function: OUTLINED_FUNCTION_2
- basic block: %bb.0 (0x24a3370)
- instruction: early-clobber %sp = STRXpre
- operand 1: %lr
LLVM ERROR: Found 1 machine code errors.
On 09/12/2017 00:43, Jessica Paquette via llvm-commits wrote:
> Author: paquette
> Date: Fri Dec 8 16:43:49 2017
> New Revision: 320229
>
> URL: http://llvm.org/viewvc/llvm-project?rev=320229&view=rev
> Log:
> [MachineOutliner] Outline calls
>
> The outliner previously would never outline calls. Calls are pretty common in
> files, so it makes sense to outline them. In fact, in the LLVM test suite, if
> you count the number of instructions that the outliner misses when you outline
> calls vs when you don't, it turns out that, on average, around 6% of the
> instructions encountered are calls. So, if we outline calls, we can find more
> candidates, and thus save some more space.
>
> This commit adds that functionality and updates the mir test to reflect 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=320229&r1=320228&r2=320229&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Target/AArch64/AArch64InstrInfo.cpp (original)
> +++ llvm/trunk/lib/Target/AArch64/AArch64InstrInfo.cpp Fri Dec 8 16:43:49 2017
> @@ -26,6 +26,7 @@
> #include "llvm/CodeGen/MachineInstr.h"
> #include "llvm/CodeGen/MachineInstrBuilder.h"
> #include "llvm/CodeGen/MachineMemOperand.h"
> +#include "llvm/CodeGen/MachineModuleInfo.h"
> #include "llvm/CodeGen/MachineOperand.h"
> #include "llvm/CodeGen/MachineRegisterInfo.h"
> #include "llvm/CodeGen/StackMaps.h"
> @@ -4747,6 +4748,20 @@ 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);
> }
> @@ -4796,6 +4811,70 @@ AArch64InstrInfo::getOutliningType(Machi
> return MachineOutlinerInstrType::Illegal;
> }
>
> + // Outline calls without stack parameters or aggregate parameters.
> + if (MI.isCall()) {
> + assert(MF->getFunction() && "MF doesn't have a function?");
> + 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 +4899,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.
> @@ -4841,12 +4919,11 @@ AArch64InstrInfo::getOutliningType(Machi
> // 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;
> -
> - // It's in range, so we can outline it.
> +
> + // It's in range, so we can outline it.
> return MachineOutlinerInstrType::Legal;
> }
>
> @@ -4889,6 +4966,43 @@ 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);
> +
> + 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;
>
> 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=320229&r1=320228&r2=320229&view=diff
> ==============================================================================
> --- llvm/trunk/test/CodeGen/AArch64/machine-outliner.mir (original)
> +++ llvm/trunk/test/CodeGen/AArch64/machine-outliner.mir Fri Dec 8 16:43:49 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]}}
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list