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.

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

[microNPU] Introduce a pass to remove redundant identity operations #10254

Merged
merged 5 commits into from
Mar 8, 2022

Conversation

lhutton1
Copy link
Contributor

Introduces a pass that aims to remove identity operations, introduced during legalization, that are immediately followed by an NPU compute operation e.g. Convolution.

cc @manupa-arm @ekalda @mbaret @NicolaLancellotti @jacobbohlin @dchauhan-arm

Copy link
Contributor

@ekalda ekalda left a comment

Choose a reason for hiding this comment

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

Thanks for the patch, looks really good so far! :) Maybe one testcase to add (to the already very comprehensive set) would be to check that there are no reshape or strided slice directly followed by concatenate.

@lhutton1
Copy link
Contributor Author

Thanks for the review @ekalda, I added a test case and removed the print among a couple more things I spotted :)

Copy link
Contributor

@ekalda ekalda left a comment

Choose a reason for hiding this comment

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

Thanks @lhutton1, looks good to me! :)

@lhutton1 lhutton1 force-pushed the identity-optimizer branch 2 times, most recently from 0db7b03 to 7d2b6c8 Compare February 17, 2022 11:49
@lhutton1
Copy link
Contributor Author

So it turns out supporting the removal of identity operations with multiple outputs was not as complicated as I first thought, and it was technically already supported in the original patch. Therefore, CountIdentityOutputs and the restriction in RemoveRedundantIdentities can be removed. Added tests for the multiple output case to support this. Apologies for the confusion @ekalda, could you take another look when you have time :)

Copy link
Contributor

@ekalda ekalda left a comment

Choose a reason for hiding this comment

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

Thanks @lhutton1 , looks all good to me! :)

Introduces a  pass that aims to remove identity operations,
introduced during legalization, that are immediately followed by an NPU
compute operation e.g. Convolution.

Change-Id: Ia3b2c7bebf8cba1f827af8e3f3335677ba8f6371
Change-Id: Idf9341ce757b849f8819944dab2fb3b1496a2caf
Changes in test_identity_optimizer.py:
* Fixed typo in docstring
* Removed print
* Fixed same output test to use correct input shape

Changes in codegen.cc:
* Remove unnecessary constructor

Change-Id: Ie4a053725110ce52d8be039ca1ce48084bc66545
Change-Id: I0a88d92dd31ca3dd07a2a495f18c10a2ebf2fc9e
lhutton1 added a commit to lhutton1/tvm that referenced this pull request Feb 28, 2022
Builds upon the work in apache#10254 to remove identity operations sandwiched
between two non-compute operations (reshape/strided slice - concatenate
is handled differently), under certain conditions. Specifically, an
identity operation is not removed when the dimensionality between the
two non-compute operations is reduced, due to non-congruent values
being accessed incorrectly. For example,

```
strided_slice(dims=4) -> identity -> reshape(dims=4)
```
becomes...
```
strided_slice -> reshape
```
but,
```
strided_slice(dims=4) -> identity -> reshape(dims=2)
```
becomes...
```
strided_slice -> reshape
```

Change-Id: Ie28ba384fcb3230d6f4651c0c19e2b9526ebcc42
lhutton1 added a commit to lhutton1/tvm that referenced this pull request Feb 28, 2022
Builds upon the work in apache#10254 to remove identity operations sandwiched
between two non-compute operations (reshape/strided slice - concatenate
is handled differently), under certain conditions. Specifically, an
identity operation is not removed when the dimensionality between the
two non-compute operations is reduced, due to non-congruent values
being accessed incorrectly. For example,

```
strided_slice(dims=4) -> identity -> reshape(dims=4)
```
becomes...
```
strided_slice -> reshape
```
but,
```
strided_slice(dims=4) -> identity -> reshape(dims=2)
```
remains as...
```
strided_slice -> identity -> reshape
```

Change-Id: Ie28ba384fcb3230d6f4651c0c19e2b9526ebcc42
Change-Id: Ib54031fe1c70159728876a23f96b72adb2ea17b0
Copy link
Contributor

@manupak manupak left a comment

Choose a reason for hiding this comment

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

LGTM!

@manupak manupak merged commit 5e81389 into apache:main Mar 8, 2022
@manupak
Copy link
Contributor

manupak commented Mar 8, 2022

Thanks @ekalda @lhutton1 !

@lhutton1 lhutton1 deleted the identity-optimizer branch March 9, 2022 10:13
lhutton1 added a commit to lhutton1/tvm that referenced this pull request Mar 9, 2022
Builds upon the work in apache#10254 to remove identity operations sandwiched
between two non-compute operations (reshape/strided slice - concatenate
is handled differently), under certain conditions. Specifically, an
identity operation is not removed when the dimensionality between the
two non-compute operations is reduced, due to non-congruent values
being accessed incorrectly. For example,

```
strided_slice(dims=4) -> identity -> reshape(dims=4)
```
becomes...
```
strided_slice -> reshape
```
but,
```
strided_slice(dims=4) -> identity -> reshape(dims=2)
```
remains as...
```
strided_slice -> identity -> reshape
```

Change-Id: Ie28ba384fcb3230d6f4651c0c19e2b9526ebcc42
ziqiangxu8457 pushed a commit to ziqiangxu8457/tvm that referenced this pull request Mar 9, 2022
…pache#10254)

* [microNPU] Introduce a pass to remove redundant identity operations

Introduces a  pass that aims to remove identity operations,
introduced during legalization, that are immediately followed by an NPU
compute operation e.g. Convolution.

Change-Id: Ia3b2c7bebf8cba1f827af8e3f3335677ba8f6371

* fix lint

Change-Id: Idf9341ce757b849f8819944dab2fb3b1496a2caf

* Addressing comments

Changes in test_identity_optimizer.py:
* Fixed typo in docstring
* Removed print
* Fixed same output test to use correct input shape

Changes in codegen.cc:
* Remove unnecessary constructor

Change-Id: Ie4a053725110ce52d8be039ca1ce48084bc66545

* skip tests when required packages are not available

Change-Id: I0a88d92dd31ca3dd07a2a495f18c10a2ebf2fc9e

* support multiple output identities and add more tests

Change-Id: Ib54031fe1c70159728876a23f96b72adb2ea17b0
lhutton1 added a commit to lhutton1/tvm that referenced this pull request Mar 23, 2022
Builds upon the work in apache#10254 to remove identity operations sandwiched
between two non-compute operations (reshape/strided slice - concatenate
is handled differently), under certain conditions. Specifically, an
identity operation is not removed when the dimensionality between the
two non-compute operations is reduced, due to non-congruent values
being accessed incorrectly. For example,

```
strided_slice(dims=4) -> identity -> reshape(dims=4)
```
becomes...
```
strided_slice -> reshape
```
but,
```
strided_slice(dims=4) -> identity -> reshape(dims=2)
```
remains as...
```
strided_slice -> identity -> reshape
```

Change-Id: Ie28ba384fcb3230d6f4651c0c19e2b9526ebcc42
manupak pushed a commit that referenced this pull request Mar 25, 2022
…10411)

Builds upon the work in #10254 to remove identity operations sandwiched
between two non-compute operations (reshape/strided slice - concatenate
is handled differently), under certain conditions. Specifically, an
identity operation is not removed when the dimensionality between the
two non-compute operations is reduced, due to non-congruent values
being accessed incorrectly. For example,

```
strided_slice(dims=4) -> identity -> reshape(dims=4)
```
becomes...
```
strided_slice -> reshape
```
but,
```
strided_slice(dims=4) -> identity -> reshape(dims=2)
```
remains as...
```
strided_slice -> identity -> reshape
```

Change-Id: Ie28ba384fcb3230d6f4651c0c19e2b9526ebcc42
pfk-beta pushed a commit to pfk-beta/tvm that referenced this pull request Apr 11, 2022
…pache#10254)

* [microNPU] Introduce a pass to remove redundant identity operations

Introduces a  pass that aims to remove identity operations,
introduced during legalization, that are immediately followed by an NPU
compute operation e.g. Convolution.

Change-Id: Ia3b2c7bebf8cba1f827af8e3f3335677ba8f6371

* fix lint

Change-Id: Idf9341ce757b849f8819944dab2fb3b1496a2caf

* Addressing comments

Changes in test_identity_optimizer.py:
* Fixed typo in docstring
* Removed print
* Fixed same output test to use correct input shape

Changes in codegen.cc:
* Remove unnecessary constructor

Change-Id: Ie4a053725110ce52d8be039ca1ce48084bc66545

* skip tests when required packages are not available

Change-Id: I0a88d92dd31ca3dd07a2a495f18c10a2ebf2fc9e

* support multiple output identities and add more tests

Change-Id: Ib54031fe1c70159728876a23f96b72adb2ea17b0
pfk-beta pushed a commit to pfk-beta/tvm that referenced this pull request Apr 11, 2022
…pache#10411)

Builds upon the work in apache#10254 to remove identity operations sandwiched
between two non-compute operations (reshape/strided slice - concatenate
is handled differently), under certain conditions. Specifically, an
identity operation is not removed when the dimensionality between the
two non-compute operations is reduced, due to non-congruent values
being accessed incorrectly. For example,

```
strided_slice(dims=4) -> identity -> reshape(dims=4)
```
becomes...
```
strided_slice -> reshape
```
but,
```
strided_slice(dims=4) -> identity -> reshape(dims=2)
```
remains as...
```
strided_slice -> identity -> reshape
```

Change-Id: Ie28ba384fcb3230d6f4651c0c19e2b9526ebcc42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants