proposed patch to default to isl-only polly

Tobias,
    Can we add something like the following to polly 3.4?

Index: CMakeLists.txt

polly_use_cloog.diff (704 Bytes)

Hi Jack,

thanks for writing a patch for your feature request. The patch itself looks good the way it is implemented. Though I wonder why it is needed. Polly only uses CLooG if available, so I assume you have a CLooG version installed, but you do not want to use it. What is the reason you do not want to use the installed CLooG version. Is the CLooG version outdated and Polly does not work with it? Or is there another reason for this?

The reason the isl code generation is not yet default is because we do not yet support openmp code generation support for it. Would you be OK with installing Polly without OpenMP generation support?

Cheers,
Tobias

Tobias,
     Can we add something like the following to polly 3.4?

Index: CMakeLists.txt

--- CMakeLists.txt (revision 195142)
+++ CMakeLists.txt (working copy)
@@ -81,9 +81,14 @@ set(CMAKE_MODULE_PATH ${CMAKE_MODULE_PAT

  FIND_PACKAGE(Isl REQUIRED)
  FIND_PACKAGE(Gmp REQUIRED)
-FIND_PACKAGE(Cloog)
  FIND_PACKAGE(Pluto)

+option(POLLY_USE_CLOOG "Build Polly with Cloog support" OFF)
+if (POLLY_USE_CLOOG)
+ # Build Cloog support in Polly (default is ISL-only).
+ FIND_PACKAGE(Cloog)
+endif(POLLY_USE_CLOOG)

Hi Jack,

thanks for writing a patch for your feature request. The patch itself
looks good the way it is implemented. Though I wonder why it is needed.
Polly only uses CLooG if available, so I assume you have a CLooG version
installed, but you do not want to use it. What is the reason you do not
want to use the installed CLooG version. Is the CLooG version outdated
and Polly does not work with it? Or is there another reason for this?

The reason the isl code generation is not yet default is because we do
not yet support openmp code generation support for it. Would you be OK
with installing Polly without OpenMP generation support?

Tobias,
    The idea is to have some control on whether Polly is built with
cloog outside of crudely deinstalling the cloog headers and libraries
each time Polly is built. If you don't want to default to ISL-only yet,
this change would still be useful even if the default is switched to...

option(POLLY_USE_CLOOG "Build Polly with Cloog support" ON)

since users could still pass cmake...

-DPOLLY_USE_CLOOG=OFF

rather than deinstalling the cloog headers/libraries to achieve an
ISL-only build of Polly.
     I am confused by your comments on OpenMP support. Is this support
parallel and independent of the new OpenMP support from Intel?

http://blog.llvm.org/2013/10/openmp-project.html

My understanding was that this code hadn't been merged in for the
llvm 3.4 release.
          Jack

Tobias,
      Can we add something like the following to polly 3.4?

Index: CMakeLists.txt

--- CMakeLists.txt (revision 195142)
+++ CMakeLists.txt (working copy)
@@ -81,9 +81,14 @@ set(CMAKE_MODULE_PATH ${CMAKE_MODULE_PAT

   FIND_PACKAGE(Isl REQUIRED)
   FIND_PACKAGE(Gmp REQUIRED)
-FIND_PACKAGE(Cloog)
   FIND_PACKAGE(Pluto)

+option(POLLY_USE_CLOOG "Build Polly with Cloog support" OFF)
+if (POLLY_USE_CLOOG)
+ # Build Cloog support in Polly (default is ISL-only).
+ FIND_PACKAGE(Cloog)
+endif(POLLY_USE_CLOOG)

Hi Jack,

thanks for writing a patch for your feature request. The patch itself
looks good the way it is implemented. Though I wonder why it is needed.
Polly only uses CLooG if available, so I assume you have a CLooG version
installed, but you do not want to use it. What is the reason you do not
want to use the installed CLooG version. Is the CLooG version outdated
and Polly does not work with it? Or is there another reason for this?

The reason the isl code generation is not yet default is because we do
not yet support openmp code generation support for it. Would you be OK
with installing Polly without OpenMP generation support?

Tobias,
     The idea is to have some control on whether Polly is built with
cloog outside of crudely deinstalling the cloog headers and libraries
each time Polly is built.

Sure, I can see a need for this. The only reason I asked is if you want to do this for package management reasons or if there is a problem with
the cloog parts of polly. In any case, I am happy to add this patch,
but if you use it to get around a bug, it would be good to also fix the actual bug.

> If you don't want to default to ISL-only yet,

this change would still be useful even if the default is switched to...

option(POLLY_USE_CLOOG "Build Polly with Cloog support" ON)

Yes, I would prefer this (also as this is then consistent with the automake build)

The patch you posted prefer, with the flag switched to off looks fine.
Feel free to commit it.

since users could still pass cmake...

-DPOLLY_USE_CLOOG=OFF

rather than deinstalling the cloog headers/libraries to achieve an
ISL-only build of Polly.
      I am confused by your comments on OpenMP support. Is this support
parallel and independent of the new OpenMP support from Intel?

http://blog.llvm.org/2013/10/openmp-project.html

My understanding was that this code hadn't been merged in for the
llvm 3.4 release.

Polly can since a long time detect parallel loops and transform them into OpenMP parallel loops using libgomp, the gcc openmp library. This
is independent of the Intel afford. Intel provides an openmp library
similar to libgomp, though we may later decide to use as the openmp library we target.

Cheers,
Tobias

Tobias,
      Can we add something like the following to polly 3.4?

Index: CMakeLists.txt

--- CMakeLists.txt (revision 195142)
+++ CMakeLists.txt (working copy)
@@ -81,9 +81,14 @@ set(CMAKE_MODULE_PATH ${CMAKE_MODULE_PAT

   FIND_PACKAGE(Isl REQUIRED)
   FIND_PACKAGE(Gmp REQUIRED)
-FIND_PACKAGE(Cloog)
   FIND_PACKAGE(Pluto)

+option(POLLY_USE_CLOOG "Build Polly with Cloog support" OFF)
+if (POLLY_USE_CLOOG)
+ # Build Cloog support in Polly (default is ISL-only).
+ FIND_PACKAGE(Cloog)
+endif(POLLY_USE_CLOOG)

Hi Jack,

thanks for writing a patch for your feature request. The patch itself
looks good the way it is implemented. Though I wonder why it is needed.
Polly only uses CLooG if available, so I assume you have a CLooG version
installed, but you do not want to use it. What is the reason you do not
want to use the installed CLooG version. Is the CLooG version outdated
and Polly does not work with it? Or is there another reason for this?

The reason the isl code generation is not yet default is because we do
not yet support openmp code generation support for it. Would you be OK
with installing Polly without OpenMP generation support?

Tobias,
     The idea is to have some control on whether Polly is built with
cloog outside of crudely deinstalling the cloog headers and libraries
each time Polly is built.

Sure, I can see a need for this. The only reason I asked is if you want
to do this for package management reasons or if there is a problem with
the cloog parts of polly. In any case, I am happy to add this patch,
but if you use it to get around a bug, it would be good to also fix the
actual bug.

Tobias,
    No. As the original posting showed, there are no regression in Polly
when built with the cloog support but the cmake build is missing fine
control over how Polly is built. My understanding was the Polly developers
were building isl-only but using configure for this since you have
the --without-cloog option to use there. This patch provides the same
functionality to the cmake build so that the isl-only build can be
achieved without forcing the user to uninstall cloog.

> If you don't want to default to ISL-only yet,

this change would still be useful even if the default is switched to...

option(POLLY_USE_CLOOG "Build Polly with Cloog support" ON)

Yes, I would prefer this (also as this is then consistent with the
automake build)

The patch you posted prefer, with the flag switched to off looks fine.
Feel free to commit it.

I don't have commit access. Can you commit the correct patch that is attached
into polly trunk and 3.4 branch? Thanks in advance.
          Jack

polly_use_cloog.diff (708 Bytes)

This makes perfect sense. Committed in r195344 and r195348.

Thanks again for the patch. I really appreciate to get the fix as an actual patch, even though I unfortunately forgot to add your name in the commit message

Cheers,
Tobias