[Openmp-commits] [PATCH] D83062: [OpenMP] Implement TR8 `present` map type modifier in runtime (2/2)

[George Rokos via Phabricator via Openmp-commits]

grokos added inline comments.


================
Comment at: openmp/libomptarget/src/omptarget.cpp:410
+        return OFFLOAD_FAIL;
+    }
 
----------------
jdenny wrote:
> grokos wrote:
> > jdenny wrote:
> > > jdoerfert wrote:
> > > > Can we print the error in non debug mode as well? Is there a way we can provide more information on the location and variable?
> > > I would prefer that too.  When I investigated this originally, I didn't find a good precedent in this runtime implementation, so I decided to be conservative.  Is there an existing diagnostic I should model this after, or is this a unique case?
> > Unfortunately, libomptarget does not have access to source code location. `__tgt_` API functions do not have a related argument, like the `ident_t *loc` argument of `__kmpc_` API functions in libomp. Of course, if there is demand for such a feature we can implement it, although we would need to change *EVERY* API function in libomptarget.
> > Unfortunately, libomptarget does not have access to source code location. `__tgt_` API functions do not have a related argument, like the `ident_t *loc` argument of `__kmpc_` API functions in libomp. Of course, if there is demand for such a feature we can implement it, although we would need to change *EVERY* API function in libomptarget.
> 
> That would be nice, but in a later patch.
> 
> For this patch, perhaps just a simple message clarifying that the error is due to the `present` modifier would be sufficient.  Should we define another macro next to `FATAL_MESSAGE` to print messages to stderr without exiting?  The exit happens later when `FATAL_MESSAGE` is actually called.
> 
That would be good, such a message is missing from other places in libomptarget as well (e.g. when we explicitly try to extend an existing mapping). Currently such messages are only printed in debug output. It's important for the user to know why offload failed even if libomptarget is running in release mode.


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

https://reviews.llvm.org/D83062