On Tue, Nov 19, 2013 at 12:07:18PM +0100, Tobias Grosser wrote:> On 11/19/2013 08:50 PM, Jack Howarth wrote: >> 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> > Cheers, > Tobias
Tobias Grosser
2013-Nov-20 16:00 UTC
[LLVMdev] proposed patch to default to isl-only polly
On 11/20/2013 04:50 PM, Jack Howarth wrote:> On Tue, Nov 19, 2013 at 12:07:18PM +0100, Tobias Grosser wrote: >> On 11/19/2013 08:50 PM, Jack Howarth wrote: >>> 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
On Wed, Nov 20, 2013 at 05:00:44PM +0100, Tobias Grosser wrote:> On 11/20/2013 04:50 PM, Jack Howarth wrote: >> On Tue, Nov 19, 2013 at 12:07:18PM +0100, Tobias Grosser wrote: >>> On 11/19/2013 08:50 PM, Jack Howarth wrote: >>>> 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> >> 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-------------- next part -------------- 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" ON) +if (POLLY_USE_CLOOG) + # Build Cloog support in Polly (default is for cloog-isl). + FIND_PACKAGE(Cloog) +endif(POLLY_USE_CLOOG) + option(POLLY_ENABLE_GPGPU_CODEGEN "Enable GPGPU code generation feature" OFF) if (POLLY_ENABLE_GPGPU_CODEGEN) # Do not require CUDA, as GPU code generation test cases can be run without