search

oneapi-src / level-zero

oneAPI Level Zero Specification Headers and Loader
https://spec.oneapi.com/versions/latest/elements/l0/source/index.html
MIT License
208 stars 90 forks source link

`ZES_STRUCTURE_TYPE_BASE_STATE` should not exist #84

Open Kerilk opened 2 years ago

Kerilk commented 2 years ago

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

jandres742 commented 2 years ago

@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

Kerilk commented 2 years ago

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.

bmyates commented 2 years ago

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

Kerilk commented 2 years ago

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.