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.

Sign up for GitHub

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

Jump to bottom

[microNPU] Integrate rolling buffers in Arm(R) Ethos(TM)-U #10344

Merged
merged 4 commits into from
Apr 21, 2022
Merged

[microNPU] Integrate rolling buffers in Arm(R) Ethos(TM)-U #10344

merged 4 commits into from
Apr 21, 2022

Conversation

NicolaLancellotti
Copy link
Contributor

This pr adds support for rolling buffers in Arm(R) Ethos(TM)-U.

@NicolaLancellotti
Copy link
Contributor Author

@mbaret @manupa-arm @ekalda @lhutton1 @jacobbohlin

@NicolaLancellotti NicolaLancellotti force-pushed the ethosu/rolling_buffer branch 2 times, most recently from e59295f to 3050d19 Compare February 23, 2022 10:12
manupak
manupak reviewed Feb 25, 2022
View reviewed changes
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.

Broadly LGTM! (bar a nit)

A review from others are appreciated here!

python/tvm/relay/backend/contrib/ethosu/codegen.py Outdated Show resolved Hide resolved
@NicolaLancellotti NicolaLancellotti force-pushed the ethosu/rolling_buffer branch 2 times, most recently from 066026b to 9ccc78f Compare February 27, 2022 14:00
@NicolaLancellotti
Copy link
Contributor Author

The CI problems have been fixed. Other reviews are appreciated.

ekalda
ekalda reviewed Mar 1, 2022
View reviewed changes
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.

Looks good to me! :) Some questions and suggestions...

python/tvm/relay/backend/contrib/ethosu/tir/passes.py Outdated Show resolved Hide resolved
python/tvm/relay/backend/contrib/ethosu/tir/producers_consumers.py Show resolved Hide resolved
python/tvm/relay/backend/contrib/ethosu/tir/transform.py Show resolved Hide resolved
python/tvm/relay/backend/contrib/ethosu/tir_to_cs_translator.py Outdated Show resolved Hide resolved
python/tvm/relay/backend/contrib/ethosu/tir/dma.py Show resolved Hide resolved
manupak
manupak requested changes Mar 18, 2022
View reviewed changes
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.

@NicolaLancellotti the previous changes still look good.

Just some concerns about the check_rolling_buffer function, otherwise LGTM.

python/tvm/relay/backend/contrib/ethosu/tir/dma.py Outdated Show resolved Hide resolved
python/tvm/relay/backend/contrib/ethosu/tir/dma.py Outdated Show resolved Hide resolved
python/tvm/relay/backend/contrib/ethosu/tir/dma.py Outdated Show resolved Hide resolved
@NicolaLancellotti
Copy link
Contributor Author

I rebased the first commit and I addressed the comments in the second one.

@manupak manupak added the status: need update need update based on feedbacks label Mar 30, 2022
@areusch areusch removed their request for review April 8, 2022 22:58
manupak
manupak reviewed Apr 19, 2022
View reviewed changes
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.

Mostly LGTM bar one request for docs.

python/tvm/relay/backend/contrib/ethosu/tir/dma.py Show resolved Hide resolved
manupak
manupak approved these changes Apr 20, 2022
View reviewed changes
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!

python/tvm/relay/backend/contrib/ethosu/tir/dma.py Show resolved Hide resolved
@manupak manupak merged commit c07a463 into apache:main Apr 21, 2022
@manupak
Copy link
Contributor

manupak commented Apr 21, 2022

Thanks @NicolaLancellotti @ekalda !

@lhutton1 lhutton1 mentioned this pull request Apr 22, 2022
Merged
shtinsa pushed a commit to Deelvin/tvm that referenced this pull request May 17, 2022
@NicolaLancellotti @shtinsa
[microNPU] Integrate rolling buffers in Arm(R) Ethos(TM)-U (apache#10344
863e01e 
)

* [microNPU] Integrate rolling buffers in Arm(R) Ethos(TM)-U

Change-Id: Iede5e68981a063f6eb1e118433cc2c92e175af52

* Add documentation for create_tiles

* Fix linter issues

* Fix integration tests
juda pushed a commit to juda/tvm that referenced this pull request Jun 21, 2022
@NicolaLancellotti
[microNPU] Integrate rolling buffers in Arm(R) Ethos(TM)-U (apache#10344
41d7cc6 
)

* [microNPU] Integrate rolling buffers in Arm(R) Ethos(TM)-U

Change-Id: Iede5e68981a063f6eb1e118433cc2c92e175af52

* Add documentation for create_tiles

* Fix linter issues

* Fix integration tests
@driazati driazati mentioned this pull request Jul 14, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ekalda ekalda ekalda left review comments

@manupak manupak manupak approved these changes

@jroesch jroesch Awaiting requested review from jroesch

@slyubomirsky slyubomirsky Awaiting requested review from slyubomirsky

@icemelon icemelon Awaiting requested review from icemelon

@MarisaKirisame MarisaKirisame Awaiting requested review from MarisaKirisame

@ZihengJiang ZihengJiang Awaiting requested review from ZihengJiang

@yzhliu yzhliu Awaiting requested review from yzhliu

@vinx13 vinx13 Awaiting requested review from vinx13

@mbrookhart mbrookhart Awaiting requested review from mbrookhart

@jwfromm jwfromm Awaiting requested review from jwfromm

@zhiics zhiics Awaiting requested review from zhiics

@anijain2305 anijain2305 Awaiting requested review from anijain2305

@wweic wweic Awaiting requested review from wweic

@junrushao junrushao Awaiting requested review from junrushao

@tqchen tqchen Awaiting requested review from tqchen

@kparzysz-quic kparzysz-quic Awaiting requested review from kparzysz-quic

@masahi masahi Awaiting requested review from masahi

@Hzfengsy Hzfengsy Awaiting requested review from Hzfengsy

@comaniac comaniac Awaiting requested review from comaniac

@merrymercy merrymercy Awaiting requested review from merrymercy

Assignees
No one assigned
Labels
status: need update need update based on feedbacks
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@NicolaLancellotti @manupak @ekalda