[Polly] Update lit config for Cloog

Star_Tan · September 2, 2013, 2:44pm

>On 09/01/2013 08:02 PM, Star Tan wrote:
>> Hi all,
>>
>>
>> Attached patch file to update lit config for Cloog. Without it, Polly always skips Cloog testings when we run "make check-polly".
>
>Dear Star Tan,
>
>thanks a lot for the patch. It looks very reasonable, but I am wondering 
>why it was not needed before or what problem it fixes exactly. Could you 
>add some information about this to the commit message.
>
I am not sure why it was not needed before; maybe it has never worked well before. The problem is that Polly never executes Cloog specific testcases no matther whether Cloog is found or not. I find this problem because I put a new testcase in test/Cloog/CodeGen/, but it is never executed.

@Sebastian, you added the Cloog directory in r169159, including all testcases and the lit.local.cfg. The lit.local.cfg ensures that  cloog specific testcases are executed only with CLOOG_FOUND by adding the following lit commands in test/Cloog/lit.local.cfg:

        cloog = config.root.cloog_found
        if cloog not in ['TRUE', 'true'] :
            config.unsupported = True

However, there are two problems:

First, since the cloog_found is set as "@CLOOG_FOUND@",  I think the following "sed" command should be added into Makefile:

        sed -e "s#@CLOOG_FOUN@#$(CLOOG_FOUND)#g"

Unfortunately such command is missed in current Polly.  If it is not needed, I am curious how could Polly determine the value of @CLOOG_FOUND@ ?

Second, even if we add the "sed" command in Makefile, the config.root.cloog_found would be set as "yes" or null, but Polly currently compares it with "TRUE" or "true", which thus always fails and the Cloog testcases will never be executed.

@Sebastian, could you do me a favor to have a review of this problem?  FYI, I have re-attached the patch file.

Thanks,

Star Tan

0001-Update-lit-config-for-Cloog.patch (1.97 KB)

Tobias_Grosser6 · September 2, 2013, 4:12pm

This is surprising. Even without your patch my test/lit.site.cfg file contains:

config.enable_gpgpu_codegen = ""
config.cloog_found = "TRUE"

(With CLOOG enabled and GPGPU_codegen disabled). Also, the CLooG test are run (and are failing) as expected.

I think it would be good to understand why this different behaviour can be observed. Sebastian, any ideas?

Cheers
Tobias

Star_Tan · September 2, 2013, 4:22pm

>On 09/02/2013 07:44 AM, Star Tan wrote:
>>
>>> On 09/01/2013 08:02 PM, Star Tan wrote:
>>>> Hi all,
>>>>
>>>>
>>>> Attached patch file to update lit config for Cloog. Without it, Polly always skips Cloog testings when we run "make check-polly".
>>>
>>> Dear Star Tan,
>>>
>>> thanks a lot for the patch. It looks very reasonable, but I am wondering
>>> why it was not needed before or what problem it fixes exactly. Could you
>>> add some information about this to the commit message.
>>>
>> I am not sure why it was not needed before; maybe it has never worked well before. The problem is that Polly never executes Cloog specific testcases no matther whether Cloog is found or not. I find this problem because I put a new testcase in test/Cloog/CodeGen/, but it is never executed.
>> @Sebastian, you added the Cloog directory in r169159, including all testcases and the lit.local.cfg. The lit.local.cfg ensures that  cloog specific testcases are executed only with CLOOG_FOUND by adding the following lit commands in test/Cloog/lit.local.cfg:
>>          cloog = config.root.cloog_found
>>          if cloog not in ['TRUE', 'true'] :
>>              config.unsupported = True
>> However, there are two problems:
>> First, since the cloog_found is set as "@CLOOG_FOUND@",  I think the following "sed" command should be added into Makefile:
>>          sed -e "s#@CLOOG_FOUN@#$(CLOOG_FOUND)#g"
>> Unfortunately such command is missed in current Polly.  If it is not needed, I am curious how could Polly determine the value of @CLOOG_FOUND@ ?
>> Second, even if we add the "sed" command in Makefile, the config.root.cloog_found would be set as "yes" or null, but Polly currently compares it with "TRUE" or "true", which thus always fails and the Cloog testcases will never be executed.
>> @Sebastian, could you do me a favor to have a review of this problem?  FYI, I have re-attached the patch file.
>
>This is surprising. Even without your patch my test/lit.site.cfg file 
>contains:
>
>config.enable_gpgpu_codegen = ""
>config.cloog_found = "TRUE"
>
>(With CLOOG enabled and GPGPU_codegen disabled). Also, the CLooG test 
>are run (and are failing) as expected.
>
>I think it would be good to understand why this different behaviour can 
>be observed. Sebastian, any ideas?
>
Interesting! In my computer, it shows as:

config.enable_gpgpu_codegen = "@CUDALIB_FOUND@"
config.cloog_found = "@CLOOG_FOUND@"

The source version is:

LLVM: r189730;  Polly: r189177; Clang:r189731

Best,

Star Tan

Sebastian_Pop3 · September 26, 2013, 7:33pm

Hi,

Star Tan wrote:

>>
>>>> Hi all,
>>>>
>>>>
>>>> Attached patch file to update lit config for Cloog. Without it, Polly always skips Cloog testings when we run "make check-polly".
>>>
>>> Dear Star Tan,
>>>
>>> thanks a lot for the patch. It looks very reasonable, but I am wondering
>>> why it was not needed before or what problem it fixes exactly. Could you
>>> add some information about this to the commit message.
>>>
>> I am not sure why it was not needed before; maybe it has never worked well before. The problem is that Polly never executes Cloog specific testcases no matther whether Cloog is found or not. I find this problem because I put a new testcase in test/Cloog/CodeGen/, but it is never executed.
>> @Sebastian, you added the Cloog directory in r169159, including all testcases and the lit.local.cfg. The lit.local.cfg ensures that cloog specific testcases are executed only with CLOOG_FOUND by adding the following lit commands in test/Cloog/lit.local.cfg:
>> cloog = config.root.cloog_found
>> if cloog not in ['TRUE', 'true'] :
>> config.unsupported = True
>> However, there are two problems:
>> First, since the cloog_found is set as "@CLOOG_FOUND@", I think the following "sed" command should be added into Makefile:
>> sed -e "s#@CLOOG_FOUN@#$(CLOOG_FOUND)#g"
>> Unfortunately such command is missed in current Polly. If it is not needed, I am curious how could Polly determine the value of @CLOOG_FOUND@ ?
>> Second, even if we add the "sed" command in Makefile, the config.root.cloog_found would be set as "yes" or null, but Polly currently compares it with "TRUE" or "true", which thus always fails and the Cloog testcases will never be executed.
>> @Sebastian, could you do me a favor to have a review of this problem? FYI, I have re-attached the patch file.
>
>This is surprising. Even without your patch my test/lit.site.cfg file
>contains:
>
>config.enable_gpgpu_codegen = ""
>config.cloog_found = "TRUE"
>
>(With CLOOG enabled and GPGPU_codegen disabled). Also, the CLooG test
>are run (and are failing) as expected.
>
>I think it would be good to understand why this different behaviour can
>be observed. Sebastian, any ideas?
>
Interesting! In my computer, it shows as:

config.enable_gpgpu_codegen = "@CUDALIB_FOUND@"
config.cloog_found = "@CLOOG_FOUND@"

I think the problem is:

lit.site.cfg.in:config.cloog_found = "@CLOOG_FOUND@"

it should be lower case letters as in:

Makefile.config.in:CLOOG_FOUND := @cloog_found@

because when we detect cloog, we use the function in find_lib_and_headers.m4

find_lib_and_headers([cloog], [cloog/isl/cloog.h], [cloog-isl])

./autoconf/m4/find_lib_and_headers.m4: AC_SUBST([$1_found],["yes"])

and this will be instantiated as AC_SUBST([cloog_found],["yes"]).

A patch that changes lit.site.cfg.in

- config.enable_gpgpu_codegen = "@CUDALIB_FOUND@"
- config.cloog_found = "@CLOOG_FOUND@"
+ config.enable_gpgpu_codegen = "@cudalib_found@"
+ config.cloog_found = "@cloog_found@"

is preaproved.

Thanks,
Sebastian

Star_Tan · September 27, 2013, 3:42am

Hi Sebastian, thanks for your reply!

At 2013-09-27 03:33:04,“Sebastian Pop” spop@codeaurora.org wrote:>Hi,

Star Tan wrote:

Hi all,

Attached patch file to update lit config for Cloog. Without it, Polly always skips Cloog testings when we run “make check-polly”.

Dear Star Tan,

thanks a lot for the patch. It looks very reasonable, but I am wondering
why it was not needed before or what problem it fixes exactly. Could you
add some information about this to the commit message.

I am not sure why it was not needed before; maybe it has never worked well before. The problem is that Polly never executes Cloog specific testcases no matther whether Cloog is found or not. I find this problem because I put a new testcase in test/Cloog/CodeGen/, but it is never executed.
@Sebastian, you added the Cloog directory in r169159, including all testcases and the lit.local.cfg. The lit.local.cfg ensures that cloog specific testcases are executed only with CLOOG_FOUND by adding the following lit commands in test/Cloog/lit.local.cfg:
cloog = config.root.cloog_found
if cloog not in [‘TRUE’, ‘true’] :
config.unsupported = True
However, there are two problems:
First, since the cloog_found is set as “@CLOOG_FOUND@”, I think the following “sed” command should be added into Makefile:
sed -e “s#@CLOOG_FOUN@#$(CLOOG_FOUND)#g”
Unfortunately such command is missed in current Polly. If it is not needed, I am curious how could Polly determine the value of @CLOOG_FOUND@ ?
Second, even if we add the “sed” command in Makefile, the config.root.cloog_found would be set as “yes” or null, but Polly currently compares it with “TRUE” or “true”, which thus always fails and the Cloog testcases will never be executed.
@Sebastian, could you do me a favor to have a review of this problem? FYI, I have re-attached the patch file.

This is surprising. Even without your patch my test/lit.site.cfg file
contains:

config.enable_gpgpu_codegen = “”
config.cloog_found = “TRUE”

(With CLOOG enabled and GPGPU_codegen disabled). Also, the CLooG test
are run (and are failing) as expected.

I think it would be good to understand why this different behaviour can
be observed. Sebastian, any ideas?

Interesting! In my computer, it shows as:

config.enable_gpgpu_codegen = “@CUDALIB_FOUND@”
config.cloog_found = “@CLOOG_FOUND@”

I think the problem is:

lit.site.cfg.in:config.cloog_found = “@CLOOG_FOUND@”

it should be lower case letters as in:

Makefile.config.in:CLOOG_FOUND := @cloog_found@

because when we detect cloog, we use the function in find_lib_and_headers.m4

find_lib_and_headers([cloog], [cloog/isl/cloog.h], [cloog-isl])

./autoconf/m4/find_lib_and_headers.m4: AC_SUBST([$1_found],[“yes”])

and this will be instantiated as AC_SUBST([cloog_found],[“yes”]).

A patch that changes lit.site.cfg.in

config.enable_gpgpu_codegen = “@CUDALIB_FOUND@”

config.cloog_found = “@CLOOG_FOUND@”

config.enable_gpgpu_codegen = “@cudalib_found@”

config.cloog_found = “@cloog_found@”

If you change “CLOOG_FOUND” to “cloog_found”, which means the final lit.site.cfg will looks like:

config.cloog_found = yes

However, the lit.local.cfg in Cloog directory is:

cloog = config.root.cloog_found
if cloog not in [‘TRUE’, ‘true’] :
config.unsupported = True

which means it will compare the config_found with ‘TRUE’/‘true’, not ‘yes’. So, it still does not work. That’s why I propose to modify the lit.local.cfg to compare cloog_found with ‘yes’ rather than ‘TRUE’ or ‘true’.
Did I miss something?

Star Tan

Tobias_Grosser6 · September 27, 2013, 5:50am

Adding yes does not seem to be a problem. So moving to cloog_found in lowercase as well as adding yes, seems the right approach, no?

Tobi

Topic		Replies	Views
[polly] removing cloog dependence in the testsuite LLVM Dev List Archives	11	61	December 3, 2012
proposed patch to default to isl-only polly LLVM Dev List Archives	5	71	November 21, 2013
-polly-codegen-isl LLVM Dev List Archives	3	144	February 11, 2013
cloog/isl required for polly 3.2? LLVM Dev List Archives	0	57	December 18, 2012
[POLLY] fix Bug 15817 LLVM Dev List Archives	12	47	June 6, 2013

[Polly] Update lit config for Cloog

Related Topics