Hello, I work on CMake upstream. I'd like to find out in what ways CMake upstream does not fit the needs of llvm/clang, and then fill those gaps. The recent update of the minimum version to CMake 2.8.8 is a good start. Before being able to assess what is missing, it should be ensured that the current codebase is as modern as the minimum version allows, and it needs to be cleaned up. 1) You need to set the minimum required CMake version before the project() command. The minimum version affects behavior by setting policies, and can affect the behavior of the project() command. http://www.cmake.org/cmake/help/git-master/manual/cmake-policies.7.html Eg http://www.cmake.org/cmake/help/git-master/policy/CMP0025.html -project(LLVM) cmake_minimum_required(VERSION 2.8.8) +project(LLVM) You need to do the same in clang/CMakeLists.txt. 2) You need to clean up tabs vs spaces. At least clang/CMakeLists.txt mixes them. I didn't look elsewhere but you should clean it all up. 3) In modern cmake code, the end*() commands have no content. See eg http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=9db31162 You can run a similar command on the llvm cmake files. At least clang CMakeLists contains endif(MSVC_IDE OR XCODE) I didn't check others. 4) There seems to be a 'cmake package config file' generated by the llvm build. http://www.cmake.org/cmake/help/git-master/manual/cmake-packages.7.html#package-configuration-file (the doc is new in master, but it applies almost entirely to CMake 2.8.8 too) but it doesn't seem to appear in my llvm-3.5-dev ubuntu package. You need to ensure that the package config file is generated by your buildsystem. If you support two buildsystems, you need to ensure that it is generated properly by both of them. As far as I can see, the Makefile buildsystem in llvm does not generate the files. This is a significant bug. It is equivalent to not generating a required header file in one of your supported buildsystems. You need to ensure that the config file is generated by both buildsystems. That can mean that you avoid the built-in CMake facilities for creating packages. [Sidebar: This is part of the reason why I strongly recommend any project to have only one buildsystem. If it is impossible for you to drop the Makefile system, then consider dropping the CMake one. It creates false expectations of ability to use packages downstream.] 5) When I see things like this: add_dependencies(clangStaticAnalyzerCheckers ClangAttrClasses ClangAttrList ClangCommentNodes and this: set(LLVM_LINK_COMPONENTS Support ) I don't know what those lines are for, but it looks like 'you're doing it wrong' from a dependency specification point of view, or CMake is not giving you the interfaces to do it right. If it's the latter, I want to fix that. 6) If you create a proper config file, then you can populate it with IMPORTED targets and use it in clang. IMPORTED targets record dependencies and usage requirements (when you start requiring CMake 2.8.9+ at least) properly. http://www.cmake.org/cmake/help/git-master/manual/cmake-buildsystem.7.html#imported-targets The main oddness seems to come from the fact that the Makefile buildsystem doesn't create the cmake package config files. Fix that first or remove the Makefile buildsystem. I recall there was a discussion about that a few years ago. Thanks, Steve.
On Sun, Feb 2, 2014 at 8:09 AM, Stephen Kelly <steveire at gmail.com> wrote:> > Hello, > > I work on CMake upstream. I'd like to find out in what ways CMake upstream > does not fit the needs of llvm/clang, and then fill those gaps. The recent > update of the minimum version to CMake 2.8.8 is a good start. Before being > able to assess what is missing, it should be ensured that the current > codebase is as modern as the minimum version allows, and it needs to be > cleaned up.Hi Stephen, There is an ongoing discussion about compiler-rt where it was mentioned that with CMake it is hard to use the just-built compiler to build the runtime libraries: http://permalink.gmane.org/gmane.comp.compilers.llvm.devel/69951 Dmitri -- main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if (j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr at gmail.com>*/
On 02/02/2014 03:02 PM, Dmitri Gribenko wrote:> On Sun, Feb 2, 2014 at 8:09 AM, Stephen Kelly <steveire at gmail.com> wrote: >> Hello, >> >> I work on CMake upstream. I'd like to find out in what ways CMake upstream >> does not fit the needs of llvm/clang, and then fill those gaps. The recent >> update of the minimum version to CMake 2.8.8 is a good start. Before being >> able to assess what is missing, it should be ensured that the current >> codebase is as modern as the minimum version allows, and it needs to be >> cleaned up. > Hi Stephen, > > There is an ongoing discussion about compiler-rt where it was > mentioned that with CMake it is hard to use the just-built compiler to > build the runtime libraries: > > http://permalink.gmane.org/gmane.comp.compilers.llvm.devel/69951I added a link to http://public.kitware.com/Bug/view.php?id=14539 implementing that would be the CMake answer to: Alexey Samsonov wrote:> But if we want more functionality,> like running the tests, we ought to have a single build system, single set> of "targets" (binaries, libraries, test suites) with dependencies between> them.What the Makefile system already does is already possible using CMake ExternalProject. Alexey Samsonov wrote:> Well, LLVM/Clang's configure+make and compiler-rt's make are disjoint -> when you run "make" in Clang build tree, at one point it simply invokes,> the Makefile in compiler-rt directory.However, I wrote that I recommend cleaning up the cmake files first, which is why I listed several cleanups. Let's focus on that first. Thanks, Steve. -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20140202/1134b0e8/attachment.html>
Stephen,> 1) You need to set the minimum required CMake version before the project() > command.Fixed in r200645.> 2) You need to clean up tabs vs spaces. At least clang/CMakeLists.txt mixes > them. I didn't look elsewhere but you should clean it all up.Fixed in r200643 and r200644.> 3) In modern cmake code, the end*() commands have no content. See eg > > http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=9db31162In historical reason, IMO, I'd not like to substitute all of them. We will prune such legacy phrases whenever we touch around them.> 4) There seems to be a 'cmake package config file' generated by the llvm > build. > > http://www.cmake.org/cmake/help/git-master/manual/cmake-packages.7.html#package-configuration-file > > (the doc is new in master, but it applies almost entirely to CMake 2.8.8 > too) > > but it doesn't seem to appear in my llvm-3.5-dev ubuntu package. > > You need to ensure that the package config file is generated by your > buildsystem. If you support two buildsystems, you need to ensure that it is > generated properly by both of them. As far as I can see, the Makefile > buildsystem in llvm does not generate the files. This is a significant bug. > It is equivalent to not generating a required header file in one of your > supported buildsystems. > > You need to ensure that the config file is generated by both buildsystems. > That can mean that you avoid the built-in CMake facilities for creating > packages. > > [Sidebar: This is part of the reason why I strongly recommend any project to > have only one buildsystem. If it is impossible for you to drop the Makefile > system, then consider dropping the CMake one. It creates false expectations > of ability to use packages downstream.]Brad King is proposing the stuff. Excuse me we haven't accepted his yet. See also http://llvm.org/bugs/show_bug.cgi?id=15732> 5) When I see things like this: > > add_dependencies(clangStaticAnalyzerCheckers > ClangAttrClasses > ClangAttrList > ClangCommentNodesThey give dependencies for generated headers. What'd be wrong to you? I have w-i-p patchset to nuke them (to go more suboptimal)> and this: > > set(LLVM_LINK_COMPONENTS > Support > )LLVM_LINK_COMPONENTS is for compatibility of autoconf build. In AddLLVM.cmake and LLVM-Config.cmake, it is translated to target_link_libraries.> I don't know what those lines are for, but it looks like 'you're doing it > wrong' from a dependency specification point of view, or CMake is not giving > you the interfaces to do it right. If it's the latter, I want to fix that.You may suppose the former is coming from historical reason. Please be patient. The latter, I am certain CMake is insufficient. I know it'd be not easy to describe optimal behavior for generated headers. FYI, ClangXXX is doing; 1. Generate clang-tblgen. 2. Generate foo.inc from foobar.td. 3. Build object files with generated foo.inc. Do you think we should give suboptimal dependency, "Build all files whenever any of *.td were touchd"?> 6) If you create a proper config file, then you can populate it with > IMPORTED targets and use it in clang. IMPORTED targets record dependencies > and usage requirements (when you start requiring CMake 2.8.9+ at least) > properly. > > http://www.cmake.org/cmake/help/git-master/manual/cmake-buildsystem.7.html#imported-targetsMost of us are building clang in llvm build tree. Aka "standalone clang" could be supposed as w-i-p. I think other projects could be good studies for external cmake build. I agree to update cmake version if LLVMConfig.cmake would work more smart. ;) ...Takumi
NAKAMURA Takumi wrote:> >> 4) There seems to be a 'cmake package config file' generated by the llvm >> build. >> >> http://www.cmake.org/cmake/help/git-master/manual/cmake-packages.7.html#package-configuration-file >> >> (the doc is new in master, but it applies almost entirely to CMake 2.8.8 >> too) >> >> but it doesn't seem to appear in my llvm-3.5-dev ubuntu package. >> >> You need to ensure that the package config file is generated by your >> buildsystem. If you support two buildsystems, you need to ensure that it >> is generated properly by both of them. As far as I can see, the Makefile >> buildsystem in llvm does not generate the files. This is a significant >> bug. It is equivalent to not generating a required header file in one of >> your supported buildsystems. >> >> You need to ensure that the config file is generated by both >> buildsystems. That can mean that you avoid the built-in CMake facilities >> for creating packages. >> >> [Sidebar: This is part of the reason why I strongly recommend any project >> [to >> have only one buildsystem. If it is impossible for you to drop the >> Makefile system, then consider dropping the CMake one. It creates false >> expectations of ability to use packages downstream.] > > Brad King is proposing the stuff. Excuse me we haven't accepted his yet.Oh, I didn't know Brad was also working on this stuff in llvm. Good. I haven't reviewed his patches, but if it's generating the same content with CMake and Makefiles, that sounds good.> See also http://llvm.org/bugs/show_bug.cgi?id=15732Some of the bugs are questionable or unclear. I've left some comments.> >> 5) When I see things like this: >> >> add_dependencies(clangStaticAnalyzerCheckers >> ClangAttrClasses >> ClangAttrList >> ClangCommentNodes > > They give dependencies for generated headers. What'd be wrong to you?Oh, ok. I didn't know what they were for, and as I've seen other code for manually managing dependencies, I thought this was more of the same.>> and this: >> >> set(LLVM_LINK_COMPONENTS >> Support >> ) > > LLVM_LINK_COMPONENTS is for compatibility of autoconf build. > In AddLLVM.cmake and LLVM-Config.cmake, it is translated to > target_link_libraries.What compatibility? A file is generated by a cmake llvm build for autoconf- using downstreams to use, or something?>> I don't know what those lines are for, but it looks like 'you're doing it >> wrong' from a dependency specification point of view, or CMake is not >> giving you the interfaces to do it right. If it's the latter, I want to >> fix that. > > You may suppose the former is coming from historical reason. Please be > patient.Sorry, I did not intend to appear impatient. I'm more looking for gaps in cmake than in the llvm buildsystem.> The latter, I am certain CMake is insufficient. > I know it'd be not easy to describe optimal behavior for generated > headers. > > FYI, ClangXXX is doing; > > 1. Generate clang-tblgen. > 2. Generate foo.inc from foobar.td. > 3. Build object files with generated foo.inc. > > Do you think we should give suboptimal dependency, > "Build all files whenever any of *.td were touchd"?Maybe this will be easier to work on after Brads patches are in.> >> 6) If you create a proper config file, then you can populate it with >> IMPORTED targets and use it in clang. IMPORTED targets record >> dependencies and usage requirements (when you start requiring CMake >> 2.8.9+ at least) properly. >> >> http://www.cmake.org/cmake/help/git-master/manual/cmake-buildsystem.7.html#imported-targets > > Most of us are building clang in llvm build tree. > Aka "standalone clang" could be supposed as w-i-p. > I think other projects could be good studies for external cmake build. > > I agree to update cmake version if LLVMConfig.cmake would work more smart. > ;)Actually, CMake 2.8.11 would be a better minimum to aim for. It handles transitive usage requirements, but earlier versions do not. I was wrong about 2.8.9. Anyway, from the description, Brads patches add IMPORTED targets, so let's have another look when they are in. Thanks, Steve.
On 2 Feb 2014, at 17:42, NAKAMURA Takumi <geek4civic at gmail.com> wrote:>> 6) If you create a proper config file, then you can populate it with >> IMPORTED targets and use it in clang. IMPORTED targets record dependencies >> and usage requirements (when you start requiring CMake 2.8.9+ at least) >> properly. >> >> http://www.cmake.org/cmake/help/git-master/manual/cmake-buildsystem.7.html#imported-targets > > Most of us are building clang in llvm build tree. > Aka "standalone clang" could be supposed as w-i-p. > I think other projects could be good studies for external cmake build.Just to add, the ability to build clang against an installed LLVM tree would make it significantly easier for the FreeBSD LLVM and Clang ports to switch to using CMake. This would make us very happy, as our build infrastructure automatically uses Ninja by default for CMake ports, so we'd get a nice speedup on package builds. David