[Intel GPU] Extract common utils for conv&qconv #139580

ZhiweiYan-96 · 2024-11-03T10:08:25Z

Motivation

This PR is a precursor to #133080. The PR extracts common logics in convolution and quantized convolution into Utils.cpp. With such modification, these two operators could share codes like input format querying, op layout querying.

Stack from ghstack (oldest at bottom):

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

[ghstack-poisoned]

pytorch-bot · 2024-11-03T10:08:29Z

🔗 Helpful Links

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

📄 Preview Python docs built from this PR
📄 Preview C++ docs built from this PR
❓ Need help or want to give feedback on the CI? Visit the bot commands wiki or our office hours

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

✅ No Failures

As of commit dd2ec92 with merge base ffb9790 ():
💚 Looks good so far! There are no failures yet. 💚

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

[ghstack-poisoned]

guangyey · 2024-11-04T06:34:56Z

Is it OK to add aten/src/ATen/native/mkldnn/xpu/*.cpp and aten/src/ATen/native/mkldnn/xpu/*.h to

pytorch/.lintrunner.toml

Line 61 in 3179eb1

'aten/src/ATen/native/mps/**/*.metal',

I think we need this autoformat.

guangyey · 2024-11-04T06:37:12Z

aten/src/ATen/native/mkldnn/xpu/detail/Utils.h

 #include <ATen/ATen.h>
 #include <ATen/Tensor.h>
 #include <ATen/core/Tensor.h>
+#include <iostream>


where do we need this code?

has removed, thanks for suggestions

EikanWang

Overall, LGTM

EikanWang · 2024-11-04T06:54:25Z

aten/src/ATen/native/mkldnn/xpu/detail/Utils.cpp

  auto dims = adims.data();
  auto data_type = static_cast<dnnl_data_type_t>(
-      get_onednn_dtype(src, /*allow_undef*/ true));
+      get_onednn_dtype_include_double(src, /*allow_undef*/ false));


What is the motivation behind true->false?

This should be a bug fixing from IPEX by @Stonepia , changing this into false would trigger runtime error if met out-of-expectation error. Otherwise, the error might be hidden.

This is because that if we allow_undef = true, this data_type might be undef, then the consecutive call like below will get wrong result, and cause silent fail.

// when data_type is undef, the md will be something like `0x1` dnnl_memory_desc_create_with_strides(&md, ndims, dims, data_type, strides);

So, pls. add comments here to describe the tricky change.

EikanWang · 2024-11-04T06:58:08Z

@ZhiweiYan-96 , pls. add detailed descriptions to help reviewers understand your changes.

ZhiweiYan-96 · 2024-11-04T07:39:35Z

Is it OK to add aten/src/ATen/native/mkldnn/xpu/*.cpp and aten/src/ATen/native/mkldnn/xpu/*.h to

pytorch/.lintrunner.toml

Line 61 in 3179eb1

'aten/src/ATen/native/mps/**/*.metal',

I think we need this autoformat.

hi, @guangyey The number of PRs (8 prs) in current stack exceeds the recommendation of ghstack, I would create another PR to enable formatting.

[ghstack-poisoned]

ZhiweiYan-96 · 2024-11-12T01:45:05Z

@pytorchbot merge

pytorchmergebot · 2024-11-12T01:46:51Z

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

pytorchmergebot · 2024-11-12T01:52:28Z

Merge failed

Reason: Command git -C /home/runner/work/pytorch/pytorch cherry-pick -x c964050d39a91d5c18b9f5b4e57cf7d2626e7e3a returned non-zero exit code 1

Auto-merging aten/src/ATen/native/mkldnn/xpu/Conv.cpp
CONFLICT (content): Merge conflict in aten/src/ATen/native/mkldnn/xpu/Conv.cpp
Auto-merging aten/src/ATen/native/mkldnn/xpu/detail/Conv.cpp
CONFLICT (content): Merge conflict in aten/src/ATen/native/mkldnn/xpu/detail/Conv.cpp
Auto-merging aten/src/ATen/native/mkldnn/xpu/detail/Deconv.cpp
CONFLICT (content): Merge conflict in aten/src/ATen/native/mkldnn/xpu/detail/Deconv.cpp
Auto-merging aten/src/ATen/native/mkldnn/xpu/detail/Utils.cpp
CONFLICT (content): Merge conflict in aten/src/ATen/native/mkldnn/xpu/detail/Utils.cpp
Auto-merging aten/src/ATen/native/mkldnn/xpu/detail/Utils.h
CONFLICT (content): Merge conflict in aten/src/ATen/native/mkldnn/xpu/detail/Utils.h
error: could not apply c964050d39... [Intel GPU] Extract common utils for conv&qconv
hint: After resolving the conflicts, mark them with
hint: "git add/rm <pathspec>", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".
hint: Disable this message with "git config advice.mergeConflict false"

Details for Dev Infra team

Raised by workflow job

ZhiweiYan-96 · 2024-11-12T01:53:12Z

@pytorchbot merge

pytorchmergebot · 2024-11-12T01:54:57Z

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

# Motivation This PR is a precursor to pytorch#133080. The PR extracts common logics in convolution and quantized convolution into `Utils.cpp`. With such modification, these two operators could share codes like input format querying, op layout querying. Pull Request resolved: pytorch#139580 Approved by: https://github.com/EikanWang, https://github.com/guangyey, https://github.com/malfet ghstack dependencies: pytorch#139721

Update

a3c7c9b

[ghstack-poisoned]

ZhiweiYan-96 mentioned this pull request Nov 3, 2024

[Intel GPU] qconv at XPU backend #133080

Closed

pytorch-bot bot added the module: cpu CPU specific problem (e.g., perf, algorithm) label Nov 3, 2024

ZhiweiYan-96 marked this pull request as draft November 3, 2024 10:10

pytorchbot added the open source label Nov 3, 2024

ZhiweiYan-96 added 2 commits November 3, 2024 10:21

Update

62b7f62

[ghstack-poisoned]

Update

50908b5

[ghstack-poisoned]

ZhiweiYan-96 added topic: not user facing topic category ciflow/xpu Run XPU CI tasks ciflow/trunk Trigger trunk jobs on your pull request labels Nov 4, 2024

ZhiweiYan-96 requested review from EikanWang and guangyey November 4, 2024 03:11

Update

1fbfaa1

[ghstack-poisoned]

guangyey reviewed Nov 4, 2024

View reviewed changes

EikanWang approved these changes Nov 4, 2024

View reviewed changes

Update

6fca075

[ghstack-poisoned]

ZhiweiYan-96 requested a review from guangyey November 4, 2024 07:55

guangyey approved these changes Nov 4, 2024

View reviewed changes

Update

f255b8c

[ghstack-poisoned]

ZhiweiYan-96 added 4 commits November 5, 2024 13:18

Update

e8719cc

[ghstack-poisoned]

Update

3de2eee

[ghstack-poisoned]

Update

7edbbfb

[ghstack-poisoned]

Update

4841757

[ghstack-poisoned]

malfet approved these changes Nov 8, 2024

View reviewed changes

ZhiweiYan-96 added 7 commits November 10, 2024 09:02

Update

a436e70

[ghstack-poisoned]

Update

d9ec30a

[ghstack-poisoned]

Update

1e803c5

[ghstack-poisoned]

Update

c232d4e

[ghstack-poisoned]

Update

ba7b6cc

[ghstack-poisoned]

Update

a85d8b6

[ghstack-poisoned]

Update

dd2ec92

[ghstack-poisoned]

pytorchmergebot added the merging label Nov 12, 2024

pytorchmergebot removed the merging label Nov 12, 2024

pytorchmergebot added the merging label Nov 12, 2024

pytorchmergebot added the Merged label Nov 12, 2024

pytorchmergebot closed this in 19eff28 Nov 12, 2024

pytorchmergebot removed the merging label Nov 12, 2024

github-actions bot deleted the gh/ZhiweiYan-96/35/head branch December 12, 2024 02:12

[Intel GPU] Extract common utils for conv&qconv #139580

[Intel GPU] Extract common utils for conv&qconv #139580

Uh oh!

Conversation

ZhiweiYan-96 commented Nov 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Uh oh!

pytorch-bot bot commented Nov 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔗 Helpful Links

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

✅ No Failures

Uh oh!

guangyey commented Nov 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

guangyey Nov 4, 2024

Choose a reason for hiding this comment

Uh oh!

ZhiweiYan-96 Nov 4, 2024

Choose a reason for hiding this comment

Uh oh!

EikanWang left a comment

Choose a reason for hiding this comment

Uh oh!

EikanWang Nov 4, 2024

Choose a reason for hiding this comment

Uh oh!

ZhiweiYan-96 Nov 4, 2024

Choose a reason for hiding this comment

Uh oh!

Stonepia Nov 4, 2024

Choose a reason for hiding this comment

Uh oh!

EikanWang Nov 5, 2024

Choose a reason for hiding this comment

Uh oh!

EikanWang commented Nov 4, 2024

Uh oh!

ZhiweiYan-96 commented Nov 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ZhiweiYan-96 commented Nov 12, 2024

Uh oh!

pytorchmergebot commented Nov 12, 2024

Merge started

Uh oh!

pytorchmergebot commented Nov 12, 2024

Merge failed

Uh oh!

ZhiweiYan-96 commented Nov 12, 2024

Uh oh!

pytorchmergebot commented Nov 12, 2024

Merge started

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

ZhiweiYan-96 commented Nov 3, 2024 •

edited

Loading

pytorch-bot bot commented Nov 3, 2024 •

edited

Loading

guangyey commented Nov 4, 2024 •

edited

Loading

ZhiweiYan-96 commented Nov 4, 2024 •

edited

Loading