Skip to content

[Inductor] Set prop_kind to forward_inference when grad is not needed for mkldnn_linear_pointwise and mkldnn_convolution_pointwise #147072

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

Closed
wants to merge 6 commits into from

Conversation

jiayisunx
Copy link
Collaborator

@jiayisunx jiayisunx commented Feb 13, 2025

Stack from ghstack (oldest at bottom):

Summary:
The prop_kind of mkldnn._linear_pointwise, mkldnn._linear_pointwise.binary, mkldnn._convolution_pointwise.binary and mkldnn._convolution_pointwise_.binary are always dnnl_forward, i.e., dnnl_forward_training , regardless of whether grad is needed. Setting prop_kind to dnnl_forward_inference for these ops when grad is not needed could have better performance.

cc @jgong5 @mingfeima @XiaobingSuper @sanchitintel @ashokei @jingxu10

[ghstack-poisoned]
@pytorch-bot pytorch-bot bot added the module: cpu CPU specific problem (e.g., perf, algorithm) label Feb 13, 2025
Copy link

pytorch-bot bot commented Feb 13, 2025

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/147072

Note: Links to docs will display an error until the docs builds have been completed.

✅ You can merge normally! (2 Unrelated Failures)

As of commit 472288c with merge base 790f93d (image):

FLAKY - The following job failed but was likely due to flakiness present on trunk:

UNSTABLE - The following job is marked as unstable, possibly due to flakiness on trunk:

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@jiayisunx jiayisunx marked this pull request as draft February 13, 2025 07:57
@jiayisunx jiayisunx marked this pull request as ready for review February 14, 2025 07:59
@jiayisunx jiayisunx added the ciflow/trunk Trigger trunk jobs on your pull request label Feb 14, 2025
@jiayisunx jiayisunx requested a review from CaoE February 14, 2025 08:11
[ghstack-poisoned]
[ghstack-poisoned]
@jiayisunx jiayisunx requested a review from mingfeima February 17, 2025 07:19
@jiayisunx jiayisunx requested a review from jansel February 17, 2025 09:27
Comment on lines 273 to 280
auto aprop_kind = ideep::prop_kind::forward;
bool maybe_backward = GradMode::is_enabled() &&
(input_t.requires_grad() || weight_t.requires_grad() ||
(bias_opt.has_value() && bias_opt->defined() &&
bias_opt->requires_grad()));
if (!maybe_backward) {
aprop_kind = ideep::prop_kind::forward_inference;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

define a helper method for this repeated code block

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I modified the code, I directly set aprop_kind in mkldnn_linear_pointwise_binary function to forward_inference since binary fusion is only for inference. Could you please review it again? Thanks!

[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
@jiayisunx jiayisunx changed the title [Inductor] Set prop_kind to forward_inference when grad is not needed for mkldnn_linear_pointwise [Inductor] Set prop_kind to forward_inference when grad is not needed for mkldnn_linear_pointwise and mkldnn_convolution_pointwise Mar 19, 2025
@jiayisunx
Copy link
Collaborator Author

@pytorchbot merge

@pytorchmergebot
Copy link
Collaborator

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging
Check the merge workflow status
here

amathewc pushed a commit to amathewc/pytorch that referenced this pull request Apr 17, 2025
… for mkldnn_linear_pointwise and mkldnn_convolution_pointwise (pytorch#147072)

Summary:
The `prop_kind` of `mkldnn._linear_pointwise`, `mkldnn._linear_pointwise.binary`, `mkldnn._convolution_pointwise.binary` and `mkldnn._convolution_pointwise_.binary` are always `dnnl_forward`, i.e., `dnnl_forward_training` , regardless of whether `grad` is needed. Setting `prop_kind` to `dnnl_forward_inference` for these ops when `grad` is not needed could have better performance.

Pull Request resolved: pytorch#147072
Approved by: https://github.com/leslie-fang-intel, https://github.com/CaoE, https://github.com/jansel
@github-actions github-actions bot deleted the gh/jiayisunx/56/head branch April 22, 2025 02:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ciflow/inductor ciflow/trunk Trigger trunk jobs on your pull request Merged module: cpu CPU specific problem (e.g., perf, algorithm) open source release notes: inductor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants