echuraev commented on code in PR #13849:
URL: https://github.com/apache/tvm/pull/13849#discussion_r1088617781


##########
src/runtime/contrib/random/mt_random_engine.cc:
##########
@@ -192,12 +192,12 @@ class RandomEngine {
     struct ParallelTask {
       static int RunTask(int task_id, TVMParallelGroupEnv* penv, void* cdata) {
         ParallelTask* task = static_cast<ParallelTask*>(cdata);
-        task->Run(task_id);
+        task->Run(task_id, penv->num_task);
         return 0;
       }
 
-      void Run(int i) {
-        int64_t chunk_size = size / num_threads;
+      void Run(int i, int num_threads) {
+        int64_t chunk_size = ceil(size / num_threads);

Review Comment:
   In case when `penv->num_task` is a pretty big number, then is it still 
correct to use this number as a divider for `size`? And how `prev->num_task` is 
correlate with number of threads?
   Is it possible that `size` is less than `num_threads`?



##########
src/runtime/contrib/random/mt_random_engine.cc:
##########
@@ -192,12 +192,12 @@ class RandomEngine {
     struct ParallelTask {
       static int RunTask(int task_id, TVMParallelGroupEnv* penv, void* cdata) {
         ParallelTask* task = static_cast<ParallelTask*>(cdata);
-        task->Run(task_id);
+        task->Run(task_id, penv->num_task);
         return 0;
       }
 
-      void Run(int i) {
-        int64_t chunk_size = size / num_threads;
+      void Run(int i, int num_threads) {

Review Comment:
   ```suggestion
         void Run(int i, int num_tasks) {
   ```



##########
src/runtime/contrib/random/mt_random_engine.cc:
##########
@@ -220,8 +220,7 @@ class RandomEngine {
     }
     if (dtype.bits == 1 || dtype.bits == 4 || dtype.bits == 8 || dtype.bits == 
16 ||
         dtype.bits == 32 || dtype.bits == 64) {
-      int num_threads = task.num_threads = 
runtime::threading::MaxConcurrency();
-      int res = TVMBackendParallelLaunch(ParallelTask::RunTask, &task, 
num_threads);
+      int res = TVMBackendParallelLaunch(ParallelTask::RunTask, &task, 0);

Review Comment:
   It looks like variable `num_threads` from `ParallelTask` structure doesn't 
use anymore. Should we remove it from the structure?



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to