View Issue Details [ Jump to Notes ] | [ Print ] | ||||||||
ID | Project | Category | View Status | Date Submitted | Last Update | ||||
0015045 | CMake | Modules | public | 2014-07-29 07:42 | 2016-06-10 14:31 | ||||
Reporter | Nils Gladitz | ||||||||
Assigned To | James Bigler | ||||||||
Priority | normal | Severity | minor | Reproducibility | always | ||||
Status | closed | Resolution | moved | ||||||
Platform | OS | OS Version | |||||||
Product Version | CMake 3.0 | ||||||||
Target Version | Fixed in Version | ||||||||
Summary | 0015045: FindCUDA.cmake passes directory COMPILE_DEFINITIONS to nvcc without handling generator expressions | ||||||||
Description | A colleague noticed this while using UseQt4.cmake which since 3.0 contains: set_property(DIRECTORY APPEND PROPERTY COMPILE_DEFINITIONS $<$<NOT:$<CONFIG:Debug>>:QT_NO_DEBUG>) To recreate the old behaviour generator expressions could be stripped with the new string(GENEX_STRIP) or perhaps even expanded with file(GENERATE). | ||||||||
Steps To Reproduce | cmake_minimum_required(VERSION 3.0) set_property(DIRECTORY APPEND PROPERTY COMPILE_DEFINITIONS $<$<NOT:$<CONFIG:Foo>>:NOT_FOO> ) find_package(CUDA REQUIRED) cuda_add_library(empty empty.cu) | ||||||||
Tags | No tags attached. | ||||||||
Attached Files | ![]() | ||||||||
Relationships | |
Relationships |
Notes | |
(0040577) Benoît Bleuzé (reporter) 2016-02-29 04:57 edited on: 2016-02-29 09:31 |
Hi Nils, I am struggling with the same problem. I have implemented what I deem an ugly solution, that works for me, but I would like the opinion of the cmake developers on the proper way to solve the issue. Here is my temporary solution (attached are all the files needed to get FindCuda working). Let me explain the problem, and the fix. The original FindCUDA script runs configure() on a cmake script that just copies a list of definitions and flags. To do that it gets the list of definitions from the directory property, and adds -D to each of them, and then adds them to a single variable ${nvcc_flags}. This cmake script is then executed by a custom command at build time. The problem, as you described, is that definitions can contain generator expressions, and those are just copied as-is to that configured cmake script. My fix does what you suggested: after the configure step I run file(GENERATE) and create configuration specific cmake scripts. the custom_command then runs the configuration specific cmake script using in turn a generator expression for the configuration. However, this is not sufficient, as the ${nvcc_flags} variable copies definitions after having put a "-D" in front of each of them. If the generator expression results in an empty string, let's say -totally randomly-: $<$<NOT:$<CONFIG:Debug>>:QT_NO_DEBUG>, then the generated file ends up having things like "-D -D SOME_VAR", and the compilation spews warnings all over the place about no identifiers for definitions. To fix this, I also patched the script "run_nvcc.cmake", and here the hack becomes plain ugly: set(nvcc_flags_tmp @nvcc_flags@) # list foreach(flag IN LISTS nvccflags_tmp) if(NOT "${flag}" STREQ "-D" ) list(APPEND nvcc_flags ${flag}) endif() endforeach() I fix the list of flags in the generated file at build time. This works fine, but is not really easy to maintain or easy to read. Is there anyone who worked on this, or who is ready to suggest other ways to fix it? I have one or two ideas, such as separating the list of definition to the rest of the nvcc_flags as long as possible and only add the "-D" in the run_nvcc.cmake script, which would make things a bit more readable, but maybe there are other places where generator expressions could lurk, and here is where I need the most your input. Ben. |
(0040595) Giovanni Funchal (reporter) 2016-03-04 05:04 |
I wanted to add that I've been hitting similar issues with FindCUDA for a while, and I think Benoit has taken a huge step forward with the patch he proposed here (thanks Benoit!). For example, I have a header-only library that declares its usage requirements using the following: ADD_LIBRARY(mylibrary INTERFACE IMPORTED GLOBAL) SET_TARGET_PROPERTIES(mylibrary PROPERTIES INTERFACE_INCLUDE_DIRECTORIES "path/to/includes") I was then trying to use requirement transitive propagation to include that library in a cuda executable, this: FIND_PACKAGE(CUDA REQUIRED) CUDA_ADD_EXECUTABLE(myexecutable ${sources}) SET_TARGET_PROPERTIES(myexecutable PROPERTIES LINK_LIBRARIES mylibrary) .. doesn't work because FindCUDA prepares a custom command when CUDA_ADD_EXECUTABLE is called and therefore the SET_TARGET_PROPERTIES has no effect. So instead I tried doing: FIND_PACKAGE(CUDA REQUIRED) INCLUDE_DIRECTORIES($<TARGET_PROPERTY:mylibrary,INTERFACE_INCLUDE_DIRECTORIES>) CUDA_ADD_EXECUTABLE(myexecutable ${sources}) Now that sets the flag before calling CUDA_ADD_EXECUTABLE so it should be fine, but it still doesn't work as intended -- the generator expression is passed down uninterpreted. This was a bit unexpected to me since both INCLUDE_DIRECTORIES and ADD_CUSTOM_COMMAND are documented as supporting generator expressions. I found that Benoit's solution attached to this issue fixes that too (as well as COMPILE_DEFINITIONS), and it works like a charm to me. The generator expression in INCLUDE_DIRECTORIES is expanded and works correctly. I think this also fixes https://cmake.org/Bug/view.php?id=14201 [^] Is there anything I can do to help get this fix reviewed and merged? |
(0040661) Benoît Bleuzé (reporter) 2016-03-11 08:03 |
Hi Giovanni, The directory property INCLUDE_DIRECTORIES is passed to run_nvcc.cmake at configuration time through a variable called ${CUDA_NVCC_INCLUDE_ARGS}. I didn't think about the target property INCLUDE_DIRECTORIES or the directory one, but you are experiencing a happy side effect of calling file(GENERATE). Every bit of text in the ${custom_target_script} that looks like a generator expression will be replaced then. Similar things are done in the Qt4Macros.cmake, but there the script also works for the target include properties(see https://cmake.org/gitweb?p=cmake.git;a=blob;f=Modules/Qt4Macros.cmake;h=7d7f5aa411f9be3761de6ffd6ec6de2d2a77a417;hb=9ce60ff5 [^]): if(moc_target) list(APPEND targetincludes "$<TARGET_PROPERTY:${moc_target},INCLUDE_DIRECTORIES>") list(APPEND targetdefines "$<TARGET_PROPERTY:${moc_target},COMPILE_DEFINITIONS>") set(targetincludes "$<$<BOOL:${targetincludes}>:-I$<JOIN:${targetincludes},\n-I>\n>") set(targetdefines "$<$<BOOL:${targetdefines}>:-D$<JOIN:${targetdefines},\n-D>\n>") file (GENERATE OUTPUT ${_moc_parameters_file} CONTENT "${targetdefines}${targetincludes}${targetoptions}${_moc_parameters}\n" CONDITION 1 ) set(targetincludes) set(targetdefines) else() file(WRITE ${_moc_parameters_file} "${_moc_parameters}\n") endif() set(_moc_extra_parameters_file @${_moc_parameters_file}) add_custom_command(OUTPUT ${outfile} COMMAND ${QT_MOC_EXECUTABLE} ${_moc_extra_parameters_file} DEPENDS ${infile} ${_moc_working_dir} VERBATIM) I guess here something similar should be done, parsing the definitions to a variable before putting them in the generated file, and parsing target and directory include properties at that same time, to a variable, and then calling file(GENERATE). Problem: are there other properties to take care of we have forgotten? I see : * INCLUDE_DIRECTORIES * COMPILE_DEFINITIONS * COMPILE_FLAGS * COMPILE_OPTIONS * ??? Anything else? |
(0040663) Giovanni Funchal (reporter) 2016-03-11 09:08 |
I understand the "happy side effect" part, I was just noting that your change fixed what I consider to be yet another bug, ie the INCLUDE_DIRECTORIES *command* (not target property) is documented to support generator expressions and FindCUDA silently broke that too. In terms of properties list, I can't think of anything else. Those four are definitely the most important. I would say you've nailed down the approach already and we can add more properties if/when required. |
(0042595) Kitware Robot (administrator) 2016-06-10 14:29 |
Resolving issue as `moved`. This issue tracker is no longer used. Further discussion of this issue may take place in the current CMake Issues page linked in the banner at the top of this page. |
Notes |
Issue History | |||
Date Modified | Username | Field | Change |
2014-07-29 07:42 | Nils Gladitz | New Issue | |
2014-07-29 07:42 | Nils Gladitz | Status | new => assigned |
2014-07-29 07:42 | Nils Gladitz | Assigned To | => James Bigler |
2016-02-29 04:57 | Benoît Bleuzé | Note Added: 0040577 | |
2016-02-29 04:57 | Benoît Bleuzé | File Added: FindCUDA.tar.gz | |
2016-02-29 09:31 | Benoît Bleuzé | Note Edited: 0040577 | |
2016-03-04 05:04 | Giovanni Funchal | Note Added: 0040595 | |
2016-03-11 08:03 | Benoît Bleuzé | Note Added: 0040661 | |
2016-03-11 09:08 | Giovanni Funchal | Note Added: 0040663 | |
2016-06-10 14:29 | Kitware Robot | Note Added: 0042595 | |
2016-06-10 14:29 | Kitware Robot | Status | assigned => resolved |
2016-06-10 14:29 | Kitware Robot | Resolution | open => moved |
2016-06-10 14:31 | Kitware Robot | Status | resolved => closed |
Issue History |
Copyright © 2000 - 2018 MantisBT Team |