[llvm] [AMDGPU][SIInsertWaitcnts][NFC] Introduce Counter class (PR #190271)

Sameer Sahasrabuddhe via llvm-commits llvm-commits at lists.llvm.org
Thu Apr 16 02:37:03 PDT 2026


ssahasra wrote:

> The document shared internally is already listing both the problem and the proposed solution, i.e., the introduction of the `Counter` class as a first step and the introduction of the `Score` class tied to each counter as a second step. It is even listing the actual code for the `Counter` class with the complete API, where none of the TODO functions are present. So I am not sure which part of the "direction" is not clear to you.

1. The document shows the `Score` class with an `unsigned get()` method, and the _future direction_ section shows a  `Counter`  class with a `bool contains(unsigned Score)` method. Comments in two places say that this will be addressed "eventually".
2. The section about tying counters to scores is undecided. It proposes two options, but a review comment actually expresses preference for a third option. This concern is about the impact on memory overhead, and the discussion so far seems to have not concluded yet.

So no, the document does not show the whole solution and no it does not show the future direction. The document, just like this patch, simply moves stuff around to feel like it is encapsulating things, while the type and name of the things being encapsulated are leaked by function signatures.

> The whole patch series I shared [vporpo at 4f12ae1](https://github.com/vporpo/llvm-project/commit/4f12ae1e2c69cb59f698127417910178fcf553c4) is just +127 -88 (and it includes this patch!).

Then why not just make one patch and review it together? What's the need to make micro patches?

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


More information about the llvm-commits mailing list