[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()
----------------
jlpeyton wrote:
> 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.
Ok, I forgot that libomp_say() prepends the "LIBOMP:" string to the message.  You could use:
`message(STATUS "ITT Notify not supported for static libraries - forcing ITT Notify off")` so now it wouldn't look out of place.


Repository:
  rL LLVM

https://reviews.llvm.org/D31466