Milestone 1: Added Fusion G3 NN library with kernels related to add, mul, quantize… #6738

ckmadhira · 2024-11-08T17:21:12Z

…, dequantize, cat, layer norm, softmax to backends/cadence folder. Added operators to backends/cadence folder

Summary

Added kernels and operators related to

Add
Mul,
Quantize
Dequatize
Cat
Layernorm
Softmax
to Fusion G3. The Kernels part of Fusion G3 NN library which is a submodule.

…, dequantize, cat, layer norm, softmax to backends/cadence folder. Added operators to backends/cadence folder

pytorch-bot · 2024-11-08T17:21:15Z

🔗 Helpful Links

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

📄 Preview Python docs built from this PR

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

❌ 1 New Failure

As of commit 92b58ef with merge base 43555d2 ():

NEW FAILURE - The following job has failed:

Lint / lintrunner / linux-job (gh)
>>> Lint for backends/cadence/fusion_g3/operators/op_softmax.cpp:

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

facebook-github-bot · 2024-11-08T17:21:19Z

Hi @ckmadhira!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at [email protected]. Thanks!

ckmadhira · 2024-11-08T17:23:31Z

Fusion G3 NN library is added with kernels related to add, mul, quantize, dequantize, cat, layernorm and softmax. Operators which use these kernels are also added to backends/cadence folder

zonglinpeng

Thanks for the change! Had one comment on the op namespace

zonglinpeng · 2024-11-08T18:47:34Z

backends/cadence/fusion_g3/operators/op_add.cpp

+
+namespace impl {
+namespace FusionG3 { 
+namespace native {


Can the name spaces of all the G3 ops build as

namespace cadence { namespace impl { namespace G3 { namespace native {

To align with other ops? HiFi example: https://github.com/pytorch/executorch/blob/main/backends/cadence/hifi/operators/op_add.cpp#L25

Updated as per review comment

zonglinpeng · 2024-11-08T18:48:05Z

backends/cadence/aot/functions_fusion_g3.yaml

+- op: _softmax.out
+  kernels:
+    - arg_meta: null
+      kernel_name: impl::FusionG3::softmax_out


Same comment on the namespace

Per @hsharma35 's comment above, please use cadence::impl::G3::native::<OP_NAME> namespace

Updated the operator name as cadence::impl::G3::<OP_NAME>. native is not explicitly mentioned. As per kernel-library-custom-aten-kernel.md, native is automatically appended to the operators.

hsharma35 · 2024-11-08T18:41:10Z

backends/cadence/aot/functions_fusion_g3.yaml

@@ -0,0 +1,119 @@
+# Copyright (c) Meta Platforms, Inc. and affiliates.


Please change the kernel_name here to match cadence::impl::G3::native::OP_NAME. For example: cadence::impl::G3::native::add_out

Updated the operator name as cadence::impl::G3::add_out. native is not explicitly mentioned.

With the change to operator namespace, I think this would now fail to compile. Can you please check on your end?

We are able to compile and verify the operator with toy model. Are you having any issue with this name space?

hsharma35 · 2024-11-08T19:20:34Z

Nit: Can we rename the folder FuG3 to FusionG3 instead?

facebook-github-bot · 2024-11-11T22:59:24Z

@zonglinpeng has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

Signed-off-by: [email protected] <[email protected]>

zonglinpeng · 2024-11-14T05:56:01Z

please make nnlib a standalone repository @ckmadhira

zonglinpeng

comment as above

ckmadhira · 2024-11-14T05:59:56Z

please make nnlib a standalone repository @ckmadhira

The decision is not to make the Fusion G3 NN library opensource. We are looking for a space where we can put the repo and provide exclusive access to Meta

zonglinpeng · 2024-11-19T21:37:31Z

@ckmadhira thanks for driving nnlib to a public repo! Can you remake this PR with just the kernel updates (i.e. everything except thirdparty nnlib) so that I can import as a clean change? Thank you
cc @mcremon-meta

Signed-off-by: [email protected] <[email protected]>

ckmadhira · 2024-11-20T05:31:59Z

@ckmadhira thanks for driving nnlib to a public repo! Can you remake this PR with just the kernel updates (i.e. everything except thirdparty nnlib) so that I can import as a clean change? Thank you cc @mcremon-meta

Updated Fusion G3 NN library as a submodule from opensource.

hsharma35

LGTM, just needs one change:
Please make the operators in op_*.cpp consistent with the operators in functions_fusion_g3.yaml.

Thanks!

hsharma35 · 2024-11-20T16:43:04Z

backends/cadence/aot/functions_fusion_g3.yaml

+- op: add.out
+  kernels:
+    - arg_meta: null
+      kernel_name: cadence::impl::G3::add_out


@ckmadhira the name used here (kernel_name: cadence::impl::G3::add_out) does not match the actual kernel in op_add.cpp (cadence::impl::G3::native::add_out). Is this compiling on your end?

This got compiled for us and we are able to test it. We should not add "native" in the functions_fusion_g3.yaml file. The word "native" implicitly gets appended. If we try to add "native", we get build errors saying, native is added twice.

hsharma35 · 2024-11-20T16:47:22Z

backends/cadence/aot/functions_fusion_g3.yaml

@@ -0,0 +1,119 @@
+# Copyright (c) Meta Platforms, Inc. and affiliates.


With the change to operator namespace, I think this would now fail to compile. Can you please check on your end?

zonglinpeng · 2024-11-20T17:20:22Z

Please resolve the merge conflict in .gitmodules, and update the PR summary

zonglinpeng · 2024-11-20T17:20:57Z

backends/cadence/fusion_g3/third-party/nnlib/nnlib-FusionG3

Nice this is awesome!

facebook-github-bot · 2024-11-20T18:30:39Z

@zonglinpeng has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

ckmadhira · 2024-11-21T10:39:47Z

This is done

mcremon-meta · 2024-11-22T00:40:44Z

@ckmadhira there are a lot of linter errors, see https://github.com/pytorch/executorch/actions/runs/11950614633/job/33350989488?pr=6738. @zonglinpeng can you share instructions on how to run the linter?

zonglinpeng · 2024-11-22T21:02:46Z

@ckmadhira there are a lot of linter errors, see https://github.com/pytorch/executorch/actions/runs/11950614633/job/33350989488?pr=6738. @zonglinpeng can you share instructions on how to run the linter?

https://github.com/pytorch/executorch/blob/main/CONTRIBUTING.md#lintrunner @ckmadhira

facebook-github-bot · 2024-11-23T00:40:09Z

@zonglinpeng has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

Signed-off-by: [email protected] <[email protected]>

zonglinpeng

there are still some lint errors, please try lintrunner -a
https://github.com/pytorch/executorch/actions/runs/12009466002/job/33489114504?pr=6738

Ok to merge, we can likely fix issues on our end

facebook-github-bot · 2024-11-25T19:44:10Z

@zonglinpeng has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

zonglinpeng · 2024-11-25T23:08:28Z

backends/cadence/aot/functions_fusion_g3.yaml

+- op: native_layer_norm.out
+  kernels:
+    - arg_meta: null
+      kernel_name: cadence::impl::G3::native_layer_norm_out     


missing quant dequant per tensor func definition. fixed in #7061

zonglinpeng · 2024-11-25T23:09:17Z

backends/cadence/aot/functions_fusion_g3.yaml

+- op: _softmax.out
+  kernels:
+    - arg_meta: null
+      kernel_name: cadence::impl::G3::softmax_out


need to be _softmax to be consistent, fixed in #7061

zonglinpeng · 2024-11-25T23:09:47Z

backends/cadence/fusion_g3/operators/op_softmax.cpp

+namespace G3 { 
+namespace native {
+
+Tensor& softmax_out(


same comment, _softmax, fixed in #7061

zonglinpeng · 2024-11-25T23:11:39Z

backends/cadence/fusion_g3/operators/op_quantize.cpp

+ */
+namespace cadence {
+namespace impl {
+namespace FusionG3 {


namespace needs to be G3. fixed in #7061

zonglinpeng · 2024-11-25T23:12:10Z

backends/cadence/fusion_g3/operators/op_dequantize.cpp

+
+
+/* Local function which calls the kernels based on the input datatype */
+void Dequantize_impl(Tensor& out,


please use all lower case fixed in #7061

Added Fusion G3 NN library with kernels related to add, mul, quantize…

0bf646e

…, dequantize, cat, layer norm, softmax to backends/cadence folder. Added operators to backends/cadence folder

zonglinpeng requested changes Nov 8, 2024

View reviewed changes

hsharma35 reviewed Nov 8, 2024

View reviewed changes

facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Nov 11, 2024

Updated name space of the operators by appending cadence

7bd011f

Signed-off-by: [email protected] <[email protected]>

ckmadhira requested review from hsharma35 and zonglinpeng November 13, 2024 14:04

zonglinpeng requested changes Nov 14, 2024

View reviewed changes

ckmadhira force-pushed the main branch from 5df774f to 042692d Compare November 20, 2024 05:21

Added nnlib-FusionG3 submodule from FOSS-xtensa git space

f75206b

Signed-off-by: [email protected] <[email protected]>

ckmadhira force-pushed the main branch from 042692d to f75206b Compare November 20, 2024 05:26

ckmadhira requested a review from zonglinpeng November 20, 2024 05:33

hsharma35 previously requested changes Nov 20, 2024

View reviewed changes

zonglinpeng reviewed Nov 20, 2024

View reviewed changes

backends/cadence/fusion_g3/third-party/nnlib/nnlib-FusionG3

Copy link

Contributor

zonglinpeng Nov 20, 2024

Choose a reason for hiding this comment

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

Nice this is awesome!

zonglinpeng added the release notes: backends label Nov 20, 2024

Merge branch 'main' into main

e4df82c

ckmadhira requested review from hsharma35 and zonglinpeng November 21, 2024 14:56

zonglinpeng approved these changes Nov 21, 2024

View reviewed changes

mcremon-meta approved these changes Nov 21, 2024

View reviewed changes

Resolved Linter errors

92b58ef

Signed-off-by: [email protected] <[email protected]>

ckmadhira force-pushed the main branch from e4d261a to 92b58ef Compare November 25, 2024 11:54

ckmadhira requested review from mcremon-meta and zonglinpeng November 25, 2024 12:15

zonglinpeng requested changes Nov 25, 2024

View reviewed changes

This comment was marked as outdated.

Sign in to view

mcremon-meta merged commit d778627 into pytorch:main Nov 25, 2024
40 of 41 checks passed

zonglinpeng reviewed Nov 25, 2024

View reviewed changes

zonglinpeng changed the title ~~Added Fusion G3 NN library with kernels related to add, mul, quantize…~~ Milestone 1: Added Fusion G3 NN library with kernels related to add, mul, quantize… Mar 12, 2025

		@@ -0,0 +1,119 @@
		# Copyright (c) Meta Platforms, Inc. and affiliates.



		/* Local function which calls the kernels based on the input datatype */
		void Dequantize_impl(Tensor& out,

Milestone 1: Added Fusion G3 NN library with kernels related to add, mul, quantize… #6738

Milestone 1: Added Fusion G3 NN library with kernels related to add, mul, quantize… #6738

Uh oh!

Conversation

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

Uh oh!

Summary

Uh oh!

pytorch-bot bot commented Nov 8, 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/pytorch/executorch/6738

❌ 1 New Failure

Uh oh!

facebook-github-bot commented Nov 8, 2024

Action Required

Process

Uh oh!

ckmadhira commented Nov 8, 2024

Uh oh!

zonglinpeng left a comment

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hsharma35 commented Nov 8, 2024

Uh oh!

facebook-github-bot commented Nov 11, 2024

Uh oh!

zonglinpeng commented Nov 14, 2024

Uh oh!

zonglinpeng left a comment

Choose a reason for hiding this comment

Uh oh!

ckmadhira commented Nov 14, 2024

Uh oh!

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

Uh oh!

Uh oh!

ckmadhira commented Nov 20, 2024

Uh oh!

hsharma35 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented Nov 20, 2024

Uh oh!

ckmadhira commented Nov 21, 2024

Uh oh!

mcremon-meta commented Nov 22, 2024

Uh oh!

zonglinpeng commented Nov 22, 2024

ckmadhira commented Nov 8, 2024 •

edited

Loading

pytorch-bot bot commented Nov 8, 2024 •

edited

Loading

zonglinpeng commented Nov 19, 2024 •

edited

Loading

ckmadhira Nov 21, 2024 •

edited

Loading

zonglinpeng commented Nov 20, 2024 •

edited

Loading