[all-commits] [llvm/llvm-project] 66b4c2: [RFC][AMDGPU][InsertWaitcnts] Move `WaitcntBracket...

[Pierre van Houtryve via All-commits]

  Branch: refs/heads/users/pierre-vh/split-waitcntbrackets
  Home:   https://github.com/llvm/llvm-project
  Commit: 66b4c2a23af4e2d61d708ad96a9da7fe3bcf3480
      https://github.com/llvm/llvm-project/commit/66b4c2a23af4e2d61d708ad96a9da7fe3bcf3480
  Author: pvanhout <pierre.vanhoutryve at amd.com>
  Date:   2026-07-17 (Fri, 17 Jul 2026)

  Changed paths:
    A llvm/lib/Target/AMDGPU/AMDGPUWaitcntTracking.cpp
    A llvm/lib/Target/AMDGPU/AMDGPUWaitcntTracking.h
    M llvm/lib/Target/AMDGPU/CMakeLists.txt
    M llvm/lib/Target/AMDGPU/SIInsertWaitcnts.cpp
    M llvm/lib/Target/AMDGPU/SIInstrInfo.h

  Log Message:
  -----------
  [RFC][AMDGPU][InsertWaitcnts] Move `WaitcntBrackets` to a separate file

My previous stack of patches de-tangled quite a bit of InsertWaitcnt,
and now `WaitcntBrackets` can be moved into a separate file fairly
easily. I'm proposing this as a RFC. While I think this is a net
positive, I don't want to force this through if others feel it's unnecessary.

Implementation:
This pretty much moves `WaitcntBrackets` as-is, with a few changes:

- Instead of having a pointer to `SIInsertWaitcnts` for the "context"
  there is now a "Info" class that `SIInsertWaitcnts` implements.
  This enforces a small separation between the classes, not enough to
  force a massive redesign, but enough that we can't just access the
  entire state of `InsertWaitcnts` at will.
- Some unused/unnecessary functions were removed or made private.
- I reorganized the code a bit: moved all private functions together,
  moved method implementations to the .cpp if they had more than 2 lines of code.
  The goal is to keep things somewhat organized so it's easy to glance at the
  file and see how this works.

Motivation: While we have no use for a reusable `WaitcntBrackets` yet, separating it
in another file and acting as-if it's meant to be reusable has the big advantage of
removing the "god-object" connection between `WaitcntBrackets` and `WaitcntGenerator` via
the `SIInsertWaitcnt` class. This helps separate concerns better which, IMHO, will reduce
the maintenance burden a bit.
This also allows for splitting the decl and definition of `WaitcntBrackets` method, making
the API easier to digest at a glance. It gives the implementation a bit more room to grow
as well.

Future direction: I can imagine splitting `WaitcntGenerator` similarly, but I am not sure
yet if that's desirable so we will see.
Now that `WaitcntBrackets` is split up, it'd also be nice too move towards a simpler API
for it so that the way things are tracked is opaque to `InsertWaitcnt`. Maybe then we can
start playing with alternative implementations of `WaitcntBrackets`, like having a more
precise, newer tracking system that can be selected dynamically. That's something I'd like
to eventually explore as there's been a mention of having a "per-event timeline instead of per-counter timeline"
for many years.

Assisted-By: Claude Sonnet 5
Claude was only used to generate the boilerplate of the files (create header/cpp + add to CMakeLists).
I wrote all the code/logic myself.



To unsubscribe from these emails, change your notification settings at https://github.com/llvm/llvm-project/settings/notifications