[Cmake] Install the isl headers into the install tree

[philip.pfaffe]

Summary

isl headers are currently missing in a Polly installation. Because the Polly headers depend on those, code can't be compiled against an installed Polly.

This patch installs the isl headers. I left a TODO, as optionally it should be possible to use a system version of isl instead of the one shipped with Polly.

When compiling, clients of the installation need to add -I${PREFIX}/include/polly/ to there include path right now, because there currently is no way to export this path automatically.

[Meinersbur]

I expected it would "just work" because eg. ScopInfo.h uses

#include "isl/ctx.h"

and therefore already find ctx.h in the isl sibling directory. (#include by quotes first searches relative to the directory of the current file). Unfortunately we have headers in subdirectories such as Support/SCEVAffinator.h which also #include "isl/ctx.h" but does not find it.

I would have liked if we would have flattened the header directory structure instead as I couldn't identify a pattern when a header it put in a subdirectory and when it does not. Unfortunately the isl headers themselves use

#include <isl/ctx.h>

and therefore do not find their sibling files without a compiler flag. Even worse, while ScopInfo.h finds isl/ctx.h, the others will fall back to the system isl (if available) and give us a mix of Polly's isl headers and system isl headers. This might not be too much of a problem because I do not expect many compatibility issues between them (as long as we do not use any C++ bindings)

Could you also write a few lines into the documentation about how to use it? For instance, it is also important to link against libPollyISL, not libisl.so.

You are right in that a PollyConfig.cmake would be a solution that could handle these things. However, this requires cmake as your build system.

lib/External/CMakeLists.txt

266 ↗

(On Diff #88344)

"optionally use system isl instead"?

268–269 ↗

(On Diff #88344)

Does it make sense to make this configurable? What would be the use case for this?

The cmake doc for install(DESTINATION)mentions it can be either absolute or relative to CMAKE_INSTALL_PREFIX. Could you mention that in the description and/or rename it to POLLY_ISL_HEADER_INSTALL_PREFIX?

[Meinersbur]

I forgot the most important part: Thanks for submitting the patch!

[philip.pfaffe]

Addressed the inline comments. It probably isn't worthwhile making the header install prefix configurable. That was actually a remnant from my experiments with exporting a Polly package.

Where in the documentation should the details about using a Polly installation go?

[philip.pfaffe]

Missed another (generated) header.

[Meinersbur]

LGTM. If you do not have write-access to the repository I can commit for you.

I started to add a PollyConfig.h at github. Is this what you had in mind to export the include path?

[philip.pfaffe]

In D29931#678400, @Meinersbur wrote:

LGTM. If you do not have write-access to the repository I can commit for you.

Thanks! I don't have write-access, so any help commiting this is appreciated.

I started to add a PollyConfig.h at github. Is this what you had in mind to export the include path?

Nice! That is what I was thinking of. I'll start a discussion over there.

polly/trunk/lib/External/CMakeLists.txt

set (ISL_FILES
     isl/imath/imath.c
     isl/imath/imrat.c
     )
 
 add_polly_library(PollyISL
   ${ISL_FILES}
 )
 
+
+# TODO: optionally use system isl instead
+if (NOT LLVM_INSTALL_TOOLCHAIN_ONLY)
+  install(DIRECTORY
+    ${ISL_SOURCE_DIR}/include/
+    ${ISL_BINARY_DIR}/include/
+    DESTINATION include/polly
+    FILES_MATCHING
+    PATTERN "*.h"
+    PATTERN "CMakeFiles" EXCLUDE
+    PATTERN ".svn" EXCLUDE
+    )
+endif()
+
 add_executable(polly-isl-test
   isl/isl_test.c
 )
 set_target_properties(polly-isl-test PROPERTIES FOLDER "Polly")
 
 target_link_libraries(polly-isl-test
   PollyISL
 )