-
Notifications
You must be signed in to change notification settings - Fork 1.1k
src: remove unnecessary compute device info header dependencies #3428
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
base: main
Are you sure you want to change the base?
Conversation
301089a
to
b661642
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you, please, extend the PR description with overview changes and what's the actual benefit of adding a direct nGEN dependency and a bunch of forward declaration to support it? Having an explanation why direct use of nGEN header is not possible given that a direct dependency is introduced would be helpful as well.
I added a description. The core issue is that the nGEN headers are only enabled within the src/gpu directory, so we get compilation errors if we add an nGEN dependency to device_info.hpp. We could alternatively just include nGEN headers everywhere in oneDNN (when applicable), but this PR keeps the current status quo as nGEN really should not be necessary as a dependency in |
95bb066
to
221f55c
Compare
@spalicki, Could you, please, take a look one this one? |
e1572a1
to
1892567
Compare
1892567
to
1cbd902
Compare
68132d7
to
5d27e35
Compare
make test |
5d27e35
to
9e7d1ac
Compare
The nGEN interface used for devices is ngen::Product. To enable better interoperation between oneDNN and nGEN, this PR modifies device_info_t to store the ngen::Product. In particular, as intel GPU third party dependencies are included via src/gpu/CMakeLists.txt, nGEN is not available outside this directory, so implicit dependencies on
gpu/intel/compute/device_info.hpp
outside of thesrc/gpu/
directories are removed. In particular, these implicit dependencies were injected viagpu/intel/compute_engine.hpp
,gpu/intel/compute_kernel.hpp
, andgpu/intel/ocl/ocl_utils.hpp
. This PR restructures these headers to remove the device_info.hpp dependency from these headers.