[X86][Atom] Convert Atom scheduler model to SchedRW (PR32431)

[RKSimon]

Summary

Atom is the only x86 target that still uses schedule itineraries, if we can remove this then we can begin the work on removing x86 itineraries. I've also found that it will help with PR36550.

I've focussed on matching the existing model as closely as possible (relying on the schedule tests), PR36895 indicated a lot of these were incorrect but we can just as easily fix these after this patch as before. Hopefully we can get llvm-exegesis to help here,

This also required me to generalize the nop padding pass to work with TargetSchedModel instead of calling TargetInstrInfo::getInstrLatency directly - this could possibly be pre-committed if people are happy with it.

There are a few instructions that rely on itinerary scheduling (mainly push/pop/return) of multiple resource stages, but I don't think any of these are show stoppers.

There are also a few codegen changes that seem related to the post-ra scheduler acting a little differently, I haven't tracked these down but they don't seem critical.

NOTE: I don't have access to any Atom hardware, so this hasn't been tested in the wild.

[craig.topper]

test/CodeGen/X86/sse-schedule.ll

6135 ↗

(On Diff #141833)

Why did the number of nops change?

test/CodeGen/X86/sse2-schedule.ll

10709 ↗

(On Diff #141833)

Any idea why the paddw wasn't commuted before to make the movdqa unnecessary?

[RKSimon]

test/CodeGen/X86/sse-schedule.ll

6135 ↗

(On Diff #141833)

The actual test nop is inside an inlineasm, which can't be scheduled. Itineraries' getInstrLatency default to 0cy, but TargetSchedModel::computeInstrLatency defaults to 1cy.

TBH in this case, I think the schedrw is slightly better but only by chance.

test/CodeGen/X86/sse2-schedule.ll

10709 ↗

(On Diff #141833)

Sorry, no - with the patch MachineCSE is performing the commute.

[RKSimon]

rebased

[RKSimon]

test/CodeGen/X86/sse2-schedule.ll

10709 ↗

(On Diff #141833)

Both with/without the patch MachineCSE the paddw is being commutated twice (there and back again....). Without the patch I'm then seeing one more commutations coming from TwoAddressInstructionPass, and that leaves us with the movdqa.

[craig.topper]

LGTM

llvm/trunk/lib/Target/X86/X86PadShortFunction.cpp

 
 #include "X86.h"
 #include "X86InstrInfo.h"
 #include "X86Subtarget.h"
 #include "llvm/ADT/Statistic.h"
 #include "llvm/CodeGen/MachineFunctionPass.h"
 #include "llvm/CodeGen/MachineInstrBuilder.h"
 #include "llvm/CodeGen/Passes.h"
-#include "llvm/CodeGen/TargetInstrInfo.h"
+#include "llvm/CodeGen/TargetSchedule.h"
 #include "llvm/IR/Function.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/raw_ostream.h"
 
 using namespace llvm;
 
 #define DEBUG_TYPE "x86-pad-short-functions"
 
struct VisitedBBInfo {
     VisitedBBInfo() : HasReturn(false), Cycles(0) {}
     VisitedBBInfo(bool HasReturn, unsigned int Cycles)
       : HasReturn(HasReturn), Cycles(Cycles) {}
   };
 
   struct PadShortFunc : public MachineFunctionPass {
     static char ID;
     PadShortFunc() : MachineFunctionPass(ID)
-                   , Threshold(4), STI(nullptr), TII(nullptr) {}
+                   , Threshold(4) {}
 
     bool runOnMachineFunction(MachineFunction &MF) override;
 
     MachineFunctionProperties getRequiredProperties() const override {
       return MachineFunctionProperties().set(
           MachineFunctionProperties::Property::NoVRegs);
     }
 
private:
 
     // ReturnBBs - Maps basic blocks that return to the minimum number of
     // cycles until the return, starting from the entry block.
     DenseMap<MachineBasicBlock*, unsigned int> ReturnBBs;
 
     // VisitedBBs - Cache of previously visited BBs.
     DenseMap<MachineBasicBlock*, VisitedBBInfo> VisitedBBs;
 
-    const X86Subtarget *STI;
-    const TargetInstrInfo *TII;
+    TargetSchedModel TSM;
   };
 
   char PadShortFunc::ID = 0;
 }
 
 FunctionPass *llvm::createX86PadShortFunctions() {
   return new PadShortFunc();
 }
 
 /// runOnMachineFunction - Loop over all of the basic blocks, inserting
 /// NOOP instructions before early exits.
 bool PadShortFunc::runOnMachineFunction(MachineFunction &MF) {
   if (skipFunction(MF.getFunction()))
     return false;
 
-  if (MF.getFunction().optForSize()) {
+  if (MF.getFunction().optForSize())
     return false;
-  }
 
-  STI = &MF.getSubtarget<X86Subtarget>();
-  if (!STI->padShortFunctions())
+  if (!MF.getSubtarget<X86Subtarget>().padShortFunctions())
     return false;
 
-  TII = STI->getInstrInfo();
+  TSM.init(&MF.getSubtarget());
 
   // Search through basic blocks and mark the ones that have early returns
   ReturnBBs.clear();
   VisitedBBs.clear();
   findReturns(&MF.front());
 
   bool MadeChange = false;
 
for (MachineInstr &MI : *MBB) {
     // functions do not count because the called function will be padded,
     // if necessary.
     if (MI.isReturn() && !MI.isCall()) {
       VisitedBBs[MBB] = VisitedBBInfo(true, CyclesToEnd);
       Cycles += CyclesToEnd;
       return true;
     }
 
-    CyclesToEnd += TII->getInstrLatency(STI->getInstrItineraryData(), MI);
+    CyclesToEnd += TSM.computeInstrLatency(&MI);
   }
 
   VisitedBBs[MBB] = VisitedBBInfo(false, CyclesToEnd);
   Cycles += CyclesToEnd;
   return false;
 }
 
 /// addPadding - Add the given number of NOOP instructions to the function
 /// just prior to the return at MBBI
 void PadShortFunc::addPadding(MachineBasicBlock *MBB,
                               MachineBasicBlock::iterator &MBBI,
                               unsigned int NOOPsToAdd) {
   DebugLoc DL = MBBI->getDebugLoc();
+  unsigned IssueWidth = TSM.getIssueWidth();
 
-  while (NOOPsToAdd-- > 0) {
-    BuildMI(*MBB, MBBI, DL, TII->get(X86::NOOP));
-    BuildMI(*MBB, MBBI, DL, TII->get(X86::NOOP));
-  }
+  for (unsigned i = 0, e = IssueWidth * NOOPsToAdd; i != e; ++i)
+    BuildMI(*MBB, MBBI, DL, TSM.getInstrInfo()->get(X86::NOOP));
 }