[Openmp-commits] [PATCH] D93727: [OpenMP] Add using bit flags to select Libomptarget Information

[Joseph Huber via Phabricator via Openmp-commits]

jhuber6 added inline comments.


================
Comment at: openmp/libomptarget/src/interface.cpp:237
   }
 
 #ifdef OMPTARGET_DEBUG
----------------
grokos wrote:
> jhuber6 wrote:
> > grokos wrote:
> > > Wouldn't we want to have
> > > ```
> > >   if (getInfoLevel() & OMP_INFOTYPE_KERNEL_ARGS)
> > >     printKernelArguments(loc, device_id, arg_num, arg_sizes, arg_types,
> > >                          arg_names);
> > > ```
> > > here as well? It is useful to see what arguments we have if something goes wrong at a `target exit data` directive or upon exiting a `target data` scope.
> > This method just prints the arguments to `__tgt_target_data_begin_mapper` and `__tgt_target_data_end_mapper`, which should have the same arguments right? I had it in previously but it didn't add any useful information when I tested it. I could add a separate message that just says something like "Exiting data region started at (loc)". Would that be good?
> For scoped constructs (e.g. `omp target data`) this is true, whatever arguments we have in `begin_mapper` the very same arguments we have in `end_mapper`. But what if we have a standalone directive (e.g. `omp target exit data`)? If I recall correctly (currently on vacation with no access to a working llvm environment), `omp target exit data` is compiled into a `__tgt_target_data_end_mapper` call. In this case there may be no matching `enter data` directive so if we don't call `printKernelArguments()` here the user will not be able to see what arguments `end_mapper` was called with.
That's true, I should probably just make it print whether its a data region entry, device kernel, or data region exit. That way it's clear what the flow is. Otherwise it would be confusing because it would still say that a variable is mapped "to" at the end of a data region.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93727