You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2022/06/03 06:58:29 UTC

[GitHub] [tvm] fPecc opened a new pull request, #11244: [CRT runtime] Added functions TVMPlatformPreFuncCall and TVMPlatformPostFuncCall

fPecc opened a new pull request, #11244:
URL: https://github.com/apache/tvm/pull/11244

   See [this thread ](https://discuss.tvm.apache.org/t/crt-add-platform-specific-pre-and-post-function-calls-in-crt-runtime/12723)for an explanation.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] fPecc closed pull request #11244: [CRT runtime] Added functions TVMPlatformPreFuncCall and TVMPlatformPostFuncCall

Posted by GitBox <gi...@apache.org>.
fPecc closed pull request #11244: [CRT runtime] Added functions TVMPlatformPreFuncCall and TVMPlatformPostFuncCall
URL: https://github.com/apache/tvm/pull/11244


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] fPecc commented on a diff in pull request #11244: [CRT runtime] Added functions TVMPlatformPreFuncCall and TVMPlatformPostFuncCall

Posted by GitBox <gi...@apache.org>.
fPecc commented on code in PR #11244:
URL: https://github.com/apache/tvm/pull/11244#discussion_r879505849


##########
src/runtime/crt/common/crt_runtime_api.c:
##########
@@ -526,6 +526,11 @@ tvm_crt_error_t RunTimeEvaluator(tvm_function_index_t function_index, TVMValue*
     int exec_count = 0;
     // do-while structure ensures we run even when `min_repeat_ms` isn't set (i.e., is 0).
     do {
+      err = TVMPlatformBeforeMeasurement();
+      if (err != kTvmErrorNoError && err != kTvmErrorFunctionCallNotImplemented) {

Review Comment:
   Just committed the change.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] areusch commented on a diff in pull request #11244: [CRT runtime] Added functions TVMPlatformPreFuncCall and TVMPlatformPostFuncCall

Posted by GitBox <gi...@apache.org>.
areusch commented on code in PR #11244:
URL: https://github.com/apache/tvm/pull/11244#discussion_r876261416


##########
include/tvm/runtime/crt/platform.h:
##########
@@ -116,6 +116,24 @@ tvm_crt_error_t TVMPlatformTimerStop(double* elapsed_time_seconds);
  */
 tvm_crt_error_t TVMPlatformGenerateRandom(uint8_t* buffer, size_t num_bytes);
 
+/*! \brief Platform-specific before measurement call.
+ *
+ * A function which is called before calling TVMFuncCall in the RuntimeEvaluator.
+ * Can be used, for example, to initialize stuff using platform-specific code.

Review Comment:
   ```suggestion
    * Can be used, for example, to reset global state which may affect the results of measurement.
   ```



##########
src/runtime/crt/common/crt_runtime_api.c:
##########
@@ -526,6 +526,11 @@ tvm_crt_error_t RunTimeEvaluator(tvm_function_index_t function_index, TVMValue*
     int exec_count = 0;
     // do-while structure ensures we run even when `min_repeat_ms` isn't set (i.e., is 0).
     do {
+      err = TVMPlatformBeforeMeasurement();
+      if (err != kTvmErrorNoError && err != kTvmErrorFunctionCallNotImplemented) {

Review Comment:
   i think you could just do the one check `err != kTvmErrorNoError` and return `kTvmErrorNoError` by default from the weak-linked functions. thoughts?



##########
include/tvm/runtime/crt/platform.h:
##########
@@ -116,6 +116,24 @@ tvm_crt_error_t TVMPlatformTimerStop(double* elapsed_time_seconds);
  */
 tvm_crt_error_t TVMPlatformGenerateRandom(uint8_t* buffer, size_t num_bytes);
 
+/*! \brief Platform-specific before measurement call.
+ *
+ * A function which is called before calling TVMFuncCall in the RuntimeEvaluator.

Review Comment:
   ```suggestion
    * A function which is called before calling TVMFuncCall in the TimeEvaluator.
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] fPecc commented on a diff in pull request #11244: [CRT runtime] Added functions TVMPlatformPreFuncCall and TVMPlatformPostFuncCall

Posted by GitBox <gi...@apache.org>.
fPecc commented on code in PR #11244:
URL: https://github.com/apache/tvm/pull/11244#discussion_r879499307


##########
include/tvm/runtime/crt/platform.h:
##########
@@ -116,6 +116,24 @@ tvm_crt_error_t TVMPlatformTimerStop(double* elapsed_time_seconds);
  */
 tvm_crt_error_t TVMPlatformGenerateRandom(uint8_t* buffer, size_t num_bytes);
 
+/*! \brief Platform-specific before measurement call.
+ *
+ * A function which is called before calling TVMFuncCall in the RuntimeEvaluator.

Review Comment:
   Fine by me!



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] fPecc commented on pull request #11244: [CRT runtime] Added functions TVMPlatformPreFuncCall and TVMPlatformPostFuncCall

Posted by GitBox <gi...@apache.org>.
fPecc commented on PR #11244:
URL: https://github.com/apache/tvm/pull/11244#issuecomment-1128463379

   @areusch I changed the names accordingly like we discussed on the forum.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] areusch commented on pull request #11244: [CRT runtime] Added functions TVMPlatformPreFuncCall and TVMPlatformPostFuncCall

Posted by GitBox <gi...@apache.org>.
areusch commented on PR #11244:
URL: https://github.com/apache/tvm/pull/11244#issuecomment-1146091347

   @fPecc i think this one needs
   ```
   $ docker/lint.sh -i clang_format
   ```


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] Mousius commented on pull request #11244: [CRT runtime] Added functions TVMPlatformPreFuncCall and TVMPlatformPostFuncCall

Posted by GitBox <gi...@apache.org>.
Mousius commented on PR #11244:
URL: https://github.com/apache/tvm/pull/11244#issuecomment-1135610437

   > Hi @areusch ,
   > 
   > It seems the problem with the CI is more deeper than the changes I made:
   > 
   > ```
   > [2022-05-23T14:13:06.372Z] Check Jenkinsfile generation
   > [2022-05-23T14:13:06.372Z] Traceback (most recent call last):
   > [2022-05-23T14:13:06.372Z]   File "jenkins/generate.py", line 18, in <module>
   > [2022-05-23T14:13:06.372Z]     import jinja2
   > [2022-05-23T14:13:06.372Z] ModuleNotFoundError: No module named 'jinja2'`
   > ```
   
   Have you rebased recently? This change landed which installs that dependency in one of the scripts (it will eventually move to the image itself):
   https://github.com/apache/tvm/pull/11258/files#diff-137a515f2ac93c13003517d7a4ca908dcbaeb6dff50d05470579e633ddec354fR36


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] areusch merged pull request #11244: [CRT runtime] Added functions TVMPlatformBeforeMeasurement and TVMPlatformAfterMeasurement

Posted by GitBox <gi...@apache.org>.
areusch merged PR #11244:
URL: https://github.com/apache/tvm/pull/11244


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] fPecc commented on a diff in pull request #11244: [CRT runtime] Added functions TVMPlatformPreFuncCall and TVMPlatformPostFuncCall

Posted by GitBox <gi...@apache.org>.
fPecc commented on code in PR #11244:
URL: https://github.com/apache/tvm/pull/11244#discussion_r879504955


##########
src/runtime/crt/common/crt_runtime_api.c:
##########
@@ -526,6 +526,11 @@ tvm_crt_error_t RunTimeEvaluator(tvm_function_index_t function_index, TVMValue*
     int exec_count = 0;
     // do-while structure ensures we run even when `min_repeat_ms` isn't set (i.e., is 0).
     do {
+      err = TVMPlatformBeforeMeasurement();
+      if (err != kTvmErrorNoError && err != kTvmErrorFunctionCallNotImplemented) {

Review Comment:
   Yes, that would also work.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] fPecc commented on pull request #11244: [CRT runtime] Added functions TVMPlatformPreFuncCall and TVMPlatformPostFuncCall

Posted by GitBox <gi...@apache.org>.
fPecc commented on PR #11244:
URL: https://github.com/apache/tvm/pull/11244#issuecomment-1121116278

   @Mousius @leandron @areusch


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] driazati commented on pull request #11244: [CRT runtime] Added functions TVMPlatformPreFuncCall and TVMPlatformPostFuncCall

Posted by GitBox <gi...@apache.org>.
driazati commented on PR #11244:
URL: https://github.com/apache/tvm/pull/11244#issuecomment-1138950637

   hey @fPecc it looks like the merge has got github all confused (it's still using the old `Jenkinsfile` to run CI which won't work). This snippet should wipe out the local branch and rebase it to just your changes:
   
   ```bash
   # get the git diff of your change
   git fetch origin main
   git checkout crt-pre-post-functions
   git merge origin/main
   git diff $(git merge-base origin/main HEAD) HEAD --stat > ~/change.diff
   
   # get to a clean branch
   git reset --hard origin/main
   
   # replay your changes
   cat ~/change.diff | patch -p1 -d .
   git add .
   git commit -m"[CRT runtime] Added functions TVMPlatformPreFuncCall and TVMPlatformPostFuncCall"
   git push --force
   ```


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] fPecc commented on pull request #11244: [CRT runtime] Added functions TVMPlatformPreFuncCall and TVMPlatformPostFuncCall

Posted by GitBox <gi...@apache.org>.
fPecc commented on PR #11244:
URL: https://github.com/apache/tvm/pull/11244#issuecomment-1136016205

   @areusch @Mousius just rebased to integrate that commit


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] areusch commented on pull request #11244: [CRT runtime] Added functions TVMPlatformPreFuncCall and TVMPlatformPostFuncCall

Posted by GitBox <gi...@apache.org>.
areusch commented on PR #11244:
URL: https://github.com/apache/tvm/pull/11244#issuecomment-1124200930

   i commented on the discuss forum thread. let's resolve there and then continue this PR.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] Mousius commented on pull request #11244: [CRT runtime] Added functions TVMPlatformPreFuncCall and TVMPlatformPostFuncCall

Posted by GitBox <gi...@apache.org>.
Mousius commented on PR #11244:
URL: https://github.com/apache/tvm/pull/11244#issuecomment-1135603689

   @areusch could we not test these functions by implementing the weak symbols in test code?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] fPecc commented on a diff in pull request #11244: [CRT runtime] Added functions TVMPlatformPreFuncCall and TVMPlatformPostFuncCall

Posted by GitBox <gi...@apache.org>.
fPecc commented on code in PR #11244:
URL: https://github.com/apache/tvm/pull/11244#discussion_r879499680


##########
include/tvm/runtime/crt/platform.h:
##########
@@ -116,6 +116,24 @@ tvm_crt_error_t TVMPlatformTimerStop(double* elapsed_time_seconds);
  */
 tvm_crt_error_t TVMPlatformGenerateRandom(uint8_t* buffer, size_t num_bytes);
 
+/*! \brief Platform-specific before measurement call.
+ *
+ * A function which is called before calling TVMFuncCall in the RuntimeEvaluator.
+ * Can be used, for example, to initialize stuff using platform-specific code.

Review Comment:
   Fine! Better comment



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] fPecc commented on pull request #11244: [CRT runtime] Added functions TVMPlatformPreFuncCall and TVMPlatformPostFuncCall

Posted by GitBox <gi...@apache.org>.
fPecc commented on PR #11244:
URL: https://github.com/apache/tvm/pull/11244#issuecomment-1135583074

   Hi @areusch ,
   
   It seems the problem with the CI is more deeper than the changes I made:
   
       [2022-05-23T14:13:06.372Z] Check Jenkinsfile generation
       [2022-05-23T14:13:06.372Z] Traceback (most recent call last):
       [2022-05-23T14:13:06.372Z]   File "jenkins/generate.py", line 18, in <module>
       [2022-05-23T14:13:06.372Z]     import jinja2
       [2022-05-23T14:13:06.372Z] ModuleNotFoundError: No module named 'jinja2'`


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org