AMDGPU/SI: Use a better method for determining the largest pressure sets

[• tstellarAMD]

Summary

There are a few different sgpr pressure sets, but we only care about
the one which covers all of the sgprs. We were using hard-coded
register pressure set names to determine the reg set id for the
biggest sgpr set. However, we were using the wrong name, and this
method is pretty fragile, since the reg pressure set names may
change.

The new method just looks for the pressure set that contains the most
reg units and sets that set as our SGPR pressure set. We've also
adopted the same technique for determining our VGPR pressure set.

[arsenm]

lib/Target/AMDGPU/SIRegisterInfo.cpp

100 ↗

(On Diff #68585)

DenseMap?

118 ↗

(On Diff #68585)

Leftevor debug printing

981 ↗

(On Diff #68585)

Can use the constructor parameters instead of make_pair, but this is also the default

987–991 ↗

(On Diff #68585)

This is the same as isSGPRSet == isVGPRSet

1006 ↗

(On Diff #68585)

Ditto

1008 ↗

(On Diff #68585)

Save size

1012–1017 ↗

(On Diff #68585)

This is the same as isSGPRSet == isVGPRSet

[• tstellarAMD]

Address review comments, and remove code that was meant for a different patch.

[• tstellarAMD]

lib/Target/AMDGPU/SIRegisterInfo.cpp

100 ↗

(On Diff #68585)

I used a vector instead.

[arsenm]

LGTM

lib/Target/AMDGPU/SIRegisterInfo.cpp

111 ↗

(On Diff #68712)

Preincrement

lib/Target/AMDGPU/SIRegisterInfo.h

190–195 ↗

(On Diff #68712)

Why do these need to check that it's not in the other set? Are there still issues from VS_32 being allocatable?

[• tstellarAMD]

lib/Target/AMDGPU/SIRegisterInfo.h

190–195 ↗

(On Diff #68712)

Yes, we need to make sure we ignore VS_32.

[arsenm]

lib/Target/AMDGPU/SIRegisterInfo.h

190–195 ↗

(On Diff #68712)

But you should never see a VS_32 virtual register. Will this be fixed when making it unallocatable? This probably needs a comment/fixme

[• tstellarAMD]

lib/Target/AMDGPU/SIRegisterInfo.h

190–195 ↗

(On Diff #68712)

It's not about virtual registers, the VS_32 register pressure set contains all sgprs and vgprs, so the pressure set is affected by pretty much every register operand. I think making it unallocatable will fix this, but we probably still want an assert or something to making register sets with sgprs and vgprs is never created.

llvm/trunk/lib/Target/AMDGPU/SIMachineScheduler.cpp

 
 // SIScheduleDAGMI //
 
 SIScheduleDAGMI::SIScheduleDAGMI(MachineSchedContext *C) :
   ScheduleDAGMILive(C, make_unique<GenericScheduler>(C)) {
   SITII = static_cast<const SIInstrInfo*>(TII);
   SITRI = static_cast<const SIRegisterInfo*>(TRI);
 
-  VGPRSetID = SITRI->getVGPR32PressureSet();
-  SGPRSetID = SITRI->getSGPR32PressureSet();
+  VGPRSetID = SITRI->getVGPRPressureSet();
+  SGPRSetID = SITRI->getSGPRPressureSet();
 }
 
 SIScheduleDAGMI::~SIScheduleDAGMI() {
 }
 
 // Code adapted from scheduleDAG.cpp
 // Does a topological sort over the SUs.
 // Both TopDown and BottomUp

llvm/trunk/lib/Target/AMDGPU/SIRegisterInfo.h

 
 namespace llvm {
 
 class SISubtarget;
 class MachineRegisterInfo;
 
 struct SIRegisterInfo final : public AMDGPURegisterInfo {
 private:
-  unsigned SGPR32SetID;
-  unsigned VGPR32SetID;
+  unsigned SGPRSetID;
+  unsigned VGPRSetID;
   BitVector SGPRPressureSets;
   BitVector VGPRPressureSets;
 
   void reserveRegisterTuples(BitVector &, unsigned Reg) const;
   void classifyPressureSet(unsigned PSetID, unsigned Reg,
                            BitVector &PressureSets) const;
 
 public:
public:
   /// \brief Give the maximum number of SGPRs that can be used by \p WaveCount
   ///        concurrent waves.
   unsigned getNumSGPRsAllowed(const SISubtarget &ST, unsigned WaveCount) const;
 
   unsigned findUnusedRegister(const MachineRegisterInfo &MRI,
                               const TargetRegisterClass *RC,
                               const MachineFunction &MF) const;
 
-  unsigned getSGPR32PressureSet() const { return SGPR32SetID; };
-  unsigned getVGPR32PressureSet() const { return VGPR32SetID; };
+  unsigned getSGPRPressureSet() const { return SGPRSetID; };
+  unsigned getVGPRPressureSet() const { return VGPRSetID; };
 
   bool isVGPR(const MachineRegisterInfo &MRI, unsigned Reg) const;
 
+  bool isSGPRPressureSet(unsigned SetID) const {
+    return SGPRPressureSets.test(SetID) && !VGPRPressureSets.test(SetID);
+  }
+  bool isVGPRPressureSet(unsigned SetID) const {
+    return VGPRPressureSets.test(SetID) && !SGPRPressureSets.test(SetID);
+  }
+
 private:
   void buildScratchLoadStore(MachineBasicBlock::iterator MI,
                              unsigned LoadStoreOp, const MachineOperand *SrcDst,
                              unsigned ScratchRsrcReg, unsigned ScratchOffset,
                              int64_t Offset,
                              RegScavenger *RS) const;
 };
 
 } // End namespace llvm
 
 #endif

llvm/trunk/lib/Target/AMDGPU/SIRegisterInfo.cpp

void SIRegisterInfo::classifyPressureSet(unsigned PSetID, unsigned Reg,
   }
 }
 
 SIRegisterInfo::SIRegisterInfo() : AMDGPURegisterInfo(),
                                    SGPRPressureSets(getNumRegPressureSets()),
                                    VGPRPressureSets(getNumRegPressureSets()) {
   unsigned NumRegPressureSets = getNumRegPressureSets();
 
-  SGPR32SetID = NumRegPressureSets;
-  VGPR32SetID = NumRegPressureSets;
-  for (unsigned i = 0; i < NumRegPressureSets; ++i) {
-    if (strncmp("SGPR_32", getRegPressureSetName(i), 7) == 0)
-      SGPR32SetID = i;
-    else if (strncmp("VGPR_32", getRegPressureSetName(i), 7) == 0)
-      VGPR32SetID = i;
+  SGPRSetID = NumRegPressureSets;
+  VGPRSetID = NumRegPressureSets;
 
+  for (unsigned i = 0; i < NumRegPressureSets; ++i) {
     classifyPressureSet(i, AMDGPU::SGPR0, SGPRPressureSets);
     classifyPressureSet(i, AMDGPU::VGPR0, VGPRPressureSets);
   }
-  assert(SGPR32SetID < NumRegPressureSets &&
-         VGPR32SetID < NumRegPressureSets);
+
+  // Determine the number of reg units for each pressure set.
+  std::vector<unsigned> PressureSetRegUnits(NumRegPressureSets, 0);
+  for (unsigned i = 0, e = getNumRegUnits(); i != e; ++i) {
+    const int *PSets = getRegUnitPressureSets(i);
+    for (unsigned j = 0; PSets[j] != -1; ++j) {
+      PressureSetRegUnits[PSets[j]]++;
+    }
+  }
+
+  unsigned VGPRMax = 0, SGPRMax = 0;
+  for (unsigned i = 0; i < NumRegPressureSets; ++i) {
+    if (isVGPRPressureSet(i) && PressureSetRegUnits[i] > VGPRMax) {
+      VGPRSetID = i;
+      VGPRMax = PressureSetRegUnits[i];
+      continue;
+    }
+    if (isSGPRPressureSet(i) && PressureSetRegUnits[i] > SGPRMax) {
+      SGPRSetID = i;
+      SGPRMax = PressureSetRegUnits[i];
+    }
+  }
+
+  assert(SGPRSetID < NumRegPressureSets &&
+         VGPRSetID < NumRegPressureSets);
 }
 
 void SIRegisterInfo::reserveRegisterTuples(BitVector &Reserved, unsigned Reg) const {
   MCRegAliasIterator R(Reg, this, true);
 
   for (; R.isValid(); ++R)
     Reserved.set(*R);
 }