[Openmp-commits] [PATCH] D86804: [OpenMP] Consolidate error handling and debug messages in Libomptarget

Ye Luo via Phabricator via Openmp-commits openmp-commits at lists.llvm.org
Mon Aug 31 15:05:25 PDT 2020

ye-luo added a comment.

In D86804#2248214 <https://reviews.llvm.org/D86804#2248214>, @jdoerfert wrote:

> In D86804#2248201 <https://reviews.llvm.org/D86804#2248201>, @ye-luo wrote:
>> In D86804#2247785 <https://reviews.llvm.org/D86804#2247785>, @jdoerfert wrote:
>>> In D86804#2247697 <https://reviews.llvm.org/D86804#2247697>, @ye-luo wrote:
>>>> It seems that functions are marked static so they should be OK. However, including the whole Debug.h in a plugin cpp makes it feel OK to use any function/macro from the header file. But actually only part of the macros are for the plugin. some  are only for the libomptarget.
>>> I'm not sure we want to make a distinction, the point is to move to a unified debug/message model. You can choose not only the level of information but also the kind of output (text, json, ...). The messages will then be tied to the webpage via enums, that allows all plugins to emit the same message for the same thing with the same link to more information. There will certainly things that are only used in libomptarget or the plugins, but I don't see how that is any worse than duplicating the parts that are used by both.
>> I didn't mean to duplicate anything. Instead you need multiple header files. One for common parts, one for libomptarget and one for plugins. The latter two both include the first one. Later you expand OFFLOAD_XXX signals, they can be added to the common file. The return signal is generated by the plugins and captured by the libomptarget. Some users may want to see only the messages captured universally by libomptarget. Some users still want to see the native error message. So the libomptarget and plugin side error handling still needs to be separated.
> I fail to see why this machinery is necessary to emit only messages from one place and not the other. I am not against a hierarchy of headers per se, but right now, and maybe also later, there seems to be little point.
> I mean, we need to introduce a new env variable, actually two, that allow separate control. Once we have that we can argue about separation. 
> Alternatively, I would have suggested to define the "location" prior to including the debug header, e.g.,:
>   #include "Debug.h"
> which we verify like:
>   #error ...
>   #endif
> At the end of the day I want to simplify things. A single location for all our debug needs sounds simpler than `1 + #plugins` to me, even if we don't use all functionality at each location. If separation does not allow anything we cannot reasonably do in a single location, I doubt it provides a benefit.

My need is only a clean separation. As long as each side doesn't see the macro used only for the other side, it doesn't matter to me using macros or separate header files.

  rG LLVM Github Monorepo



More information about the Openmp-commits mailing list