[PATCH] D159452: [AMDGPU] Machine scheduler should take care of the 'amdgpu-num-vgp' function attribute

[Alexander via Phabricator via llvm-commits]

alex-t added a comment.

In D159452#4638703 <https://reviews.llvm.org/D159452#4638703>, @kerbowa wrote:

> I thought this already was used in the excess VGPRs pressure and wave-per-eu is that just not true?

trying to give a detailed response:

We measure the occupancy after the current schedule stage by querying the RegisterPressure class:

  unsigned WavesAfter =
        std::min(TargetOccupancy, PressureAfter.getOccupancy(ST));

which does not care about attributes set on function.
Then couple of lines after we query the right method which returns the real number of VGPRs available
for the function and we mark the region as excess RP accordingly:

  unsigned MaxVGPRs = ST.getMaxNumVGPRs(MF);
  if (PressureAfter.getVGPRNum(false) > MaxVGPRs ||
         *****
      DAG.RegionsWithExcessRP[RegionIdx] = true;
    }

the problem is that we decide if we need to revert the schedule based on both - excess RP and WavesAfter.
The latter is computed w/o the knowledge of the restrictions applied by the attribute:

  bool GCNSchedStage::mayCauseSpilling(unsigned WavesAfter) {
    if (WavesAfter <= MFI.getMinWavesPerEU() &&
        !PressureAfter.less(ST, PressureBefore) &&
        isRegionWithExcessRP()) {
      LLVM_DEBUG(dbgs() << "New pressure will result in more spilling.\n");
      return true;
    }

So, we never revert the "bad" schedule!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D159452/new/

https://reviews.llvm.org/D159452