GPUOpen-Tools / gpu_performance_api

GPU Performance API for AMD GPUs
MIT License
250 stars 46 forks source link

Add unit test for OpenCounterContext #72

Closed jcortell68 closed 1 year ago

jcortell68 commented 1 year ago

The unit tests should have a check that GpaCounterLibOpenCounterContext() can be called with that various possibilities for GpaOpenContextBits. There were gaps and bugs with the GPUs I was adding but that didn't reveal itself until a client tried asking for the hw counters when creating a counter context. This could and should have been caught by the GPA unit test.

Also, I obsoleted two GpaOpenContextBits enum values, as they are pointless with the latest GPA. kGpaOpenContextHideSoftwareCountersBit was made pointless when support for SW counters was removed in GPA 3.0. Similarly, it appears at one point GPA dished out the HW counters by default and excluded them only on demand. Now they are excluded by default, and shown only on demand. That made kGpaOpenContextHideHardwareCountersBit pointless. Both those enums have been renamed to have an _obsolete suffix. This aproach maintains backwards runtime compatibility, but building against the latest GPA will make clients cleanup (remove) any use of those now pointless flags. This is beneficial to them and allows us to improve the GPA interface and implementation.

jcortell68 commented 1 year ago

Since the change may require end-users to update their code, Can you add a "Main" section to the release notes to clarify that these enums were updated in main and not in the most recent release? The notes below should be sufficient:

I added the section but personally thought "Main" was a little but unclear. I used "Upcoming Release" for the section title. But I'll happily change it if you prefer "Main"