[Scheduling][ARM] Consistently enable PostRA Machine scheduling

[dmgreen]

Summary

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.

[samparker]

llvm/lib/Target/ARM/ARMSubtarget.cpp

389

I know this is the existing logic, but what are we trying to do here with respect to IT blocks? If we're worried about IT blocks, like the comment suggests, shouldn't we be returning isThumb1Only()? Also, if we're in arm mode then we won't have them, but are we still concerned about predicated instructions (I assume not)?

[dmgreen]

llvm/lib/Target/ARM/ARMSubtarget.cpp

389

I think the comment should really say something like "Thumb1 cores are too simple to benefit from scheduling". I'm not sure what it means by IT blocks.

I will change that. I don't think it would be particularly bad thing to do post-ra scheduling on thumb1 cores, it just won't benefit from it either, so we might as well save the compile time.

[dmgreen]

Updated comment

[samparker]

llvm/lib/Target/ARM/ARMTargetMachine.cpp

520

nit: opportunity

522

Should we instead still skip this at -O0?

[dmgreen]

Only enable when getOptLevel() != CodeGenOpt::None

[samparker]

LGTM

llvm/include/llvm/CodeGen/TargetSubtargetInfo.h

public:
   virtual bool enableJoinGlobalCopies() const;
 
   /// True if the subtarget should run a scheduler after register allocation.
   ///
   /// By default this queries the PostRAScheduling bit in the scheduling model
   /// 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;
 
   /// True if the subtarget should run the indirectbr expansion pass.
   virtual bool enableIndirectBrExpand() const;
 
   /// Override generic scheduling policy within a region.
   ///

llvm/lib/CodeGen/MachineScheduler.cpp

 
 bool PostMachineScheduler::runOnMachineFunction(MachineFunction &mf) {
   if (skipFunction(mf.getFunction()))
     return false;
 
   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;
   }
   LLVM_DEBUG(dbgs() << "Before post-MI-sched:\n"; mf.print(dbgs()));
 
   // Initialize the context of the pass.
   MF = &mf;
   MLI = &getAnalysis<MachineLoopInfo>();

llvm/lib/CodeGen/TargetSubtargetInfo.cpp

 bool TargetSubtargetInfo::enableAdvancedRASplitCost() const {
   return false;
 }
 
 bool TargetSubtargetInfo::enablePostRAScheduler() const {
   return getSchedModel().PostRAScheduler;
 }
 
+bool TargetSubtargetInfo::enablePostRAMachineScheduler() const {
+  return enableMachineScheduler() && enablePostRAScheduler();
+}
+
 bool TargetSubtargetInfo::useAA() const {
   return false;
 }
 
 void TargetSubtargetInfo::mirFileLoaded(MachineFunction &MF) const { }

llvm/lib/Target/ARM/ARMSubtarget.h

public:
   unsigned getMispredictionPenalty() const;
 
   /// Returns true if machine scheduler should be enabled.
   bool enableMachineScheduler() const override;
 
   /// 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; }
 
   // enableAtomicExpand- True if we need to expand our atomics.
   bool enableAtomicExpand() const override;
 
   /// getInstrItins - Return the instruction itineraries based on subtarget

llvm/lib/Target/ARM/ARMSubtarget.cpp

if (isMClass() && hasMinSize())
     return false;
   // Enable the MachineScheduler before register allocation for subtargets
   // with the use-misched feature.
   return useMachineScheduler();
 }
 
 // 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();
 }
 
 bool ARMSubtarget::enableAtomicExpand() const { return hasAnyDataBarrier(); }
 
 bool ARMSubtarget::useStride4VFPs() const {
   // For general targets, the prologue can grow when VFPs are allocated with
   // stride 4 (more vpush instructions). But WatchOS uses a compact unwind

llvm/lib/Target/ARM/ARMTargetMachine.h

bool isTargetHardFloat() const {
     return TargetTriple.getEnvironment() == Triple::GNUEABIHF ||
            TargetTriple.getEnvironment() == Triple::MuslEABIHF ||
            TargetTriple.getEnvironment() == Triple::EABIHF ||
            (TargetTriple.isOSBinFormatMachO() &&
             TargetTriple.getSubArch() == Triple::ARMSubArch_v7em) ||
            TargetTriple.isOSWindows() ||
            TargetABI == ARMBaseTargetMachine::ARM_ABI_AAPCS16;
   }
+
+  bool targetSchedulesPostRAScheduling() const override { return true; };
 };
 
 /// ARM/Thumb little endian target machine.
 ///
 class ARMLETargetMachine : public ARMBaseTargetMachine {
 public:
   ARMLETargetMachine(const Target &T, const Triple &TT, StringRef CPU,
                      StringRef FS, const TargetOptions &Options,

llvm/lib/Target/ARM/ARMTargetMachine.cpp

ARMBETargetMachine::ARMBETargetMachine(const Target &T, const Triple &TT,
     : ARMBaseTargetMachine(T, TT, CPU, FS, Options, RM, CM, OL, false) {}
 
 namespace {
 
 /// ARM Code Generator Pass Configuration Options.
 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<ARMBaseTargetMachine>();
   }
 
   ScheduleDAGInstrs *
   createMachineScheduler(MachineSchedContext *C) const override {
     ScheduleDAGMILive *DAG = createGenericSchedLive(C);
if (getOptLevel() != CodeGenOpt::None) {
     }));
 
     addPass(createIfConverter([](const MachineFunction &MF) {
       return !MF.getSubtarget<ARMSubtarget>().isThumb1Only();
     }));
   }
   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() {
   addPass(createThumb2SizeReductionPass());
 
   // Constant island pass work on unbundled instructions.
   addPass(createUnpackMachineBundles([](const MachineFunction &MF) {
     return MF.getSubtarget<ARMSubtarget>().isThumb2();

llvm/test/CodeGen/ARM/O3-pipeline.ll

 ; CHECK-NEXT:      MachineDominator Tree Construction
 ; CHECK-NEXT:      Machine Natural Loop Construction
 ; CHECK-NEXT:      Machine Block Frequency Analysis
 ; CHECK-NEXT:      If Converter
 ; CHECK-NEXT:      MVE VPT block insertion pass
 ; 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
 ; CHECK-NEXT:      MachinePostDominator Tree Construction
 ; CHECK-NEXT:      Branch Probability Basic Block Placement
 ; CHECK-NEXT:      Thumb2 instruction size reduce pass
 ; CHECK-NEXT:      Unpack machine instruction bundles
 ; CHECK-NEXT:      optimise barriers pass

llvm/test/CodeGen/ARM/cortex-a57-misched-ldm-wrback.ll

 ; 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
 @b = global i32 0, align 4
 @c = global i32 0, align 4
 
 ; CHECK:       ********** MI Scheduling **********
 ; We need second, post-ra scheduling to have LDM instruction combined from single-loads

llvm/test/CodeGen/ARM/cortex-a57-misched-ldm.ll

 ; 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
 ; CHECK:       ********** MI Scheduling **********
 ; CHECK:       LDMIA
 ; CHECK:       rdefs left
 ; CHECK-NEXT:  Latency            : 3
 ; CHECK:       Successors:

llvm/test/CodeGen/ARM/cortex-a57-misched-stm-wrback.ll

 ; 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 **********
 ; We need second, post-ra scheduling to have STM instruction combined from single-stores
 ; CHECK:       ********** MI Scheduling **********
 ; CHECK:       schedule starting
 ; CHECK:       STMIA_UPD
 ; CHECK:       rdefs left

llvm/test/CodeGen/ARM/cortex-a57-misched-stm.ll

 ; 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 **********
 ; We need second, post-ra scheduling to have STM instruction combined from single-stores
 ; CHECK:       ********** MI Scheduling **********
 ; CHECK:       schedule starting
 ; CHECK:       STMIB
 ; CHECK:       rdefs left

llvm/test/CodeGen/ARM/cortex-a57-misched-vldm-wrback.ll

 ; 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
 @b = global double 0.0, align 4
 @c = global double 0.0, align 4
 
 ; CHECK:       ********** MI Scheduling **********
 ; We need second, post-ra scheduling to have VLDM instruction combined from single-loads

llvm/test/CodeGen/ARM/cortex-a57-misched-vldm.ll

 ; 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
 ; CHECK:       ********** MI Scheduling **********
 ; CHECK:       VLDMDIA
 ; CHECK:       rdefs left
 ; CHECK-NEXT:  Latency            : 6
 ; CHECK:       Successors:

llvm/test/CodeGen/ARM/cortex-a57-misched-vstm-wrback.ll

 ; 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
 ; CHECK:       ********** MI Scheduling **********
 ; CHECK:       schedule starting
 ; CHECK:       VSTMDIA_UPD
 ; CHECK:       rdefs left
 ; CHECK-NEXT:  Latency            : 4

llvm/test/CodeGen/ARM/cortex-a57-misched-vstm.ll

 ; 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
 ; CHECK:       ********** MI Scheduling **********
 ; CHECK:       schedule starting
 ; CHECK:       VSTMDIA
 ; CHECK:       rdefs left
 ; CHECK-NEXT:  Latency            : 2

llvm/test/CodeGen/ARM/postrasched.ll

+; 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
+}
+