[llvm] [AMDGPU] Prefer lower total register usage in regions with spilling (PR #71882)

Jeffrey Byrnes via llvm-commits llvm-commits at lists.llvm.org
Fri Dec 1 15:18:27 PST 2023


================
@@ -102,20 +102,56 @@ bool GCNRegPressure::less(const GCNSubtarget &ST,
     std::min(MaxOccupancy,
              ST.getOccupancyWithNumVGPRs(O.getVGPRNum(ST.hasGFX90AInsts())));
 
+  unsigned MaxVGPRs = ST.getMaxNumVGPRs(MF);
+  unsigned MaxSGPRs = ST.getMaxNumSGPRs(MF);
+
+  bool ExcessVGPR = getVGPRNum(false) > MaxVGPRs || getAGPRNum() > MaxVGPRs;
+  bool ExcessSGPR = getSGPRNum() > MaxSGPRs;
+  bool OtherExcessVGPR =
+      O.getVGPRNum(false) > MaxVGPRs || O.getAGPRNum() > MaxVGPRs;
----------------
jrbyrnes wrote:

Thanks for pointing this out -- 

These conditions were reformulated from existing code in `checkScheduling`

```
  unsigned MaxVGPRs = ST.getMaxNumVGPRs(MF);
  unsigned MaxSGPRs = ST.getMaxNumSGPRs(MF);

  if (PressureAfter.getVGPRNum(false) > MaxVGPRs ||
      PressureAfter.getAGPRNum() > MaxVGPRs ||
      PressureAfter.getSGPRNum() > MaxSGPRs) {
    DAG.RescheduleRegions[RegionIdx] = true;
    DAG.RegionsWithHighRP[RegionIdx] = true;
    DAG.RegionsWithExcessRP[RegionIdx] = true;
  }
 
```

This appears to be wrong in unified register file case. A scheduling region with pressure - 276 VGPR, 0 AGPR is not flagged as having excess with min-waves-per-eu of 1 even though the ISA states "A wave may have up to 512 total VGPRs, 256 of each type". As it stands, it will allow up to 512 VGPR before flagging. I'll address this in a separate patch.

https://github.com/llvm/llvm-project/pull/71882


More information about the llvm-commits mailing list