[Openmp-commits] [PATCH] D31466: [OpenMP] Fix for http://bugs.llvm.org/show_bug.cgi?id=32456

[Jonathan Peyton via Phabricator via Openmp-commits]

jlpeyton added inline comments.


================
Comment at: runtime/CMakeLists.txt:311
+if(NOT LIBOMP_ENABLE_SHARED)
+  set(LIBOMP_USE_ITT_NOTIFY FALSE)
+endif()
----------------
AndreyChurbanov wrote:
> jlpeyton wrote:
> > Can you add a message like:
> > libomp_say("ITT Notify not supported for static libraries - forcing ITT Notify off.")
> Is it worth checking if the build is standalone, as we usually keep silence otherwise?
> And if ITT notify was already turned off before this setting, we actually do not forcing it off, so no warning needed.
> 
> I'd make both checks, like
>   if(LIBOMP_USE_ITT_NOTIFY AND NOT LIBOMP_ENABLE_SHARED)
>     if(${LIBOMP_STANDALONE_BUILD})
>       libomp_say("ITT Notify not supported for static libraries - forcing ITT Notify off")
>     endif()
>     set(LIBOMP_USE_ITT_NOTIFY FALSE)
>   endif()
> 
> Is it better?
I don't think we need to check for the standalone build.  Only the block of messages in the `# Print configuration after all variables are set.` section are kept silent during the LLVM in-tree build.  Everything else still prints as normal.  The message won't look out of place in an LLVM in-tree build.


Repository:
  rL LLVM

https://reviews.llvm.org/D31466