[libc-commits] [PATCH] D148485: [libc] Add the '--threads' and '--blocks' option to the GPU loaders

[Joseph Huber via Phabricator via libc-commits]

jhuber6 added inline comments.


================
Comment at: libc/utils/gpu/loader/Main.cpp:34
+    if (argv[offset] == std::string("--threads")) {
+      params.num_threads_x = offset + 1 < argc ? atoi(argv[offset + 1]) : 1;
+      offset++;
----------------
tra wrote:
> Nit: Can we use `strtoul` instead? Sometimes it's convenient to be able to use hex numbers and you would also get an indication if the input is not a number.
Good point, just used to using the quick-and-dirty `atoi`.


================
Comment at: libc/utils/gpu/loader/nvptx/Loader.cpp:137-139
+          cuLaunchKernel(function, params.num_blocks_x, /*gridDimY=*/1,
+                         /*gridDimZ=*/1, params.num_threads_x, /*blockDimY=*/1,
                          /*bloackDimZ=*/1, 0, stream, nullptr, args_config))
----------------
tra wrote:
> If we're allowing controlling the number of blocks/threads at all, is there a reason not to allow specifying all dimensions?
I wasn't sure if I should bother, but I could definitely add it since it's hardly anymore work from what's here.

I think internally for implementations we'll need to generate all of our thread id's using the full dimensions as well, but that's probably standard.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D148485/new/

https://reviews.llvm.org/D148485