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

Explicitly tell users how to include ze_api.h and friends.

Open jpeyton52 opened this issue 3 years ago • 5 comments

The CMakeLists.txt file suggests that the main headers are installed to <prefix>/include/level_zero/ which would imply that users should #include <level_zero/ze_api.h> instead of #include <ze_api.h>. However, the documentation (https://spec.oneapi.com/level-zero/latest/core/INTRO.html#application-binary-interface) states that users should just include "ze_api.h".

Can we make a concrete statement in the documentation about how to include <level_zero/ze_api.h> instead of <ze_api.h>?

This also brings into question how the include parts of #50 should be structured (basically should we remove the level_zero component of the include?)

jpeyton52 avatar May 14 '21 14:05 jpeyton52

The install location of the headers isn't defined in the level zero spec. That's an implementation detail of the loader package. I think it's really up the the application developer if they want to add <prefix>/include/level_zero/ to their include path and include files with #include <ze_api.h>, or add <prefix>/include/ to include path and use #include <level_zero/ze_api.h>

bmyates avatar May 14 '21 19:05 bmyates

When I run through a basic cmake configure && make && make install into the standard system headers and then I try to #include <ze_api.h>, the compiler can't find the header. This has me believe this implementation wants users to #include <level_zero/ze_api.h> since the compiler can find that one without explicit -I flags. Why not tell users this much through example or documentation for this implementation of the level-zero API? This lack of clarity seems especially confusing when the SPEC only mentions the ze_api.h header. There is no mention (anywhere I can find) of the extra level_zero directory prefix (and there isn't one for the actual loader library). I think it's reasonable to inform users the intended way you want them to include the header. If they choose to do it differently that's up to them, but nudging them in the proper direction seems appropriate.

jpeyton52 avatar May 18 '21 14:05 jpeyton52

Is this still an issue with the *.pc files added in May, or can this be closed now?

eero-t avatar Nov 08 '21 15:11 eero-t

The pc files added in May aren't actually useful as said in the last comments of #50, we need better documentation.

bgoglin avatar Nov 08 '21 16:11 bgoglin

I think all that's needed is a modification to README.md under Building and Installing which notes where the header files, ze_api.h, etc. are installed by default: <prefix>/include/level_zero. I actually see a reference to this in doc/loader_api.md which notes:

Exposed Loader APIs will be defined in header files located in this repository at include/loader, and installed to <prefix>/include/level_zero/loader

jpeyton52 avatar Nov 30 '21 19:11 jpeyton52