[llvm-commits] [llvm] r114747 - in /llvm/trunk: CMakeLists.txt cmake/modules/AddLLVM.cmake unittests/CMakeLists.txt utils/unittest/CMakeLists.txt

[Michael Spencer]

On Fri, Sep 24, 2010 at 11:41 PM, Óscar Fuentes <ofv at wanadoo.es> wrote:
> The following message is a courtesy copy of an article
> that has been posted to gmane.comp.compilers.llvm.cvs as well.
>
> Michael Spencer <bigcheesegs-Re5JQEeQqe8AvxtiuMwx3w at public.gmane.org>
> writes:
>
>> The only way to build (that we could find) clang from the command line
>> using VCBuild is to build the entire solution, which includes the
>> tools, examples, and tests no matter what. LLVM_BUILD_X only controls
>> if they are part of the ALL_BUILD target, and building ALL_BUILD
>> doesn't build anything with VCBuild.
>
> cmake --build .

This works for me (VS 2010), but on the build bot (VS 2008) it
attempts to run VCExpress instead of VCBuild, and it fails. I don't
really know much else as Daniel has everything setup.

> on the build directory works fine here, omitting the examples as
> required by the setting of LLVM_BUILD_EXAMPLES.
>
> And is it really important to build with VCBuild? NMake works fine and
> seems faster than VCBuild. And JOM allows parallel builds, in case the
> bot is a multicore machine.

VCBuild allows parallel builds with the -M switch.

> But if you want to speed-up the VS build you could investigate why
> tblgen is so slow on Windows. The time it takes to process the files
> associated to the X86 target is quite larger than the required for
> building the examples.

tblgen seems fast to me. Where exactly are you seeing these slowdowns?

>>> And why you don't want to build examples and unittests on the bot, if
>>> precisely the purpose of the bot is to test the build? (*all* the build,
>>> I guess)
>>
>> This is a clang tester, it slows it down. And the unittests don't
>> build on msvc9 (they do in 10).
>
> This is a pity, having a bot that ignores part of the build. Apart from
> that, failed unittests are significant for clang, aren't they?

The unit tests are the LLVM unit tests that I just added CMake support
for. MSVC 2008 fails to build them for some reason and I don't use
2008 so I can't really fix them.

> [snip]
>
>>> Please explain in more detail what the problem is and maybe we can find
>>> a better implementation.
>>
>> There are two options per group. LLVM_BUILD_X and LLVM_INCLUDE_X.
>> LLVM_BUILD_X says that X should be built by ALL_BUILD, and
>> LLVM_INCLUDE_X says that X should be included as a target that is
>> available to be built. LLVM_BUILD_X must imply LLVM_INCLUDE_X, as you
>> cannot build a target that doesn't exist.
>>
>> The problem is that I could not find a way to change the cache value
>> of LLVM_INCLUDE_X displayed in ccache and cmake-gui to ON if
>> LLVM_BUILD_X is ON. This would make it clearer to the user and make
>> the code simpler.
>
> [snip]
>
>>> and is unnecessarily complex (why not
>>>
>>> if( LLVM_INCLUDE_X )
>>>  add_subdirectory(X)
>>> endif()
>>>
>>> instead of changing add_llvm_X)
>>
>> I agree. However, it should be:
>>
>> if( LLVM_INCLUDE_X OR LLVM_BUILD_X )
>>
>> Because of the problem above.
>
> What we could do is simply
>
> if( LLVM_INCLUDE_TOOLS )
>  add_subdirectory(tools)
> endif()
>
> and add to the documentation of LLVM_BUILD_TOOLS a note about it being
> conditioned by LLVM_INCLUDE_TOOLS (both on the definition of the
> variable and on the html file.) Likewise for EXAMPLES.

Did you take a look at my patch? LLVM_INCLUDE_X should be conditioned
by LLVM_BUILD_X.

> But all this may be irrelevant if the command "cmake --build ." works
> for you as it works for me. In that case simply revert the commit and
> you are done.

I really would prefer this, but apparently the build bot doesn't like it :(.

> [snip]
>
>>> Oh, and next time it would be a good idea to discuss the patch before
>>> committing it.
>>
>> It didn't (well, wasn't supposed to) have any functionality changes,
>> it's rather small, and it was needed to fix the build bot. Is this
>> really too much for review after commit?
>
> Well, you yourself were not satisfied with the change. That is a big
> hint that others may share your opinion ;-) so describing the problem
> and posting the patch for discussion is a good way of not creating
> surprises and even achieving a better solution.
>