[Openmp-commits] [PATCH] D156996: [OpenMP][AMDGPU] Add Envar for controlling HSA busy queue tracking

[Michael Halkenhäuser via Phabricator via Openmp-commits]

mhalk added a comment.

Just as a heads-up, what I'll be looking at.
Maybe this far away from what you had in mind.

One thing I noticed when refactoring this to the selection dependent on "user count" is that:
In conjunction with an early exit, which I definitely wanted, I think we can eliminate one iteration on the `for` loop?!
(Alleviates the re-introduced complexity a bit, I guess.)
I'm positive we find a good solution to this.

Changed `isBusy` to `getUserCount` and added `isInitialized`.

@kevinsala Let me know what you think.
Also, tell me if I should just update the diff rather than posting code snippets.

  inline Error assignNextQueue(AMDGPUStreamTy *Stream) {
      uint32_t SelectedIndex = NextQueue % MaxNumQueues;
  
      if (OMPX_QueueTracking) {
        // Take utilization into account, begin at SelectedIndex
        uint32_t Index = SelectedIndex;
  
        for (uint32_t I = 1; I < MaxNumQueues; ++I) {
          // Early exit when an initialized queue is idle
          if (Queues[SelectedIndex].isInitialized() &&
              Queues[SelectedIndex].getUserCount() == 0)
            break;
  
          // Increment Index & potential wrap around
          if (++Index >= MaxNumQueues)
            Index = 0;
  
          // Update the least contested queue
          if (Queues[SelectedIndex].getUserCount() > Queues[Index].getUserCount())
            SelectedIndex = Index;
        }
      }
  
      // Make sure we assign an initialized queue, then add user & assign
      if (auto Err = Queues[SelectedIndex].init(Agent, QueueSize))
        return Err;
      Queues[SelectedIndex].addUser();
      Stream->Queue = &Queues[SelectedIndex];
  
      // Move cursor to the next queue
      ++NextQueue;
      return Plugin::success();
    }


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156996