[Openmp-commits] [PATCH] D93055: [OpenMP] Add time profiling for libomptarget

[Johannes Doerfert via Phabricator via Openmp-commits]

jdoerfert added a comment.

In D93055#2461668 <https://reviews.llvm.org/D93055#2461668>, @ayermolo wrote:

> Maybe add explicit flag NOT_LINK_SUPPORT or something like that for this case, or break up the ENABLE_LIBOMPTARGET/OPENMP_ENABLE_LIBOMPTARGET_PROFILING dependency?

Why add a new flag if you can disable profiling with the one added? I mean, `OPENMP_ENABLE_LIBOMPTARGET_PROFILING=OFF` should unbreak your build, correct?

In D93055#2461829 <https://reviews.llvm.org/D93055#2461829>, @JonChesterfield wrote:

> I'd prefer we default to profiling disabled (and the llvm libs not linked in) for now.
>
> (actually my preferred solution is to link libomptarget statically, but that feels like a longer term goal)

So far there was no convincing reason why it should be off. Having it on by default has the distinct advantage that most people get the feature and can use profiling with their local build or release of LLVM, that is arguably useful. If you start with some fancy build you might need to disable it, or upstream support of your fancy build, neither is unprecedented.

In D93055#2461842 <https://reviews.llvm.org/D93055#2461842>, @ayermolo wrote:

>> I'd prefer we default to profiling disabled (and the llvm libs not linked in) for now.
>>
>> (actually my preferred solution is to link libomptarget statically, but that feels like a longer term goal)
>
> I would prefer that also. :)

See above.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93055