From 1643bee451266b5ef7d82d7dd3dadde5bd81802e Mon Sep 17 00:00:00 2001 From: David Green Date: Tue, 5 Nov 2019 09:10:58 +0000 Subject: [PATCH] [Scheduling][ARM] Consistently enable PostRA Machine scheduling In the ARM backend, for historical reasons we have only some targets using Machine Scheduling. The rest use the old list scheduler as they are using itinaries and the list scheduler seems to produce better code (and not crash running out of register on v6m codes). So whether to use the MIScheduler or not is checked at runtime from the subtarget features. This is fine, except for post-ra scheduling. Whether to use the old post-ra list scheduler or the post-ra machine schedule is decided as the pass manager is set up, in arms case from a newly constructed subtarget. Under some situations, like LTO, this won't include the correct cpu so can pick the wrong option. This can have a surprising effect on performance. To fix that, this patch overrides targetSchedulesPostRAScheduling and addPreSched2 in the ARM backend, adding _both_ post-ra schedulers and picking at runtime which to execute. To pick between the two I've had to add a enablePostRAMachineScheduler() method that normally returns enableMachineScheduler() && enablePostRAScheduler(), which can be overridden to enable just one of PostRAMachineScheduler vs PostRAScheduler. Thanks to David Penry for the identifying this problem. Differential Revision: https://reviews.llvm.org/D69775 --- include/llvm/CodeGen/TargetSubtargetInfo.h | 4 +++ lib/CodeGen/MachineScheduler.cpp | 2 +- lib/CodeGen/TargetSubtargetInfo.cpp | 4 +++ lib/Target/ARM/ARMSubtarget.cpp | 12 +++++++- lib/Target/ARM/ARMSubtarget.h | 3 ++ lib/Target/ARM/ARMTargetMachine.cpp | 16 +++++----- lib/Target/ARM/ARMTargetMachine.h | 2 ++ test/CodeGen/ARM/O3-pipeline.ll | 1 + .../ARM/cortex-a57-misched-ldm-wrback.ll | 2 +- test/CodeGen/ARM/cortex-a57-misched-ldm.ll | 2 +- .../ARM/cortex-a57-misched-stm-wrback.ll | 2 +- test/CodeGen/ARM/cortex-a57-misched-stm.ll | 2 +- .../ARM/cortex-a57-misched-vldm-wrback.ll | 2 +- test/CodeGen/ARM/cortex-a57-misched-vldm.ll | 2 +- .../ARM/cortex-a57-misched-vstm-wrback.ll | 2 +- test/CodeGen/ARM/cortex-a57-misched-vstm.ll | 2 +- test/CodeGen/ARM/postrasched.ll | 30 +++++++++++++++++++ 17 files changed, 72 insertions(+), 18 deletions(-) create mode 100644 test/CodeGen/ARM/postrasched.ll diff --git a/include/llvm/CodeGen/TargetSubtargetInfo.h b/include/llvm/CodeGen/TargetSubtargetInfo.h index 56018eca8c2..6768cea8940 100644 --- a/include/llvm/CodeGen/TargetSubtargetInfo.h +++ b/include/llvm/CodeGen/TargetSubtargetInfo.h @@ -206,6 +206,10 @@ public: /// which is the preferred way to influence this. virtual bool enablePostRAScheduler() const; + /// True if the subtarget should run a machine scheduler after register + /// allocation. + virtual bool enablePostRAMachineScheduler() const; + /// True if the subtarget should run the atomic expansion pass. virtual bool enableAtomicExpand() const; diff --git a/lib/CodeGen/MachineScheduler.cpp b/lib/CodeGen/MachineScheduler.cpp index f0721ea3b76..caebb953439 100644 --- a/lib/CodeGen/MachineScheduler.cpp +++ b/lib/CodeGen/MachineScheduler.cpp @@ -402,7 +402,7 @@ bool PostMachineScheduler::runOnMachineFunction(MachineFunction &mf) { if (EnablePostRAMachineSched.getNumOccurrences()) { if (!EnablePostRAMachineSched) return false; - } else if (!mf.getSubtarget().enablePostRAScheduler()) { + } else if (!mf.getSubtarget().enablePostRAMachineScheduler()) { LLVM_DEBUG(dbgs() << "Subtarget disables post-MI-sched.\n"); return false; } diff --git a/lib/CodeGen/TargetSubtargetInfo.cpp b/lib/CodeGen/TargetSubtargetInfo.cpp index 59eb2f9c88c..63766df4d2b 100644 --- a/lib/CodeGen/TargetSubtargetInfo.cpp +++ b/lib/CodeGen/TargetSubtargetInfo.cpp @@ -54,6 +54,10 @@ bool TargetSubtargetInfo::enablePostRAScheduler() const { return getSchedModel().PostRAScheduler; } +bool TargetSubtargetInfo::enablePostRAMachineScheduler() const { + return enableMachineScheduler() && enablePostRAScheduler(); +} + bool TargetSubtargetInfo::useAA() const { return false; } diff --git a/lib/Target/ARM/ARMSubtarget.cpp b/lib/Target/ARM/ARMSubtarget.cpp index 09603057b2c..c9316a71bdf 100644 --- a/lib/Target/ARM/ARMSubtarget.cpp +++ b/lib/Target/ARM/ARMSubtarget.cpp @@ -381,9 +381,19 @@ bool ARMSubtarget::enableMachineScheduler() const { // This overrides the PostRAScheduler bit in the SchedModel for any CPU. bool ARMSubtarget::enablePostRAScheduler() const { + if (enableMachineScheduler()) + return false; + if (disablePostRAScheduler()) + return false; + // Thumb1 cores will generally not benefit from post-ra scheduling + return !isThumb1Only(); +} + +bool ARMSubtarget::enablePostRAMachineScheduler() const { + if (!enableMachineScheduler()) + return false; if (disablePostRAScheduler()) return false; - // Don't reschedule potential IT blocks. return !isThumb1Only(); } diff --git a/lib/Target/ARM/ARMSubtarget.h b/lib/Target/ARM/ARMSubtarget.h index ef460342a69..ad0b1bb5dee 100644 --- a/lib/Target/ARM/ARMSubtarget.h +++ b/lib/Target/ARM/ARMSubtarget.h @@ -806,6 +806,9 @@ public: /// True for some subtargets at > -O0. bool enablePostRAScheduler() const override; + /// True for some subtargets at > -O0. + bool enablePostRAMachineScheduler() const override; + /// Enable use of alias analysis during code generation (during MI /// scheduling, DAGCombine, etc.). bool useAA() const override { return UseAA; } diff --git a/lib/Target/ARM/ARMTargetMachine.cpp b/lib/Target/ARM/ARMTargetMachine.cpp index 7f85b93beac..10f68542e7e 100644 --- a/lib/Target/ARM/ARMTargetMachine.cpp +++ b/lib/Target/ARM/ARMTargetMachine.cpp @@ -322,14 +322,7 @@ namespace { class ARMPassConfig : public TargetPassConfig { public: ARMPassConfig(ARMBaseTargetMachine &TM, PassManagerBase &PM) - : TargetPassConfig(TM, PM) { - if (TM.getOptLevel() != CodeGenOpt::None) { - ARMGenSubtargetInfo STI(TM.getTargetTriple(), TM.getTargetCPU(), - TM.getTargetFeatureString()); - if (STI.hasFeature(ARM::FeatureUseMISched)) - substitutePass(&PostRASchedulerID, &PostMachineSchedulerID); - } - } + : TargetPassConfig(TM, PM) {} ARMBaseTargetMachine &getARMTargetMachine() const { return getTM(); @@ -523,6 +516,13 @@ void ARMPassConfig::addPreSched2() { } addPass(createMVEVPTBlockPass()); addPass(createThumb2ITBlockPass()); + + // Add both scheduling passes to give the subtarget an opportunity to pick + // between them. + if (getOptLevel() != CodeGenOpt::None) { + addPass(&PostMachineSchedulerID); + addPass(&PostRASchedulerID); + } } void ARMPassConfig::addPreEmitPass() { diff --git a/lib/Target/ARM/ARMTargetMachine.h b/lib/Target/ARM/ARMTargetMachine.h index cb8650d8139..ac55d2bdcc2 100644 --- a/lib/Target/ARM/ARMTargetMachine.h +++ b/lib/Target/ARM/ARMTargetMachine.h @@ -70,6 +70,8 @@ public: TargetTriple.isOSWindows() || TargetABI == ARMBaseTargetMachine::ARM_ABI_AAPCS16; } + + bool targetSchedulesPostRAScheduling() const override { return true; }; }; /// ARM/Thumb little endian target machine. diff --git a/test/CodeGen/ARM/O3-pipeline.ll b/test/CodeGen/ARM/O3-pipeline.ll index cb6a005445b..dd741388d74 100644 --- a/test/CodeGen/ARM/O3-pipeline.ll +++ b/test/CodeGen/ARM/O3-pipeline.ll @@ -141,6 +141,7 @@ ; CHECK-NEXT: Thumb IT blocks insertion pass ; CHECK-NEXT: MachineDominator Tree Construction ; CHECK-NEXT: Machine Natural Loop Construction +; CHECK-NEXT: PostRA Machine Instruction Scheduler ; CHECK-NEXT: Post RA top-down list latency scheduler ; CHECK-NEXT: Analyze Machine Code For Garbage Collection ; CHECK-NEXT: Machine Block Frequency Analysis diff --git a/test/CodeGen/ARM/cortex-a57-misched-ldm-wrback.ll b/test/CodeGen/ARM/cortex-a57-misched-ldm-wrback.ll index 2c0aa98eae0..be3df4aae50 100644 --- a/test/CodeGen/ARM/cortex-a57-misched-ldm-wrback.ll +++ b/test/CodeGen/ARM/cortex-a57-misched-ldm-wrback.ll @@ -1,5 +1,5 @@ ; REQUIRES: asserts -; RUN: llc < %s -mtriple=armv8r-eabi -mcpu=cortex-a57 -misched-postra -enable-misched -verify-misched -debug-only=machine-scheduler -o - 2>&1 > /dev/null | FileCheck %s +; RUN: llc < %s -mtriple=armv8r-eabi -mcpu=cortex-a57 -mattr=use-misched -verify-misched -debug-only=machine-scheduler -o - 2>&1 > /dev/null | FileCheck %s ; @a = global i32 0, align 4 diff --git a/test/CodeGen/ARM/cortex-a57-misched-ldm.ll b/test/CodeGen/ARM/cortex-a57-misched-ldm.ll index 02d1c2f55f9..1835870850e 100644 --- a/test/CodeGen/ARM/cortex-a57-misched-ldm.ll +++ b/test/CodeGen/ARM/cortex-a57-misched-ldm.ll @@ -1,5 +1,5 @@ ; REQUIRES: asserts -; RUN: llc < %s -mtriple=armv8r-eabi -mcpu=cortex-a57 -misched-postra -enable-misched -verify-misched -debug-only=machine-scheduler -o - 2>&1 > /dev/null | FileCheck %s +; RUN: llc < %s -mtriple=armv8r-eabi -mcpu=cortex-a57 -mattr=use-misched -verify-misched -debug-only=machine-scheduler -o - 2>&1 > /dev/null | FileCheck %s ; CHECK: ********** MI Scheduling ********** ; We need second, post-ra scheduling to have LDM instruction combined from single-loads diff --git a/test/CodeGen/ARM/cortex-a57-misched-stm-wrback.ll b/test/CodeGen/ARM/cortex-a57-misched-stm-wrback.ll index 67cddc14d04..2ee5f75bec3 100644 --- a/test/CodeGen/ARM/cortex-a57-misched-stm-wrback.ll +++ b/test/CodeGen/ARM/cortex-a57-misched-stm-wrback.ll @@ -1,5 +1,5 @@ ; REQUIRES: asserts -; RUN: llc < %s -mtriple=armv8r-eabi -mcpu=cortex-a57 -misched-postra -enable-misched -verify-misched -debug-only=machine-scheduler -o - 2>&1 > /dev/null | FileCheck %s +; RUN: llc < %s -mtriple=armv8r-eabi -mcpu=cortex-a57 -mattr=use-misched -verify-misched -debug-only=machine-scheduler -o - 2>&1 > /dev/null | FileCheck %s ; N=3 STMIA_UPD should have latency 2cyc and writeback latency 1cyc ; CHECK: ********** MI Scheduling ********** diff --git a/test/CodeGen/ARM/cortex-a57-misched-stm.ll b/test/CodeGen/ARM/cortex-a57-misched-stm.ll index 474f39d84ba..026a62f3520 100644 --- a/test/CodeGen/ARM/cortex-a57-misched-stm.ll +++ b/test/CodeGen/ARM/cortex-a57-misched-stm.ll @@ -1,5 +1,5 @@ ; REQUIRES: asserts -; RUN: llc < %s -mtriple=armv8r-eabi -mcpu=cortex-a57 -misched-postra -enable-misched -verify-misched -debug-only=machine-scheduler -o - 2>&1 > /dev/null | FileCheck %s +; RUN: llc < %s -mtriple=armv8r-eabi -mcpu=cortex-a57 -mattr=use-misched -verify-misched -debug-only=machine-scheduler -o - 2>&1 > /dev/null | FileCheck %s ; N=3 STMIB should have latency 2cyc ; CHECK: ********** MI Scheduling ********** diff --git a/test/CodeGen/ARM/cortex-a57-misched-vldm-wrback.ll b/test/CodeGen/ARM/cortex-a57-misched-vldm-wrback.ll index 1baf472ca49..88b772cc294 100644 --- a/test/CodeGen/ARM/cortex-a57-misched-vldm-wrback.ll +++ b/test/CodeGen/ARM/cortex-a57-misched-vldm-wrback.ll @@ -1,5 +1,5 @@ ; REQUIRES: asserts -; RUN: llc < %s -mtriple=armv8r-eabi -mcpu=cortex-a57 -misched-postra -enable-misched -verify-misched -debug-only=machine-scheduler -o - 2>&1 > /dev/null | FileCheck %s +; RUN: llc < %s -mtriple=armv8r-eabi -mcpu=cortex-a57 -mattr=use-misched -verify-misched -debug-only=machine-scheduler -o - 2>&1 > /dev/null | FileCheck %s ; @a = global double 0.0, align 4 diff --git a/test/CodeGen/ARM/cortex-a57-misched-vldm.ll b/test/CodeGen/ARM/cortex-a57-misched-vldm.ll index 8da133e806e..ac208c65af2 100644 --- a/test/CodeGen/ARM/cortex-a57-misched-vldm.ll +++ b/test/CodeGen/ARM/cortex-a57-misched-vldm.ll @@ -1,5 +1,5 @@ ; REQUIRES: asserts -; RUN: llc < %s -mtriple=armv8r-eabi -mcpu=cortex-a57 -misched-postra -enable-misched -verify-misched -debug-only=machine-scheduler -o - 2>&1 > /dev/null | FileCheck %s +; RUN: llc < %s -mtriple=armv8r-eabi -mcpu=cortex-a57 -mattr=use-misched -verify-misched -debug-only=machine-scheduler -o - 2>&1 > /dev/null | FileCheck %s ; CHECK: ********** MI Scheduling ********** ; We need second, post-ra scheduling to have VLDM instruction combined from single-loads diff --git a/test/CodeGen/ARM/cortex-a57-misched-vstm-wrback.ll b/test/CodeGen/ARM/cortex-a57-misched-vstm-wrback.ll index 05c498eee49..c517f46e561 100644 --- a/test/CodeGen/ARM/cortex-a57-misched-vstm-wrback.ll +++ b/test/CodeGen/ARM/cortex-a57-misched-vstm-wrback.ll @@ -1,5 +1,5 @@ ; REQUIRES: asserts -; RUN: llc < %s -mtriple=armv8r-eabi -mcpu=cortex-a57 -misched-postra -enable-misched -verify-misched -debug-only=machine-scheduler -o - 2>&1 > /dev/null | FileCheck %s +; RUN: llc < %s -mtriple=armv8r-eabi -mcpu=cortex-a57 -mattr=use-misched -verify-misched -debug-only=machine-scheduler -o - 2>&1 > /dev/null | FileCheck %s ; CHECK: ********** MI Scheduling ********** ; We need second, post-ra scheduling to have VSTM instruction combined from single-stores diff --git a/test/CodeGen/ARM/cortex-a57-misched-vstm.ll b/test/CodeGen/ARM/cortex-a57-misched-vstm.ll index f31474f6655..5e9041ce084 100644 --- a/test/CodeGen/ARM/cortex-a57-misched-vstm.ll +++ b/test/CodeGen/ARM/cortex-a57-misched-vstm.ll @@ -1,5 +1,5 @@ ; REQUIRES: asserts -; RUN: llc < %s -mtriple=armv8r-eabi -mcpu=cortex-a57 -misched-postra -enable-misched -verify-misched -debug-only=machine-scheduler -o - 2>&1 > /dev/null | FileCheck %s +; RUN: llc < %s -mtriple=armv8r-eabi -mcpu=cortex-a57 -mattr=use-misched -verify-misched -debug-only=machine-scheduler -o - 2>&1 > /dev/null | FileCheck %s ; CHECK: ********** MI Scheduling ********** ; We need second, post-ra scheduling to have VSTM instruction combined from single-stores diff --git a/test/CodeGen/ARM/postrasched.ll b/test/CodeGen/ARM/postrasched.ll new file mode 100644 index 00000000000..85593d55105 --- /dev/null +++ b/test/CodeGen/ARM/postrasched.ll @@ -0,0 +1,30 @@ +; REQUIRES: asserts +; RUN: llc < %s -mtriple=thumbv8m.main-none-eabi -debug-only=machine-scheduler,post-RA-sched -print-before=machine-scheduler -o - 2>&1 > /dev/null | FileCheck %s + +; CHECK-LABEL: test_misched +; Pre and post ra machine scheduling +; CHECK: ********** MI Scheduling ********** +; CHECK: t2LDRi12 +; CHECK: Latency : 2 +; CHECK: ********** MI Scheduling ********** +; CHECK: t2LDRi12 +; CHECK: Latency : 2 + +define i32 @test_misched(i32* %ptr) "target-cpu"="cortex-m33" { +entry: + %l = load i32, i32* %ptr + store i32 0, i32* %ptr + ret i32 %l +} + +; CHECK-LABEL: test_rasched +; CHECK: Subtarget disables post-MI-sched. +; CHECK: ********** List Scheduling ********** + +define i32 @test_rasched(i32* %ptr) { +entry: + %l = load i32, i32* %ptr + store i32 0, i32* %ptr + ret i32 %l +} +