level-zero icon indicating copy to clipboard operation
level-zero copied to clipboard

`ZES_STRUCTURE_TYPE_BASE_STATE` should not exist

Open Kerilk opened this issue 2 years ago • 4 comments

The enum entry ZES_STRUCTURE_TYPE_BASE_STATE should not exist as zes_base_state_t is supposed to be abstract. see: https://github.com/oneapi-src/level-zero/blob/bb7fff05b801e26c3d7858e03e509d1089914d59/include/zes_api.h#L131 and: https://github.com/oneapi-src/level-zero/blob/bb7fff05b801e26c3d7858e03e509d1089914d59/include/zes_api.h#L158-L165

Kerilk avatar Mar 07 '22 23:03 Kerilk

@Kerilk thanks. Idea is that the base stype is used to query which struct is being pointed at by pNext. We dont have code yet on Sysman using it I believe, but we have it in the Core API. See here for how this is used:

https://github.com/intel/compute-runtime/blob/8f5dd3cff53859f6494f8ede67fb5753dbc9e3ca/level_zero/core/source/helpers/properties_parser.h#L77

jandres742 avatar Mar 07 '22 23:03 jandres742

The base structures in the Core API do not have corresponding enum entries, and of the 5 base types in sysman, only base state has an enum entry.

Kerilk avatar Mar 07 '22 23:03 Kerilk

@Kerilk I see what you're saying. It's not consistent the way it is, and it doesn't really serve a purpose.

That said, removing this enum entry would cause backwards compatibility issues if someone happened to find a use for it. Perhaps we should add a comment marking it deprecated.

bmyates avatar Mar 08 '22 13:03 bmyates

This is mostly an issue with switch constructs on stype warning you that you have to check for it. That's how I stumbled upon it in the first place.

Kerilk avatar Mar 08 '22 14:03 Kerilk