Fixing LLVM's CMake interface before LLVM3.5 release

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

Fixing LLVM's CMake interface before LLVM3.5 release

Dan Liew
Hi All,

I've been playing [1] with the newly introduced CMake interface for
using exported LLVM CMake targets (e.g. the LLVMSupport library) in
CMake projects and although it works there are a few things I think we
should fix before the LLVM 3.5 release.

Here are the current issues I see that I'd like to discuss.

Just to clarify by "Targets" I mean targets in the CMake sense (e.g..
an executable or a library)

1 - We currently have both the old way (i.e.
llvm_map_components_to_libraries() as in [2] ) and the new way of
using built LLVM targets in CMake projects in trunk. I think we should
at some point remove the llvm_map_components_to_libraries() stuff
because it's messy. I'm not convinced it should be done in the LLVM
3.5 release because it would probably be nicer to announce for LLVM
3.5 that the old way is deprecated and remove it in LLVM 3.6 to give
clients more time to cope with the change. Either way we need to
decide on a plan for this. Thoughts?

2. The new way of using built LLVM targets in CMake projects is not
documented. If it's not documented people will probably not use it. I
am happy to start writing a patch for the documentation but I'd like
to know our position on the old way is (i.e. are we deprecating or
removing it for LLVM 3.5?) so the documentation can be rewritten
accordingly.

3. The LLVMConfig.cmake file provides something like this...

```
# LLVM_BUILD_* values available only from LLVM build tree.
set(LLVM_BUILD_BINARY_DIR "/home/dan/dev/llvm/bin")
set(LLVM_BUILD_ENABLE_ASSERTIONS "ON")
set(LLVM_BUILD_LIBRARY_DIR "/home/dan/dev/llvm/bin/./lib")
set(LLVM_BUILD_MAIN_INCLUDE_DIR "/home/dan/dev/llvm/src/include")
set(LLVM_BUILD_MAIN_SRC_DIR "/home/dan/dev/llvm/src")
set(LLVM_BUILD_TOOLS_BINARY_DIR "/home/dan/dev/llvm/bin/./bin")
```

The comment notes that these LLVM_BUILD_* variables are only available
from the build tree (i.e. they won't be available in an installed
version of LLVM).

This seems like a very odd design decision. The build system would
know where everything is being installed so really all the
LLVM_BUILD_* variables are also known at install time and a user
probably shouldn't have to care if they building against LLVM in the
build tree or against an installed version, it should "just work".

I can imagine a legitimate use of LLVM_BUILD_TOOLS_BINARY might be if
your project needed to use some LLVM tools (.e.g opt, llvm-as) as part
of its compilation process, perhaps to build a supporting runtime for
use with a JIT for example. Again I as a developer should not need to
care if my project builds against the LLVM build tree or an installed
LLVM.

In [1] I hacked around this by reading the ``LOCATION`` property of
one of the imported targets. Unfortunately CMake 3.0 really doesn't
like this I get warnings like

```
  Policy CMP0026 is not set: Disallow use of the LOCATION target property.
  Run "cmake --help-policy CMP0026" for policy details.  Use the cmake_policy
  command to set the policy and suppress this warning.
```

The CMake docs seem to say that the reason for this is the LOCATION
might not be completely known at configure time. In the case of
imported targets I would disagree because the LOCATION is most
definitely known. So my hack probably isn't the best way to fix this.

@Brad - I would be interested to know your thoughts on this because
you obviously know a lot more about CMake than I do.

4. Why don't we expose any of the CXXFLAGS in the CMake interface?

To use LLVM libraries as a client at minimum ``-std=c++11 -f-no-rtti``
are needed. This information is shown by the ``llvm-config
--cxxflags`` command but why aren't we exposing this information in
LLVMConfig.cmake so clients can make use of it. For example in [1] I
had to specify these by hand.

[1] https://github.com/delcypher/srg-llvm-pass-tutorial
[2] http://llvm.org/docs/CMake.html#embedding-llvm-in-your-project

Thanks,
Dan Liew.
_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: Fixing LLVM's CMake interface before LLVM3.5 release

Brad King
On 07/16/2014 07:48 PM, Dan Liew wrote:
> I've been playing [1] with the newly introduced CMake interface for
> using exported LLVM CMake targets (e.g. the LLVMSupport library) in
> CMake projects

Thanks for trying it out.

> at some point remove the llvm_map_components_to_libraries() stuff

I agree it should be removed eventually but I'll defer to others on
when and how.

> I am happy to start writing a patch for the documentation

Thanks.  Please Cc me for review.

> # LLVM_BUILD_* values available only from LLVM build tree.

Those were created to simplify building Clang locally against a
LLVM build tree.  Clang needs the LLVM source and build trees too,
so this gives it that information.  No information is missing from
the install-tree interface about what is actually installed, AFAIK.

> In [1] I hacked around this by reading the ``LOCATION`` property of
> one of the imported targets. Unfortunately CMake 3.0 really doesn't
> like this I get warnings

As suggested in the CMP0026 documentation:

 http://www.cmake.org/cmake/help/v3.0/policy/CMP0026.html

you can use the $<TARGET_FILE:SomeTool> generator expression to
get the location in a well-defined manner.  It works in
add_custom_command so that should be sufficient for your example
use case.

> 4. Why don't we expose any of the CXXFLAGS in the CMake interface?
>
> To use LLVM libraries as a client at minimum ``-std=c++11 -f-no-rtti``
> are needed. This information is shown by the ``llvm-config
> --cxxflags`` command but why aren't we exposing this information in
> LLVMConfig.cmake so clients can make use of it.

That was an oversight.  If you want to add them, they could go in
a variable named something like "LLVM_REQUIRED_CXX_FLAGS".  Note
that this content will need to be populated by both build systems:

 cmake/modules/CMakeLists.txt
 cmake/modules/Makefile

Thanks,
-Brad

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: Fixing LLVM's CMake interface before LLVM3.5 release

Óscar Fuentes
In reply to this post by Dan Liew
Hello. Original author of the LLVM CMake build system here.

Dan Liew <[hidden email]> writes:

> I've been playing [1] with the newly introduced CMake interface for
> using exported LLVM CMake targets (e.g. the LLVMSupport library) in
> CMake projects and although it works there are a few things I think we
> should fix before the LLVM 3.5 release.
>
> Here are the current issues I see that I'd like to discuss.
>
> Just to clarify by "Targets" I mean targets in the CMake sense (e.g..
> an executable or a library)
>
> 1 - We currently have both the old way (i.e.
> llvm_map_components_to_libraries() as in [2] ) and the new way of
> using built LLVM targets in CMake projects in trunk. I think we should
> at some point remove the llvm_map_components_to_libraries() stuff
> because it's messy. I'm not convinced it should be done in the LLVM
> 3.5 release because it would probably be nicer to announce for LLVM
> 3.5 that the old way is deprecated and remove it in LLVM 3.6 to give
> clients more time to cope with the change. Either way we need to
> decide on a plan for this. Thoughts?

llvm_map_components_to_libraries et al. was created because I opted for
a method that allows to "export" complex heuristics to the installed
build and that works on the unistalled build too. That means that I
leaned towars implementing our own logic for linking LLVM's into the
client code rather than relying on what CMake alone provided at the
time. This was for the sake of being more flexible and future-proof.

If what CMake provides is good enough for current and future LLVM usage
requirements, I guess that llvm_map_components_to_libraries can be
removed. (Please note that I'm not the current maintainer, so this is
just personal opinion.)

[snip]

I'll not comment on the rest of your message and leave that to the
maintainers, except for a nitpick:

> 4. Why don't we expose any of the CXXFLAGS in the CMake interface?
>
> To use LLVM libraries as a client at minimum ``-std=c++11 -f-no-rtti``
> are needed.

-fno-rtti is not really *required* for using LLVM. Some LLVM parts
require it (mostly those that are used by deriving an LLVM class.) For
instance, I have a JIT engine implemented across multiple files and only
one of those (amounting to less than 100 lines) require -fno-rtti.

> This information is shown by the ``llvm-config --cxxflags``

That information shows (some of) the global flags used for building
LLVM. Applying those flags on your client code would "guarantee" that
you will be free from some minor annoyances, but it is nor really
required to everywhere apply all those flags for using LLVM, as
mentioned above.

[snip]

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: Fixing LLVM's CMake interface before LLVM3.5 release

Stephen Kelly
In reply to this post by Dan Liew
Dan Liew wrote:

> In [1] I hacked around this by reading the ``LOCATION`` property of
> one of the imported targets. Unfortunately CMake 3.0 really doesn't
> like this I get warnings like
>
> ```
>   Policy CMP0026 is not set: Disallow use of the LOCATION target property.
>   Run "cmake --help-policy CMP0026" for policy details.  Use the
>   cmake_policy command to set the policy and suppress this warning.
> ```
>
> The CMake docs seem to say that the reason for this is the LOCATION
> might not be completely known at configure time. In the case of
> imported targets I would disagree because the LOCATION is most
> definitely known.

Yes, for IMPORTED targets, this should work and is unit tested:

 http://cmake.org/gitweb?p=cmake.git;a=blob;f=Tests/RunCMake/CMP0026/CMP0026-IMPORTED.cmake;h=650c8a5c;hb=HEAD

If you can provide an sscce, I would be interested.

Thanks,

Steve.


_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: Fixing LLVM's CMake interface before LLVM3.5 release

Dan Liew
In reply to this post by Brad King
>> I am happy to start writing a patch for the documentation
>
> Thanks.  Please Cc me for review.

Will do.

>> # LLVM_BUILD_* values available only from LLVM build tree.
>
> Those were created to simplify building Clang locally against a
> LLVM build tree.  Clang needs the LLVM source and build trees too,
> so this gives it that information.  No information is missing from
> the install-tree interface about what is actually installed, AFAIK.
>
>> In [1] I hacked around this by reading the ``LOCATION`` property of
>> one of the imported targets. Unfortunately CMake 3.0 really doesn't
>> like this I get warnings
>
> As suggested in the CMP0026 documentation:
>
>  http://www.cmake.org/cmake/help/v3.0/policy/CMP0026.html
>
> you can use the $<TARGET_FILE:SomeTool> generator expression to
> get the location in a well-defined manner.  It works in
> add_custom_command so that should be sufficient for your example
> use case.

I've taken another look at this. For the simple example project [1]
I'm effectively doing this.

```
add_library(ReplaceGetGlobalID MODULE ReplaceGetGlobalID.cpp)

get_property(MODULE_FILE TARGET ReplaceGetGlobalID PROPERTY LOCATION)
configure_file(run.sh.in run.sh @ONLY)
```

where run.sh is a simple shell script to demonstrate how to invoke the
built library as a plug-in to the opt tool. With this CMP0026 policy
the above produces warnings.

After reading ``cmake --help-policy CMP0026`` I tried doing this instead...

```
add_library(ReplaceGetGlobalID MODULE ReplaceGetGlobalID.cpp)

file(GENERATE OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/run.sh INPUT
${CMAKE_CURRENT_SOURCE_DIR}/run.sh.in)
```

where run.sh.in looks like...

```
#!/bin/bash

# THIS IS A HACK - the clang target is missing, just assume its in the
same directory as the opt tool
$<TARGET_FILE_DIR:opt>/clang -emit-llvm -c simple_prog.c

# Load our "Hello pass" as a plug-in to opt and run it on the LLVM
bitcode for simple_prog
$<TARGET_FILE:opt> - load $<TARGET_FILE:ReplaceGetGlobalID> -hello
-stats simple_prog.bc > /dev/null
```

This exposes a few issues

* run.sh is supposed to have executable permissions. Previously the
``configure_file`` command just copied the permissions on
``run.sh.in``. The ``file(GENERATE ...)`` command does not do this. Is
there a way to fix this?
* The clang target is not being exported (I built clang in the usually
way by placing its source inside /path/to/llvm/source/tools/clang)

Right now it seems much easier to use ``cmake_policy(SET CMP0026
OLD)``. If I use this can I rely on the old behaviour being available
forever?

>> 4. Why don't we expose any of the CXXFLAGS in the CMake interface?
>>
>> To use LLVM libraries as a client at minimum ``-std=c++11 -f-no-rtti``
>> are needed. This information is shown by the ``llvm-config
>> --cxxflags`` command but why aren't we exposing this information in
>> LLVMConfig.cmake so clients can make use of it.
>
> That was an oversight.  If you want to add them, they could go in
> a variable named something like "LLVM_REQUIRED_CXX_FLAGS".  Note
> that this content will need to be populated by both build systems:
>
>  cmake/modules/CMakeLists.txt
>  cmake/modules/Makefile

Oh the joy of multiple build systems :)

Given Óscar's comments that these flags aren't always necessary
perhaps we should still expose them to clients but name it as
LLVM_USED_CXX_FLAGS , we could also export the C flags (i.e.
llvm-config --cflags) as LLVM_USED_C_FLAGS

Also should we also expose whether or not LLVM was built with
assertions? It's exposed as LLVM_BUILD_ENABLE_ASSERTIONS in the build
directory but isn't available in the installed version of LLVM. I see
no reason to hide this information. This is potentially useful
information as well because it tells the client if NDEBUG was defined
when LLVM was built. Mixing LLVM built with NDEBUG defined but then
including LLVM header files in a project with NDEBUG not defined can
lead to trouble (e.g. trying to use facilities in llvm/Support/Debug.h
causes problems).

[1] https://github.com/delcypher/srg-llvm-pass-tutorial/tree/master/usingIRBuilder

Thanks,
Dan.

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: Fixing LLVM's CMake interface before LLVM3.5 release

Dan Liew
In reply to this post by Stephen Kelly
>> The CMake docs seem to say that the reason for this is the LOCATION
>> might not be completely known at configure time. In the case of
>> imported targets I would disagree because the LOCATION is most
>> definitely known.
>
> Yes, for IMPORTED targets, this should work and is unit tested:
>
>  http://cmake.org/gitweb?p=cmake.git;a=blob;f=Tests/RunCMake/CMP0026/CMP0026-IMPORTED.cmake;h=650c8a5c;hb=HEAD
>
> If you can provide an sscce, I would be interested.

Apologies. My project tried to read the LOCATION property of imported
targets and my own targets. I misread the warning messages (they were
only from me reading the property on my own targets). It seems that
reading the LOCATION property of imported targets does work without
warning so there isn't an issue here.
_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: Fixing LLVM's CMake interface before LLVM3.5 release

Brad King
In reply to this post by Dan Liew
On 07/18/2014 08:51 AM, Dan Liew wrote:
> * run.sh is supposed to have executable permissions. Previously the
> ``configure_file`` command just copied the permissions on
> ``run.sh.in``. The ``file(GENERATE ...)`` command does not do this. Is
> there a way to fix this?

The file(GENERATE) command needs to be taught options to set permissions.
I've made a note for that.

For now can't you just run the script through a shell:

 /usr/bin/env bash /path/to/run.sh $args

?

> * The clang target is not being exported (I built clang in the usually
> way by placing its source inside /path/to/llvm/source/tools/clang)

Actually I don't think any Clang targets are exported yet.  That
may or may not belong as a separate find_package(Clang).  Not sure.

> Right now it seems much easier to use ``cmake_policy(SET CMP0026
> OLD)``. If I use this can I rely on the old behaviour being available
> forever?

One day the OLD behaviors may be removed.  Never set policies to OLD
except as warning fixups on existing release maintenance branches.

Since "clang" isn't exported you have to pick another target to
get the LOCATION from anyway, which is the same hack as you're
doing in the $<TARGET_FILE_DIR:opt>/clang example.

One could export a LLVM_TOOLS_DIR value from both the build and
install trees to resolve this case the old fashioned way.

> Also should we also expose whether or not LLVM was built with
> assertions? It's exposed as LLVM_BUILD_ENABLE_ASSERTIONS in the build
> directory but isn't available in the installed version of LLVM.

We already have a few LLVM_ENABLE_<SOMETHING> values exported.
One could add LLVM_ENABLE_ASSERTIONS.

Thanks,
-Brad

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: Fixing LLVM's CMake interface before LLVM3.5 release

Dan Liew
On 18 July 2014 14:38, Brad King <[hidden email]> wrote:
> On 07/18/2014 08:51 AM, Dan Liew wrote:
>> * run.sh is supposed to have executable permissions. Previously the
>> ``configure_file`` command just copied the permissions on
>> ``run.sh.in``. The ``file(GENERATE ...)`` command does not do this. Is
>> there a way to fix this?
>
> The file(GENERATE) command needs to be taught options to set permissions.
> I've made a note for that.

Ok thanks.

> For now can't you just run the script through a shell:
>
>  /usr/bin/env bash /path/to/run.sh $args

I could. The small project I have is just a small tutorial though so I
prefer ease of use for anyone using it. So for now I'll just use
``cmake_policy(SET CMP0026 OLD)``, even though as you say it is the
wrong thing to do.


>> * The clang target is not being exported (I built clang in the usually
>> way by placing its source inside /path/to/llvm/source/tools/clang)
>
> Actually I don't think any Clang targets are exported yet.  That
> may or may not belong as a separate find_package(Clang).  Not sure.

I believe Clang has quite a few libraries of its own that are useful
so I think there should be a separate find_package(Clang).

> One could export a LLVM_TOOLS_DIR value from both the build and
> install trees to resolve this case the old fashioned way.

If I find time I'll write a patch to add this. Even though a client
can read the directory from one of the exported targets they might not
be aware they could do this so providing a LLVM_TOOLS_DIR variable
could be useful.

Would you mind if I implemented this by having code in
LLVMConfig.cmake that actually reads the $<TARGET_FILE_DIR:opt>
variable to set LLVM_TOOLS_DIR? I can guard this with a check for the
existence of the target so that if LLVM_BUILD_TOOLS is disabled an
error isn't produced.

>> Also should we also expose whether or not LLVM was built with
>> assertions? It's exposed as LLVM_BUILD_ENABLE_ASSERTIONS in the build
>> directory but isn't available in the installed version of LLVM.
>
> We already have a few LLVM_ENABLE_<SOMETHING> values exported.
> One could add LLVM_ENABLE_ASSERTIONS.

Okay. I've written a patch [1] for this. Please take a look when you the time.



[1] http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20140714/226548.html

Thanks,
Dan.
_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: Fixing LLVM's CMake interface before LLVM3.5 release

Brad King
On 07/20/2014 06:22 AM, Dan Liew wrote:
> Would you mind if I implemented this by having code in
> LLVMConfig.cmake that actually reads the $<TARGET_FILE_DIR:opt>
> variable to set LLVM_TOOLS_DIR?

You can't do that because the generator expression is not evaluated
during configuration.  You need to configure the actual value.  We
already configure include directories.

-Brad

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev