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

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

Open jpeyton52 opened 3 years ago

jpeyton52 commented 3 years ago

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 ?

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?)

bmyates commented 3 years ago

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>

jpeyton52 commented 3 years ago

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.

eero-t commented 2 years ago

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

bgoglin commented 2 years ago

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

jpeyton52 commented 2 years ago

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