[LLVMdev] proposed patch to default to isl-only polly

[Tobias Grosser]

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