AMDGPU: Use set for tracked registers

[arsenm]

Summary

The majority of the time spent in the pass checking
for the register reads. Rather than searching all of
the defined registers for uses in each instruction,
use a set of defined registers and check the operands
of the instruction.

This process still is algorithmically not great,
but with the additional trick of skipping the analysis
for addresses with one use, this brings one slow
testcase into a reasonable range.

[rampitec]

LGTM, just remove blank line 253.

lib/Target/AMDGPU/SILoadStoreOptimizer.cpp

253

Extra blank line.

[javed.absar]

lib/Target/AMDGPU/SILoadStoreOptimizer.cpp

148

This doesn't seem necessary.

149

Generally one should have a message string with the assertion so that when it fails, it is clear why it did e.g. assert( Inserted && "No valid defs")

[arsenm]

r312206

lib/Target/AMDGPU/SILoadStoreOptimizer.cpp

148

It prevents people from trying to make this a non-SSA pass again. I've replaced that with an isSSA assert at the beginning of the pass

lib/Target/AMDGPU/SILoadStoreOptimizer.cpp

static void moveInstsAfter(MachineBasicBlock::iterator I,
   MachineBasicBlock *MBB = I->getParent();
   ++I;
   for (MachineInstr *MI : InstsToMove) {
     MI->removeFromParent();
     MBB->insert(I, MI);
   }
 }
 
-static void addDefsToList(const MachineInstr &MI,
-                          SmallVectorImpl<const MachineOperand *> &Defs) {
+static void addDefsToList(const MachineInstr &MI, DenseSet<unsigned> &Defs) {
+  // XXX: Should this be looking for implicit defs?
   for (const MachineOperand &Def : MI.defs()) {
-    Defs.push_back(&Def);
+    bool Inserted = Defs.insert(Def.getReg()).second;
+    (void)Inserted;
+    assert(Inserted);
   }
 }
 
 static bool memAccessesCanBeReordered(MachineBasicBlock::iterator A,
                                       MachineBasicBlock::iterator B,
                                       const SIInstrInfo *TII,
                                       AliasAnalysis * AA) {
-  return (TII->areMemAccessesTriviallyDisjoint(*A, *B, AA) ||
-    // RAW or WAR - cannot reorder
-    // WAW - cannot reorder
-    // RAR - safe to reorder
-    !(A->mayStore() || B->mayStore()));
+  return !(A->mayStore() || B->mayStore()) ||
+    TII->areMemAccessesTriviallyDisjoint(*A, *B, AA);
 }
 
 // Add MI and its defs to the lists if MI reads one of the defs that are
 // already in the list. Returns true in that case.
 static bool
 addToListsIfDependent(MachineInstr &MI,
-                      SmallVectorImpl<const MachineOperand *> &Defs,
+                      DenseSet<unsigned> &Defs,
                       SmallVectorImpl<MachineInstr*> &Insts) {
-  for (const MachineOperand *Def : Defs) {
-    bool ReadDef = MI.readsVirtualRegister(Def->getReg());
-    // If ReadDef is true, then there is a use of Def between I
-    // and the instruction that I will potentially be merged with. We
-    // will need to move this instruction after the merged instructions.
-    if (ReadDef) {
+  for (MachineOperand &Use : MI.operands()) {
+    // If one of the defs is read, then there is a use of Def between I and the
+    // instruction that I will potentially be merged with. We will need to move
+    // this instruction after the merged instructions.
+
+    if (Use.isReg() && Use.readsReg() && Defs.count(Use.getReg())) {
       Insts.push_back(&MI);
       addDefsToList(MI, Defs);
       return true;
     }
   }
 
   return false;
 }
if (isUInt<8>(OffsetDiff)) {
     CI.Offset0 = EltOffset0 - CI.BaseOff / CI.EltSize;
     CI.Offset1 = EltOffset1 - CI.BaseOff / CI.EltSize;
     return true;
   }
 
   return false;
 }
 
+
 bool SILoadStoreOptimizer::findMatchingDSInst(CombineInfo &CI) {
-  MachineBasicBlock::iterator E = CI.I->getParent()->end();
+  MachineBasicBlock *MBB = CI.I->getParent();
+  MachineBasicBlock::iterator E = MBB->end();
   MachineBasicBlock::iterator MBBI = CI.I;
 
   int AddrIdx = AMDGPU::getNamedOperandIdx(CI.I->getOpcode(),
                                            AMDGPU::OpName::addr);
   const MachineOperand &AddrReg0 = CI.I->getOperand(AddrIdx);
 
   // We only ever merge operations with the same base address register, so don't
   // bother scanning forward if there are no other uses.
-  if (MRI->hasOneNonDBGUse(AddrReg0.getReg()))
+  if (TargetRegisterInfo::isPhysicalRegister(AddrReg0.getReg()) ||
+      MRI->hasOneNonDBGUse(AddrReg0.getReg()))
     return false;
 
   ++MBBI;
 
-  SmallVector<const MachineOperand *, 8> DefsToMove;
+  DenseSet<unsigned> DefsToMove;
   addDefsToList(*CI.I, DefsToMove);
 
   for ( ; MBBI != E; ++MBBI) {
     if (MBBI->getOpcode() != CI.I->getOpcode()) {
       // This is not a matching DS instruction, but we can keep looking as
       // long as one of these conditions are met:
       // 1. It is safe to move I down past MBBI.
       // 2. It is safe to move MBBI down past the instruction that I will