[Openmp-commits] [PATCH] D105239: [libomptarget][nfc] Group environment variables, drop accesses to DeviceInfo global
Jon Chesterfield via Phabricator via Openmp-commits
openmp-commits at lists.llvm.org
Thu Jul 1 01:34:24 PDT 2021
JonChesterfield added inline comments.
================
Comment at: openmp/libomptarget/plugins/amdgpu/src/rtl.cpp:2098
if (print_kernel_trace >= LAUNCH) {
+ int num_groups = GridSize / WorkgroupSize;
// enum modes are SPMD, GENERIC, NONE 0,1,2
----------------
JonChesterfield wrote:
> dhruvachak wrote:
> > Since we already computed num_groups within getLaunchVals, should we return that as well? Otherwise, we have duplicate computation of num_groups and they may go out of sync. Basically, the structure launchVals can have 3 fields including NumGroups.
> It isn't used for anything aside from printing, and the printing will need to change if we start using a value that isn't a multiple of workgroup size anyway.
>
> Adding a redundant third return field that is ~unused would make the function harder to test. Suggest changing the print statement if it's the division you'd like to remove.
Oh, and this part is also in the smaller patch proposed before this one. I was hoping we could review the base first, then rebase this for better signal to noise. I can't remember how to build stacked diffs in phab
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D105239/new/
https://reviews.llvm.org/D105239
More information about the Openmp-commits
mailing list