[PATCH] D37267: AMDGPU: Use set for tracked registers

[Matt Arsenault via Phabricator via llvm-commits]

arsenm created this revision.
Herald added subscribers: t-tye, tpr, dstuttard, yaxunl, nhaehnle, wdng, kzhuravl.

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.


https://reviews.llvm.org/D37267

Files:
  lib/Target/AMDGPU/SILoadStoreOptimizer.cpp


Index: lib/Target/AMDGPU/SILoadStoreOptimizer.cpp
===================================================================
--- lib/Target/AMDGPU/SILoadStoreOptimizer.cpp
+++ lib/Target/AMDGPU/SILoadStoreOptimizer.cpp
@@ -141,36 +141,38 @@
   }
 }
 
-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()));
+  // RAW or WAR - cannot reorder
+  // WAW - cannot reorder
+  // RAR - safe to reorder
+  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;
@@ -248,22 +250,25 @@
   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) {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D37267.113116.patch
Type: text/x-patch
Size: 3467 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170829/52e8b9dd/attachment.bin>