[Openmp-commits] [PATCH] D100774: [OpenMP] Add function for setting LIBOMPTARGET_INFO at runtime

[Joseph Huber via Phabricator via Openmp-commits]

jhuber6 added a comment.

In D100774#2709722 <https://reviews.llvm.org/D100774#2709722>, @JonChesterfield wrote:

> In D100774#2709610 <https://reviews.llvm.org/D100774#2709610>, @jhuber6 wrote:
>
>> The problem I had was that in `libomptarget` there wasn't a single instance of `#include "Debug.h"`. This meant that there were about three different versions of `InfoLevel`. This meant if the variable was static it wouldn't set everyone's version of `InfoLevel` when called from the function. It might've been a better solution to just require that `Debug.h` is included only once per library, then use a method like you described. It would certainly be nicer to not require setting the extern variables for each plugin, which is what I originally removed.
>
> I think that was collateral damage from getInfoLevel being static. Changing that to just inline will mean a single copy per shared library, which seems to be the right semantics. Probably still want the local variable to be atomic qualified though.

Seems reasonable. I'd probably have another function that returns a reference to the internal atomic variable, then `getInfoLevel()` just returns the value inside the atomic. Should this be a another review? Or a revert + fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100774