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.

Sign up for GitHub

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

Jump to bottom

[INTEL MKL] Remove unnecessary MKL build macros and bug fixes #47375

Merged
merged 5 commits into from Feb 26, 2021
Merged

[INTEL MKL] Remove unnecessary MKL build macros and bug fixes #47375

merged 5 commits into from Feb 26, 2021

Conversation

gzmkl
Copy link
Contributor

@gzmkl gzmkl commented Feb 24, 2021 •
edited

Remove the following MKL build options (and usage of related MACRO's):

  1. ENABLE_INTEL_MKL_BFLOAT16 (treat as "TRUE" value)
  2. INTEL_MKL_DNN_ONLY (treat same as "INTEL_MKL")
  3. ENABLE_MKLDNN_V1 (DNN 0.x code cleanup; cleanup was overwritten by later PRs).

#3 fixes some bugs caused by the merge of final DNN 0.x code cleanup PR (#46370), most MKL unit test failure.

@gzmkl gzmkl requested a review from penpornk as a code owner February 24, 2021 17:14
@google-ml-butler google-ml-butler bot added the size:M CL Change Size: Medium label Feb 24, 2021
@google-cla google-cla bot added the cla: yes label Feb 24, 2021
@gzmkl gzmkl changed the title [INTEL MKL] Single binary preparation - remove unnecessary MKL build options [INTEL MKL] Remove unnecessary MKL build macros and bug fixes Feb 24, 2021
penpornk
penpornk approved these changes Feb 25, 2021
View reviewed changes
Copy link
Member

@penpornk penpornk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks so much cleaner. Thank you very much!

This PR will take a while to merge. I might try submitting a separate commit to fix mkl_dequantize_op.cc first so the CI can stop failing.

@google-ml-butler google-ml-butler bot added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels Feb 25, 2021
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Feb 25, 2021
copybara-service bot pushed a commit that referenced this pull request Feb 25, 2021
@penpornk @tensorflower-gardener
Expediting the oneDNN compilation failure fix from
cbaadc3 
#47375

PiperOrigin-RevId: 359427492
Change-Id: Id34285bee29c75985a6a0162232e4ebdf3a38f61
@gbaned gbaned self-assigned this Feb 25, 2021
@gbaned gbaned added the comp:mkl MKL related issues label Feb 25, 2021
@gbaned gbaned added this to Assigned Reviewer in PR Queue via automation Feb 25, 2021
@penpornk
Copy link
Member

The fix was in cbaadc3, but the CI still have some other failures (about benchmarks).
Could you please resync to master to resolve the merge conflict? Thank you!

@gbaned gbaned removed the ready to pull PR ready for merge process label Feb 25, 2021
@gzmkl
Copy link
Contributor Author

gzmkl commented Feb 25, 2021

This looks so much cleaner. Thank you very much!

This PR will take a while to merge. I might try submitting a separate commit to fix mkl_dequantize_op.cc first so the CI can stop failing.

Thank you Penporn! I have just addressed the merge conflicts

@gzmkl
Copy link
Contributor Author

gzmkl commented Feb 25, 2021

For CI failures (about benchmarks, there is another PR (submitted by my teammate).

penpornk
penpornk approved these changes Feb 25, 2021
View reviewed changes
Copy link
Member

@penpornk penpornk left a 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 update! I'll look for that other fixing PR. :)

PR Queue automation moved this from Assigned Reviewer to Approved by Reviewer Feb 25, 2021
@google-ml-butler google-ml-butler bot added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels Feb 25, 2021
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Feb 25, 2021
@gbaned gbaned added ready to pull PR ready for merge process and removed ready to pull PR ready for merge process labels Feb 26, 2021
@penpornk penpornk added the kokoro:force-run Tests on submitted change label Feb 26, 2021
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Feb 26, 2021
@copybara-service copybara-service bot merged commit e966723 into tensorflow:master Feb 26, 2021
PR Queue automation moved this from Approved by Reviewer to Merged Feb 26, 2021
@gzmkl gzmkl mentioned this pull request Mar 2, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@penpornk penpornk penpornk approved these changes

Assignees

@gbaned gbaned

Labels
cla: yes comp:mkl MKL related issues ready to pull PR ready for merge process size:M CL Change Size: Medium
Projects
PR Queue
  
Merged
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@gzmkl @penpornk @kokoro-team @gbaned