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

[via llvm-commits]

vporpo wrote:

> 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.

Both points where discussed and resolved in the design doc discussion, I am not sure why you are bringing them up here.

But anyway to help others follow the discussion, regarding (1) you have already commented in the doc about how this will work: once all scores migrated to the Score class there won't be a need for "unsigned" scores, this is quite obvious. Regarding (2) this was pretty much decided, we will go with the approach that uses the least amount of memory, which is what other reviewers also preferred.

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

Because these changes can introduce subtle bugs, and having a separate commit for each new function will help with root-causing. It also helps with code reviews as we can focus on one API function at a time.

Regarding this particular patch, do you have any specific changes in mind ?

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