Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Added several questions inline
openmp/CMakeLists.txt | ||
---|---|---|
100 | What does the 140 mean? Is this supposed to be the llvm release number? | |
openmp/runtime/CMakeLists.txt | ||
359 | Similar as above, what is 140? Why not make destinction Debug/non-Debug here? | |
openmp/runtime/src/CMakeLists.txt | ||
162 | CMAKE_BUILD_TYPE is case sensitive. Why make it case-insensitive here? | |
242–253 | The IMP does not need the 140? |
openmp/runtime/CMakeLists.txt | ||
---|---|---|
359 | The version number corresponds to the toolset the dll is built for. The last time the Visual C++ toolset broke binary compatibility was Visual Studio 14, so the version number is 14.0. | |
openmp/runtime/src/CMakeLists.txt | ||
162 | Every other use of CMAKE_BUILD_TYPE I found were case insensitive as well. | |
242–253 | Only dlls need the 140. The version # is part of the dll name to allow different versions of the same dll to exist on the machine side-by-side without interfering with each other at runtime/load time. Library files are only used at build time, so they don't need the version number. |
openmp/runtime/src/CMakeLists.txt | ||
---|---|---|
162 | @protze.joachim @jlpeyton , I'm still confused by the comments about DEBUG_BUILD. As far as I can tell, a case-insensitive version of CMAKE_BUILD_TYPE (sometimes upper case, sometimes lower case) is the method used to determine build type in every CMakeLists.txt. DEBUG_BUILD is set in a CMakeLists.txt file, but it is only used for header files and setting compiler flags. Is there a plan to switch to using DEBUG_BUILD in CMakeLists.txt files instead of checking CMAKE_BUILD_TYPE at some point in the future? |
openmp/runtime/CMakeLists.txt | ||
---|---|---|
359 | As for last question (why not make debug/non-debug distinction here), the dlls are named libomp140(d).dll whereas the libraries are named libomp(d).dll in the MSVC naming scheme. Making the name decision closer to where the name is actually used makes which name is used where clearer and helped ensure I didn't miss a name. |
openmp/runtime/src/CMakeLists.txt | ||
---|---|---|
162 | I've created https://reviews.llvm.org/D112951 to alleviate any concerns I would have with your approach for CMAKE_BUILD_TYPE. There are two ways to build libomp
The standalone build doesn't know what uppercase_CMAKE_BUILD_TYPE is since it is defined in llvm/CMakeLists.txt. D112951 will add uppercase_CMAKE_BUILD_TYPE for the standalone build so there won't be any issue with your approach. |
I only have one other suggestion. I think its fine to use uppercase_CMAKE_BUILD_TYPE since its defined everywhere. The DEBUG_BUILD (and friends) are useful as booleans for append_if() type macro/functions in CMake.
openmp/CMakeLists.txt | ||
---|---|---|
100 | All the user-definable CMake variables in this CMakeLists.txt should be prefixed with OPENMP_, so I think the variable should be OPENMP_MSVC_NAME_SCHEME. |
My apologies, I don't have commit access. Could someone commit on my behalf? Thank you in advance.
What does the 140 mean? Is this supposed to be the llvm release number?
I suggest to use a more descriptive name for this Cmake variable and not to encode a version number into the variable name.