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

Enable building oneDNN with MKL when integrated with PyTorch and IPEX via ideep #1808

Closed
gtkramer opened this issue Feb 21, 2024 · 4 comments
Assignees
Labels

Comments

@gtkramer
Copy link

Summary

When building PyTorch or IPEX, oneDNN gets built indirectly because it's included as a git submodule. When oneDNN is built, there should be an option to allow for PyTorch and IPEX to select the BLAS vendor of oneDNN in a way that ensures uniform and consistent usage of MKL in the entire IPEX build process (and optionally PyTorch).

Problem statement

When building IPEX, MKL can be imported to the build process in three different ways:

  • From IPEX's FindoneMKL.cmake with find_package(oneMKL)
  • From the first-party MKL >= 2021.3.0 CMake via IPEX's FindOMP.cmake (copied from PyTorch, which is copied and modified from CMake) with find_package(MKL)
  • And if building IPEX directly with CMake and passing -DDNNL_BLAS_VENDOR=MKL when configuring with CMake, through oneDNN's blas.cmake with find_package(BLAS)

This is a nightmare when building IPEX and wanting to ensure MKL is used consistently in the end-to-end build. The best I can do today is link libmkl_core.so, libmkl_intel_lp64.so, libmkl_gnu_thread.so for IPEX, and libmkl_rt.so from oneDNN (which gets linked onto the final IPEX build artifacts anyway) because BLA_VENDOR gets set to Intel10_64_dyn.

Some of this needs to be fixed on the IPEX side, but without coordination on the oneDNN side, it's impossible to guarantee that MKL is pulled in once and in one way (same library type and library selection).

Preferred solution

The first part to understand here is whether it's worth building oneDNN with MKL's BLAS routines. Are these faster than what oneDNN will use out of the box when PyTorch and IPEX try to build with the dnnl target library in CMake? If so, then I think it's worth pursuing this.

As far as a solution goes, I'm thinking that if DNNL_BLAS_VENDOR=MKL, we have our best chance at integrating well in all build environments if we can do find_package(MKL). We can check if the MKL::MKL target exists, and if it does, use it, and if not, use the variables that the PyTorch MKL find module created. But this is still a bit messy. Ideally, PyTorch would also be updated to just use MKL::MKL. If we can get the same done for IPEX, then this would pretty much be a non-issue, as long as we can require MKL >= 2021.3.0. Otherwise, getting these three projects to coordinate on a consistent MKL usage seems like a case of needing the stars to align perfectly... :(

@gtkramer gtkramer added the enhancement A feature or an optimization request label Feb 21, 2024
@vpirogov
Copy link
Member

Thanks for bringing up this question, @gtkramer. There's no need to build oneDNN with MKL's BLAS routines since oneDNN v1.0. See discussion in #1794.

So the issue is relevant to IPEX only.

+@jgong5 for input on situation with IPEX.

@vpirogov vpirogov self-assigned this Feb 21, 2024
@vpirogov vpirogov added question and removed enhancement A feature or an optimization request labels Feb 21, 2024
@gtkramer
Copy link
Author

That's excellent background, @vpirogov, thank you! If I'm reading the thread right, using MKL with oneDNN has no significant performance gains then? And is even discouraged nowadays given it's not a supported BLAS library and may result in undefined behavior?

@vpirogov
Copy link
Member

That's correct. I looked at IPEX as well and it does not build oneDNN with MKL either.

@gtkramer
Copy link
Author

Awesome, thank you! That's all I needed there then

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants