[aotinductor] support at::convolution for AOTInductor #114961

chenyang78 · 2023-12-01T17:45:30Z

Stack from ghstack (oldest at bottom):

-> [aotinductor] support at::convolution for AOTInductor #114961

This PR adds support to at::convolution for AOTInductor

cc @voznesenskym @penguinwu @EikanWang @jgong5 @Guobing-Chen @XiaobingSuper @zhuhaozhe @blzheng @wenzhe-nrv @jiayisunx @peterbell10 @ipiszy @yf225 @kadeng @muchulee8 @aakhundov @ColinPeppler

This PR adds support to at::convolution for AOTInductor [ghstack-poisoned]

pytorch-bot · 2023-12-01T17:45:33Z

🔗 Helpful Links

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

📄 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.

✅ You can merge normally! (2 Unrelated Failures)

As of commit 938b8c6 with merge base af5a3bd ():

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

inductor-periodic / cuda12.1-py3.10-gcc9-sm86-periodic-dynamo-benchmarks / test (dynamic_aot_eager_torchbench, 1, 2, linux.g5.4xlarge.nvidia.gpu) (gh)

BROKEN TRUNK - The following job failed but was present on the merge base:

👉 Rebase onto the `viable/strict` branch to avoid these failures

trunk / macos-12-py3-arm64 / test (default, 3, 3, macos-m1-12) (gh)

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

eellison · 2023-12-01T17:52:07Z

torch/csrc/inductor/aoti_torch/shim_common.cpp

  });
 }

+AOTI_TORCH_EXPORT AOTITorchError aoti_torch_convolution(


Would it make sense to codegen these ?

Yeah, I think we rely on both codegen and at::convolution extern call, depending on the profiling results:

https://github.com/pytorch/pytorch/blob/main/torch/_inductor/kernel/conv.py#L411-L413

To support the extern call case, we need to include this into the C shim.

I think @eellison meant auto-generating shim functions. There were not many interfaces needed initially. Hopefully with more complete decomposition coverage, MM and Conv are the only cases we need to have in the C shim layer.

I see. Yeah, we would definitely go for that direction if we see lots of shim functions.

This PR adds support to at::convolution for AOTInductor cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx peterbell10 ipiszy yf225 kadeng muchulee8 aakhundov ColinPeppler [ghstack-poisoned]

This PR adds support to at::convolution for AOTInductor ghstack-source-id: 4afa503 Pull Request resolved: #114961

desertfire · 2023-12-01T20:44:51Z

torch/csrc/inductor/aoti_torch/shim_common.cpp

  });
 }

+AOTI_TORCH_EXPORT AOTITorchError aoti_torch_convolution(


I think @eellison meant auto-generating shim functions. There were not many interfaces needed initially. Hopefully with more complete decomposition coverage, MM and Conv are the only cases we need to have in the C shim layer.

desertfire · 2023-12-01T20:56:00Z

torch/_inductor/codegen/wrapper.py

        self.writeline(f"AOTI_TORCH_ERROR_CODE_CHECK({shim_fn}({', '.join(args)}));")

-    def generate_c_shim_extern_kernel_alloc_call(self, extern_kernel, args):
+    def generate_c_shim_extern_kernel_alloc(self, extern_kernel, args):


We could use some refactoring for ir.ExternKernelAlloc and ir.FallbackKernel at some point.

We could use some refactoring for ir.ExternKernelAlloc and ir.FallbackKernel at some point.

Yeah, agreed.

desertfire · 2023-12-01T20:56:23Z

torch/csrc/inductor/aoti_torch/c/shim.h

+    int64_t padding_size,
+    int64_t* dilation_ptr,
+    int64_t dilation_size,
+    bool transposed,


use int instead

This PR adds support to at::convolution for AOTInductor cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx peterbell10 ipiszy yf225 kadeng muchulee8 aakhundov ColinPeppler [ghstack-poisoned]

This PR adds support to at::convolution for AOTInductor ghstack-source-id: b8c519f Pull Request resolved: #114961

chenyang78 · 2023-12-03T01:29:29Z

@pytorchbot merge

pytorchmergebot · 2023-12-03T01:31:29Z

Merge failed

Reason: This PR needs a release notes: label
If your changes are user facing and intended to be a part of release notes, please use a label starting with release notes:.

If not, please add the topic: not user facing label.

To add a label, you can comment to pytorchbot, for example
@pytorchbot label "topic: not user facing"

For more information, see
https://github.com/pytorch/pytorch/wiki/PyTorch-AutoLabel-Bot#why-categorize-for-release-notes-and-how-does-it-work.

Details for Dev Infra team

Raised by workflow job

chenyang78 · 2023-12-03T07:49:51Z

@pytorchbot merge

pytorchmergebot · 2023-12-03T07:51:49Z

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

swolchok · 2023-12-05T23:31:11Z

torch/csrc/inductor/aoti_torch/c/shim.h

+    int64_t* stride_ptr,
+    int64_t stride_size,
+    int64_t* padding_ptr,
+    int64_t padding_size,
+    int64_t* dilation_ptr,
+    int64_t dilation_size,
+    int transposed,
+    int64_t* output_padding_ptr,


I think all these pointers needed to be decorated with const. piggybacking fix onto #113577

This PR adds support to at::convolution for AOTInductor Pull Request resolved: pytorch#114961 Approved by: https://github.com/desertfire

[aotinductor] support at::convolution for AOTInductor

a2d114a

This PR adds support to at::convolution for AOTInductor [ghstack-poisoned]

github-actions bot added module: inductor ciflow/inductor labels Dec 1, 2023

eellison reviewed Dec 1, 2023

View reviewed changes

chenyang78 requested review from SherlockNoMad, desertfire and jansel December 1, 2023 17:58

chenyang78 added a commit that referenced this pull request Dec 1, 2023

[aotinductor] support at::convolution for AOTInductor

6fe7705

This PR adds support to at::convolution for AOTInductor ghstack-source-id: 4afa503 Pull Request resolved: #114961

desertfire approved these changes Dec 1, 2023

View reviewed changes

chenyang78 added a commit that referenced this pull request Dec 2, 2023

[aotinductor] support at::convolution for AOTInductor

a0ba530

This PR adds support to at::convolution for AOTInductor ghstack-source-id: b8c519f Pull Request resolved: #114961

pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Dec 3, 2023

pytorchmergebot added the merging label Dec 3, 2023

pytorchmergebot removed the merging label Dec 3, 2023

chenyang78 added the topic: not user facing topic category label Dec 3, 2023

pytorchmergebot added the merging label Dec 3, 2023

pytorchmergebot added the Merged label Dec 3, 2023

pytorchmergebot closed this in 4d8b996 Dec 3, 2023

pytorchmergebot removed the merging label Dec 3, 2023

swolchok reviewed Dec 5, 2023

View reviewed changes

facebook-github-bot deleted the gh/chenyang78/7/head branch December 6, 2023 15:28

[aotinductor] support at::convolution for AOTInductor #114961

[aotinductor] support at::convolution for AOTInductor #114961

Uh oh!

Conversation

chenyang78 commented Dec 1, 2023 • edited by pytorch-bot bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pytorch-bot bot commented Dec 1, 2023 • 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/114961

✅ You can merge normally! (2 Unrelated Failures)

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chenyang78 commented Dec 3, 2023

Uh oh!

pytorchmergebot commented Dec 3, 2023

Merge failed

Uh oh!

chenyang78 commented Dec 3, 2023

Uh oh!

pytorchmergebot commented Dec 3, 2023

Merge started

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

chenyang78 commented Dec 1, 2023 •

edited by pytorch-bot bot

Loading

pytorch-bot bot commented Dec 1, 2023 •

edited

Loading