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

[µTVM] Add platform timer and RPCTimeEvaluator to enable AutoTVM #6964

Merged
merged 5 commits into from
Dec 28, 2020

Conversation

areusch
Copy link
Contributor

@areusch areusch commented Nov 24, 2020

This PR adds a platform-specific timing function plus a device-side implementation of RPCTimeEvaluator. In a follow-on, an AutoTVM test will be added (there are a few different dependent PRs in flight here so need to let them all land before I can test that properly).

@areusch areusch force-pushed the utvm-platform-timer branch 3 times, most recently from 27b1561 to f464ee6 Compare December 11, 2020 18:20
@areusch areusch marked this pull request as ready for review December 12, 2020 01:58
@areusch
Copy link
Contributor Author

areusch commented Dec 12, 2020

@@ -43,7 +43,7 @@
#define TVM_CRT_MAX_REGISTERED_MODULES 2

/*! Size of the global function registry, in bytes. */
#define TVM_CRT_GLOBAL_FUNC_REGISTRY_SIZE_BYTES 200
#define TVM_CRT_GLOBAL_FUNC_REGISTRY_SIZE_BYTES 256
Copy link
Contributor

Choose a reason for hiding this comment

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

out of curiosity, what's the reason for increasing the registry size? just having a nice power of 2?

Copy link
Contributor

@tmoreau89 tmoreau89 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 PR @areusch, LGTM

@tmoreau89
Copy link
Contributor

@liangfu care to take a pass? thanks!

Copy link
Member

@liangfu liangfu left a comment

Choose a reason for hiding this comment

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

The changes look good, except for a minor change request.

On the other hand, I think it's time to consider using a goto chain when leaving a function on error, since we could easily leave the function with a memory leak in C implementation.

src/runtime/crt/common/crt_runtime_api.c Outdated Show resolved Hide resolved
@liangfu
Copy link
Member

liangfu commented Dec 24, 2020

ping @areusch

@areusch
Copy link
Contributor Author

areusch commented Dec 24, 2020

sorry @liangfu for the delay. i've addressed your comment and sync'd to main.

Copy link
Member

@liangfu liangfu 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 update, the changes looks good.

@areusch
Copy link
Contributor Author

areusch commented Dec 27, 2020

@tqchen I think this is ready to merge

@tqchen
Copy link
Member

tqchen commented Dec 28, 2020

@liangfu @tmoreau89 feel free to merge next time given both committer approves :) no need to block on myself

@tqchen tqchen merged commit b8ac8d9 into apache:main Dec 28, 2020
tkonolige pushed a commit to tkonolige/incubator-tvm that referenced this pull request Jan 11, 2021
…che#6964)

* Add platform timer to microTVM.

* Address liangfu comments

* cppformat

* clang-format

Co-authored-by: Liangfu Chen <[email protected]>
TusharKanekiDey pushed a commit to TusharKanekiDey/tvm that referenced this pull request Jan 20, 2021
…che#6964)

* Add platform timer to microTVM.

* Address liangfu comments

* cppformat

* clang-format

Co-authored-by: Liangfu Chen <[email protected]>
trevor-m pushed a commit to neo-ai/tvm that referenced this pull request Jan 21, 2021
…che#6964)

* Add platform timer to microTVM.

* Address liangfu comments

* cppformat

* clang-format

Co-authored-by: Liangfu Chen <[email protected]>
electriclilies pushed a commit to electriclilies/tvm that referenced this pull request Feb 18, 2021
…che#6964)

* Add platform timer to microTVM.

* Address liangfu comments

* cppformat

* clang-format

Co-authored-by: Liangfu Chen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants