Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide defines for build-time (header) version checks #111

Open
eero-t opened this issue Mar 13, 2023 · 7 comments
Open

Provide defines for build-time (header) version checks #111

eero-t opened this issue Mar 13, 2023 · 7 comments

Comments

@eero-t
Copy link

eero-t commented Mar 13, 2023

Currently building L0 programs give "mystery" errors if system L0 headers are not new enough.

It would be much user-friendlier if ze_api.h would include defines that can be used to check whether L0 headers are new enough, and which are (automatically) updated on every release.

E.g. with something like this:

#define L0_HEADER_MAJOR 1
#define L0_HEADER_MINOR 9
#define L0_HEADER_PATCHLEVEL 9
#define L0_HEADER_VERSION (L0_HEADER_MAJOR * 10000 + L0_HEADER_MINOR * 100  + L0_HEADER_PATCHLEVEL)

Code could then give much better error:

# if L0_HEADER_VERSION < 10909
#  error "At least version v1.9.9 of the L0 header(s) required"
#endif

(L0_HEADER_VERSION obviously assumes that project does not use more than 99 minor & patchlevel versions.)

PS. This is needed because doing package / pkg-config version check is not enough for all cases. Even if system would have new enough version installed from distro packages, code could e.g. be getting some older version from /usr/local/include. Or user could be directly compiling C/C++ files for self-contained L0 utilities.

@bmyates
Copy link
Contributor

bmyates commented Mar 13, 2023

Header compatibility is determined by L0 spec version, not L0 loader version. There is already a define for spec version that could be used as you describe.

See: ZE_API_VERSION_CURRENT in ze_api.h

@eero-t
Copy link
Author

eero-t commented Mar 13, 2023

@bmyates That's inadequate, as it handles only major and minor API version.

New structs and defines were added to the L0 headers e.g. between 1.5.8 and 1.5.17 spec versions, and e.g. compute-runtime zello_sysman build works only with headers for latter, not for the former.

Should I update this ticket title & description to target ZE_API_VERSION_CURRENT inaccuracy, or (close this and) file separate ticket for that?

@bmyates
Copy link
Contributor

bmyates commented Mar 13, 2023

The new structs added in 1.5.17 are extensions. Extensions have their own version defines that can be checked. Example: ZE_KERNEL_MAX_GROUP_SIZE_PROPERTIES_EXT_VERSION_CURRENT

@eero-t
Copy link
Author

eero-t commented Mar 13, 2023

Us of ext versions checks would be needed if spec would have multiple branches where extensions advance at different pace. But I do not think that's the case, currently at least spec version last number (patchlevel) is increased on extension updates, and newer always includes APIs from one with lower version number, right? [1]

In latter case, just checking for single "whole spec" version is easier for the user. One does not need to care / find out how different extensions are versioned, and which defines match to which APIs.

[1] Things deprecated & removed on major version changes, may need also separate check for too large major version.

@bmyates
Copy link
Contributor

bmyates commented Mar 13, 2023

A user can simply put extension specific code inside #ifdef *_EXT_VERSION_CURRENT . This pattern is used in level-zero-tests repo.

@eero-t
Copy link
Author

eero-t commented Mar 13, 2023

Sure.

Was there some particular reason why patchlevel was not included to ZE_MAKE_VERSION() and ZE_API_VERSION_CURRENT?

(It might still be fixable without breaking things, but once L0 major version updates start removing features i.e. comparisons are needed both for older and newer versions, it's harder.)

@eero-t
Copy link
Author

eero-t commented Nov 21, 2023

Was there some particular reason why patchlevel was not included to ZE_MAKE_VERSION() and ZE_API_VERSION_CURRENT?

(It might still be fixable without breaking things, but once L0 major version updates start removing features i.e. comparisons are needed both for older and newer versions, it's harder.)

@bmyates ? (feel free to close this after answering)

Jemale pushed a commit to Jemale/level-zero that referenced this issue Jun 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants