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
[Intel MKL] Make Vanilla TF and MKL-based TF use common oneDNN build #47679
[Intel MKL] Make Vanilla TF and MKL-based TF use common oneDNN build #47679
Conversation
@penpornk Here is the PR to unify the oneDNN build. Thank you! |
@mahmoud-abuzaina Thank you for the PR! |
@penpornk Thank you for the update. If you can share the oneDNN-related failure details, that would help us to look at it and fix it. |
@mahmoud-abuzaina Can you please resolve conflicts? Thanks! |
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.
Thank you for the PR!
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.
The test failures are similar to those of PR #47743 (which needed headers in include/**/*
for dnnl_single_threaded
).
third_party/mkl_dnn/mkldnn_v1.BUILD
Outdated
textual_hdrs = glob([ | ||
"include/**/*", | ||
"src/common/*.hpp", | ||
"src/cpu/*.hpp", | ||
"src/cpu/**/*.hpp", | ||
"src/cpu/x64/jit_utils/jitprofiling/*.h", | ||
"src/cpu/x64/xbyak/*.h", | ||
]), |
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 add this textual_hdrs
list to the mkl_dnn
target? Please also remove header files (*.h
, *.hpp
) from mkl_dnn
's src
list.
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.
Sure. Done.
Thank you for pointing out the solution for the build error. I have made the changes. |
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.
Thank you for the changes!
No description provided.