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

[oneDNN] New port #15068

Merged
merged 8 commits into from Dec 17, 2020
Merged

[oneDNN] New port #15068

merged 8 commits into from Dec 17, 2020

Conversation

jacobkahn
Copy link
Contributor

@jacobkahn jacobkahn commented Dec 11, 2020

Adds a new port for the oneDNN deep learning library.

cc @vpirogov, @dzarukin, @echeresh, @mgouicem, @akharito, @densamoilov

cc @syhw, @padentomasello, @tlikhomanenko, @vineelpratap, @xuqiantong, @avidov

  • Which triplets are supported/not supported? Have you updated the CI baseline?

x64 & !uwp. Tested myself on x64-linux.

Yes

@mgouicem
Copy link

great to see that oneDNN is making its way here! And thanks for the port @jacobkahn :-)
One quick question, is there any reason to link to Intel MKL? oneDNN without MKL should perform on par or better than without (and the build with MKL is mostly here for testing purpose).

Do you have any particular workloads where you see oneDNN with MKL faster than oneDNN without MKL?

@jacobkahn
Copy link
Contributor Author

@mgouicem — happy to help 👍 - we're about to use the port in flashlight for our new CPU/OpenCL backends

I have zero current benchmarks or supporting evidence that linking with MKL is faster besides that it used to be a bit faster with very old versions of MKL-DNN that I last benchmarked on. If it's just as good without, I'll happily gut the dependency (it makes life easier!). Thanks for the pointer there.

jacobkahn added a commit to jacobkahn/flashlight that referenced this pull request Dec 12, 2020
Summary:
Fix the Docker image build
- Use multiple `RUN` commands with `docker prune` to allow checkingpointing the base image builds to make it easier to debug stuff in the future
- Make `MKLROOT` a global env var
- Remove the MKL BLAS vendor for oneDNN (apparently faster without MKL) - see microsoft/vcpkg#15068 (comment)
- Enable building tests in the CPU CI build since it works now and `gtest_discover_tests` is removed so we don't have runtime `libmlk_rt` issues. We also shouldn't have those issues anymore in general because we are no longer linking MKL to the MKL-DNN/oneDNN build

Differential Revision: D25514612

fbshipit-source-id: d0f52bced60c5265f322f80363bd888e3d50ab5a
jacobkahn added a commit to jacobkahn/flashlight that referenced this pull request Dec 12, 2020
Summary:
Pull Request resolved: flashlight#330

Fix the Docker image build
- Use multiple `RUN` commands with `docker prune` to allow checkingpointing the base image builds to make it easier to debug stuff in the future
- Make `MKLROOT` a global env var
- Remove the MKL BLAS vendor for oneDNN (apparently faster without MKL) - see microsoft/vcpkg#15068 (comment)
- Enable building tests in the CPU CI build since it works now and `gtest_discover_tests` is removed so we don't have runtime `libmlk_rt` issues. We also shouldn't have those issues anymore in general because we are no longer linking MKL to the MKL-DNN/oneDNN build

Reviewed By: tlikhomanenko

Differential Revision: D25514612

fbshipit-source-id: a14ba9113b7d3f36ecf9a7478613ec1354518458
facebook-github-bot pushed a commit to flashlight/flashlight that referenced this pull request Dec 12, 2020
Summary:
Pull Request resolved: #330

Fix the Docker image build
- Use multiple `RUN` commands with `docker prune` to allow checkingpointing the base image builds to make it easier to debug stuff in the future
- Make `MKLROOT` a global env var
- Remove the MKL BLAS vendor for oneDNN (apparently faster without MKL) - see microsoft/vcpkg#15068 (comment)
- Enable building tests in the CPU CI build since it works now and `gtest_discover_tests` is removed so we don't have runtime `libmlk_rt` issues. We also shouldn't have those issues anymore in general because we are no longer linking MKL to the MKL-DNN/oneDNN build

Reviewed By: tlikhomanenko

Differential Revision: D25514612

fbshipit-source-id: 9f371a24e8c4de0c6779092539ffc544eaf21812
@JonLiu1993 JonLiu1993 assigned PhoebeHui and JackBoosY and unassigned PhoebeHui Dec 14, 2020
@JackBoosY JackBoosY added the category:new-port The issue is requesting a new library to be added; consider making a PR! label Dec 14, 2020
ports/onednn/portfile.cmake Outdated Show resolved Hide resolved
@JackBoosY JackBoosY added info:reviewed Pull Request changes follow basic guidelines and removed requires:author-response labels Dec 15, 2020
@BillyONeal BillyONeal merged commit 4c9cdfe into microsoft:master Dec 17, 2020
@jacobkahn jacobkahn deleted the onednn branch December 17, 2020 10:38
ryukw7 pushed a commit to ryukw7/vcpkg that referenced this pull request Dec 24, 2020
jacobkahn added a commit to flashlight/text that referenced this pull request Mar 25, 2022
Summary:
Pull Request resolved: flashlight/flashlight#330

Fix the Docker image build
- Use multiple `RUN` commands with `docker prune` to allow checkingpointing the base image builds to make it easier to debug stuff in the future
- Make `MKLROOT` a global env var
- Remove the MKL BLAS vendor for oneDNN (apparently faster without MKL) - see microsoft/vcpkg#15068 (comment)
- Enable building tests in the CPU CI build since it works now and `gtest_discover_tests` is removed so we don't have runtime `libmlk_rt` issues. We also shouldn't have those issues anymore in general because we are no longer linking MKL to the MKL-DNN/oneDNN build

Reviewed By: tlikhomanenko

Differential Revision: D25514612

fbshipit-source-id: 9f371a24e8c4de0c6779092539ffc544eaf21812
jacobkahn added a commit to flashlight/text that referenced this pull request Mar 25, 2022
Summary:
Pull Request resolved: flashlight/flashlight#330

Fix the Docker image build
- Use multiple `RUN` commands with `docker prune` to allow checkingpointing the base image builds to make it easier to debug stuff in the future
- Make `MKLROOT` a global env var
- Remove the MKL BLAS vendor for oneDNN (apparently faster without MKL) - see microsoft/vcpkg#15068 (comment)
- Enable building tests in the CPU CI build since it works now and `gtest_discover_tests` is removed so we don't have runtime `libmlk_rt` issues. We also shouldn't have those issues anymore in general because we are no longer linking MKL to the MKL-DNN/oneDNN build

Reviewed By: tlikhomanenko

Differential Revision: D25514612

fbshipit-source-id: 9f371a24e8c4de0c6779092539ffc544eaf21812
jacobkahn added a commit to flashlight/sequence that referenced this pull request Oct 13, 2022
Summary:
Pull Request resolved: flashlight/flashlight#330

Fix the Docker image build
- Use multiple `RUN` commands with `docker prune` to allow checkingpointing the base image builds to make it easier to debug stuff in the future
- Make `MKLROOT` a global env var
- Remove the MKL BLAS vendor for oneDNN (apparently faster without MKL) - see microsoft/vcpkg#15068 (comment)
- Enable building tests in the CPU CI build since it works now and `gtest_discover_tests` is removed so we don't have runtime `libmlk_rt` issues. We also shouldn't have those issues anymore in general because we are no longer linking MKL to the MKL-DNN/oneDNN build

Reviewed By: tlikhomanenko

Differential Revision: D25514612

fbshipit-source-id: 9f371a24e8c4de0c6779092539ffc544eaf21812
jacobkahn added a commit to flashlight/sequence that referenced this pull request Oct 13, 2022
Summary:
Pull Request resolved: flashlight/flashlight#330

Fix the Docker image build
- Use multiple `RUN` commands with `docker prune` to allow checkingpointing the base image builds to make it easier to debug stuff in the future
- Make `MKLROOT` a global env var
- Remove the MKL BLAS vendor for oneDNN (apparently faster without MKL) - see microsoft/vcpkg#15068 (comment)
- Enable building tests in the CPU CI build since it works now and `gtest_discover_tests` is removed so we don't have runtime `libmlk_rt` issues. We also shouldn't have those issues anymore in general because we are no longer linking MKL to the MKL-DNN/oneDNN build

Reviewed By: tlikhomanenko

Differential Revision: D25514612

fbshipit-source-id: 9f371a24e8c4de0c6779092539ffc544eaf21812
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:new-port The issue is requesting a new library to be added; consider making a PR! info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants