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] Expose compute cycle annotations to TIR lowering #11288

Merged
merged 6 commits into from
May 26, 2022

Conversation

lhutton1
Copy link
Contributor

@lhutton1 lhutton1 commented May 12, 2022

Adds an AttrSttmt "compute_cycles" to each NPU operation for later passes to consume.

cc @Mousius @NicolaLancellotti @ekalda @manupa-arm

@github-actions github-actions bot requested a review from manupak May 12, 2022 13:14
@manupak
Copy link
Contributor

manupak commented May 12, 2022

Thanks @lhutton1! I'd suggest let us use "compute_cycles_hint" to make it closer to what it really is.

Also, do you think that we would be able to add a RelayToTIR(...) hook test that shows the annotations ?

@lhutton1 lhutton1 force-pushed the add-compute-cycles-pragma branch from 0ac943b to 84ad95a Compare May 13, 2022 09:07
@github-actions github-actions bot requested a review from Mousius May 13, 2022 09:07
@lhutton1
Copy link
Contributor Author

Thanks for the review @manupa-arm, I've pushed those changes

@lhutton1 lhutton1 force-pushed the add-compute-cycles-pragma branch from 84ad95a to a8f9bb2 Compare May 16, 2022 13:52
@lhutton1 lhutton1 marked this pull request as draft May 16, 2022 13:54
@lhutton1 lhutton1 force-pushed the add-compute-cycles-pragma branch from a8f9bb2 to 954c56e Compare May 17, 2022 10:50
@lhutton1 lhutton1 marked this pull request as ready for review May 17, 2022 12:49
@ekalda
Copy link
Contributor

ekalda commented May 18, 2022

@lhutton1 FYI, I think this patch needs to be rebased on top of #10959. If I run your patch locally on top of the copy compute reordering patch, it fails in CopyComputeReordeing with

E     Check failed: (eval_node) is false: Expected statement to be an evaluate node, but was tir.AttrStmt

so we shouldn't merge it before the rebase is done, despite CI being green...

lhutton1 added 5 commits May 24, 2022 10:05
Adds an AttrSttmt "compute_cycles_hint" to each NPU operation for later
passes to consume.

Change-Id: I09779bdab6de6ef2094db610bb20d6e052e68ee3
Change-Id: Iebd71e699522e92a28fd321ffdb41ed7924db4e0
Change-Id: Idcdcc8c8b5536c4732f297246b71aa8378a2732c
Change-Id: I007ba19732e16081fa2ea9baca40c64a653c93cf
Change-Id: Ib812c4151fab03f4c1adcc016b4e798003a22e5e
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 (bar the rebase)!

Change-Id: I653101908706096ae25ad1ebf08e7b6c4f1196c7
@lhutton1 lhutton1 force-pushed the add-compute-cycles-pragma branch from 954c56e to e5a7542 Compare May 24, 2022 13:36
Copy link
Contributor

@NicolaLancellotti NicolaLancellotti left a comment

Choose a reason for hiding this comment

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

LGTM!

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 f6ddd52 into apache:main May 26, 2022
@manupak
Copy link
Contributor

manupak commented May 26, 2022

Thanks @lhutton1 @ekalda @NicolaLancellotti !

@lhutton1 lhutton1 deleted the add-compute-cycles-pragma branch May 26, 2022 13:39
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.

4 participants