-
Notifications
You must be signed in to change notification settings - Fork 6.7k
[submodule] Upgrade to oneDNN v1.6 #18801
Conversation
|
Hey @TaoLv , Thanks for submitting the PR
CI supported jobs: [centos-cpu, sanity, unix-gpu, unix-cpu, website, windows-cpu, centos-gpu, clang, edge, miscellaneous, windows-gpu] Note: |
|
The new library doesn't compile with GCC 7.3.1 used in the CI of master branch. But it does compile in PR #18822 to v1.x branch where the CI uses GCC 8. @ChaiBapchya @leezu |
|
@leezu any guidance on unblocking the centos pipelines which fail coz of the GCC version
|
|
1.x branch tests gcc 8 as "some newer supported gcc version", whereas master branch tests gcc 7 as "oldest supported gcc version". As long as we support gcc7, we should ensure that we can compile with gcc7 |
|
@mxnet-bot run ci [centos-cpu, centos-gpu] |
|
Jenkins CI successfully triggered : [centos-cpu, centos-gpu] |
pengzhao-intel
left a comment
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.
LGTM
@bgawrych could you try gcc7? |
|
The error log is: It's a known issue in GCC 7: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83204 and was addressed in GCC 8. |
On Ubuntu 18.04.4 LTS with gcc 7.5 MXNet is succesfully build. |
|
@TaoLv @leezu GCC 7.4.0 and 7.5.0 (from sources) shows: But as I've mentioned earlier - GCC 7.5.0 provided by ubuntu doesn't trigger this error. I've checked also if standalone oneDNN is compiling with GCC 7.4.0 and it is. |
|
MXNet is building with C++ 17 standard: https://github.com/apache/incubator-mxnet/blob/master/CMakeLists.txt#L10. |
|
@vpirogov Could you please share more information about the compiler issue? |
|
@TaoLv, I do not have anything to add to information in GCC Bugzilla. |
Thank you for confirming this @bgawrych. Actually Ubuntu backported the fix to their GCC 7 version: One option is to require GCC8 on non-Ubuntu machines. WDYT? For the CI, we just need to replace all occurrences of But before that, @bgawrych @vpirogov could you please try adding the Thereby we use CXX11 standard to compile DNNL, which should avoid the bug in gcc7. |
|
Not sure why it has both |
|
One idea is to remove the global C++17 standard setting and instead set the C++17 only on the mxnet (and related, eg unittest) target. You may need to try a bit locally. |
|
From the release notes of https://github.com/oneapi-src/oneDNN/releases/tag/v1.6.1 it looks like v1.6.1 is mostly bugfixes of v.1.6. Would you mind bumping to v1.6.1? |
Yes, we need to change to 1.6.1 @TaoLv |
|
@anko-intel please create a new PR to update oneDNN. I am closing this one. |
Description
(Brief description on what this PR is about)
Checklist
Essentials
Please feel free to remove inapplicable items for your PR.
Changes
Comments