[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

"optionally use system isl instead"?

268–269

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.

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
 )