Patchwork Remove warning while compiling with CLooG isl

login
register
mail settings
Submitter Tobias Grosser
Date Nov. 12, 2010, 4:30 p.m.
Message ID <4CDD6BAA.9000208@fim.uni-passau.de>
Download mbox | patch
Permalink /patch/70987/
State New
Headers show

Comments

Tobias Grosser - Nov. 12, 2010, 4:30 p.m.
Hi,

this patch fixes a warning that appears when compiling graphite with 
CLooG isl.

         * graphite-cloog-util.c (oppose_constraint,
         cloog_matrix_to_ppl_constraint,
         new_Constraint_System_from_Cloog_Matrix): Explicitly cast to int
         as CLooG isl uses unsigned integers. This triggered a warning.

@Jack: This patch should fix the compile failure with CLooG isl. Can you 
verify this?

Cheers
Tobi
Jack Howarth - Nov. 12, 2010, 7:37 p.m.
On Fri, Nov 12, 2010 at 11:30:34AM -0500, Tobias Grosser wrote:
> Hi,
>
> this patch fixes a warning that appears when compiling graphite with  
> CLooG isl.
>
>         * graphite-cloog-util.c (oppose_constraint,
>         cloog_matrix_to_ppl_constraint,
>         new_Constraint_System_from_Cloog_Matrix): Explicitly cast to int
>         as CLooG isl uses unsigned integers. This triggered a warning.
>
> @Jack: This patch should fix the compile failure with CLooG isl. Can you  
> verify this?

Tobi,
   I can confirm that this patch allows gcc trunk to bootstrap on x86_64-apple-darwin10
with ppl 0.15.9 and cloog.org git built as cloog-isl. Also..

make -k check RUNTESTFLAGS="graphite.exp --target_board=unix'{-m32,-m64}'"

shows no regressions.
         Jack
ps Are there any instructions for building cloog-ppl from cloog.org git? I would like to
test gcc trunk against that instead of legacy cloog's ppl support. I assume that cloog.org's
cloog-ppl might require ppl 0.11, no?

>
> Cheers
> Tobi

> >From 4ecdd356cd507dabeffc9c1b6c9f066a530624b7 Mon Sep 17 00:00:00 2001
> From: Tobias Grosser <grosser@fim.uni-passau.de>
> Date: Fri, 12 Nov 2010 11:22:52 -0500
> Subject: [PATCH] Remove warning because of CLooG isl/ppl difference
> 
> 	* graphite-cloog-util.c (oppose_constraint,
> 	cloog_matrix_to_ppl_constraint,
> 	new_Constraint_System_from_Cloog_Matrix): Explicitly cast to int
> 	as CLooG isl uses unsigned integers. This triggered a warning.
> ---
>  gcc/graphite-cloog-util.c |   15 ++++++++++++---
>  1 files changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/gcc/graphite-cloog-util.c b/gcc/graphite-cloog-util.c
> index 40c6fbc..df90b83 100644
> --- a/gcc/graphite-cloog-util.c
> +++ b/gcc/graphite-cloog-util.c
> @@ -60,7 +60,10 @@ oppose_constraint (CloogMatrix *m, int row)
>    int k;
>  
>    /* Do not oppose the first column: it is the eq/ineq one.  */
> -  for (k = 1; k < m->NbColumns; k++)
> +  /* Cast needed to remove warning that is generated as CLooG isl
> +     is using an unsigned int for NbColumns and CLooG PPL is
> +     using a signed int for NBColumns.  */
> +  for (k = 1; k < (int)m->NbColumns; k++)
>      mpz_neg (m->p[row][k], m->p[row][k]);
>  }
>  
> @@ -177,7 +180,10 @@ cloog_matrix_to_ppl_constraint (CloogMatrix *matrix, int row)
>    ppl_new_Coefficient (&coef);
>    ppl_new_Linear_Expression_with_dimension (&expr, dim);
>  
> -  for (j = 1; j < matrix->NbColumns - 1; j++)
> +  /* Cast needed to remove warning that is generated as CLooG isl
> +     is using an unsigned int for NbColumns and CLooG PPL is
> +     using a signed int for NBColumns.  */
> +  for (j = 1; j < (int)matrix->NbColumns - 1; j++)
>      {
>        ppl_assign_Coefficient_from_mpz_t (coef, matrix->p[row][j]);
>        ppl_Linear_Expression_add_to_coefficient (expr, j - 1, coef);
> @@ -207,7 +213,10 @@ new_Constraint_System_from_Cloog_Matrix (ppl_Constraint_System_t *pcs,
>  
>    ppl_new_Constraint_System (pcs);
>  
> -  for (i = 0; i < matrix->NbRows; i++)
> +  /* Cast needed to remove warning that is generated as CLooG isl
> +     is using an unsigned int for NbColumns and CLooG PPL is
> +     using a signed int for NBColumns.  */
> +  for (i = 0; i < (int)matrix->NbRows; i++)
>      {
>        ppl_Constraint_t c = cloog_matrix_to_ppl_constraint (matrix, i);
>        ppl_Constraint_System_insert_Constraint (*pcs, c);
> -- 
> 1.7.1
>
Tobias Grosser - Nov. 12, 2010, 8:04 p.m.
On 11/12/2010 02:37 PM, Jack Howarth wrote:
> On Fri, Nov 12, 2010 at 11:30:34AM -0500, Tobias Grosser wrote:
>> Hi,
>>
>> this patch fixes a warning that appears when compiling graphite with
>> CLooG isl.
>>
>>          * graphite-cloog-util.c (oppose_constraint,
>>          cloog_matrix_to_ppl_constraint,
>>          new_Constraint_System_from_Cloog_Matrix): Explicitly cast to int
>>          as CLooG isl uses unsigned integers. This triggered a warning.
>>
>> @Jack: This patch should fix the compile failure with CLooG isl. Can you
>> verify this?
>
> Tobi,
>     I can confirm that this patch allows gcc trunk to bootstrap on x86_64-apple-darwin10
> with ppl 0.15.9 and cloog.org git built as cloog-isl. Also..
>
> make -k check RUNTESTFLAGS="graphite.exp --target_board=unix'{-m32,-m64}'"
>
> shows no regressions.
>           Jack

Great. Thanks for testing.

> ps Are there any instructions for building cloog-ppl from cloog.org git? I would like to
> test gcc trunk against that instead of legacy cloog's ppl support. I assume that cloog.org's
> cloog-ppl might require ppl 0.11, no?

No. ;-)

I believe this is how it should work:

git clone  git://repo.or.cz/cloog-parma.git
cd cloog-parma
./get_submodules.sh
./autogen.sh
./configure
make
make check

However this one is not as well tested as cloog-isl. I believe it should 
bootstrap, but I did not verify this recently.

Cheers
Tobi
Jack Howarth - Nov. 12, 2010, 8:27 p.m.
On Fri, Nov 12, 2010 at 03:04:53PM -0500, Tobias Grosser wrote:
> On 11/12/2010 02:37 PM, Jack Howarth wrote:
>> On Fri, Nov 12, 2010 at 11:30:34AM -0500, Tobias Grosser wrote:
>>> Hi,
>>>
>>> this patch fixes a warning that appears when compiling graphite with
>>> CLooG isl.
>>>
>>>          * graphite-cloog-util.c (oppose_constraint,
>>>          cloog_matrix_to_ppl_constraint,
>>>          new_Constraint_System_from_Cloog_Matrix): Explicitly cast to int
>>>          as CLooG isl uses unsigned integers. This triggered a warning.
>>>
>>> @Jack: This patch should fix the compile failure with CLooG isl. Can you
>>> verify this?
>>
>> Tobi,
>>     I can confirm that this patch allows gcc trunk to bootstrap on x86_64-apple-darwin10
>> with ppl 0.15.9 and cloog.org git built as cloog-isl. Also..
>>
>> make -k check RUNTESTFLAGS="graphite.exp --target_board=unix'{-m32,-m64}'"
>>
>> shows no regressions.
>>           Jack
>
> Great. Thanks for testing.
>
>> ps Are there any instructions for building cloog-ppl from cloog.org git? I would like to
>> test gcc trunk against that instead of legacy cloog's ppl support. I assume that cloog.org's
>> cloog-ppl might require ppl 0.11, no?
>
> No. ;-)
>
> I believe this is how it should work:
>
> git clone  git://repo.or.cz/cloog-parma.git
> cd cloog-parma
> ./get_submodules.sh
> ./autogen.sh
> ./configure
> make
> make check
>
> However this one is not as well tested as cloog-isl. I believe it should  
> bootstrap, but I did not verify this recently.

Tobi,
   Thanks for the clarification. The original patch...

http://gcc.gnu.org/ml/gcc-patches/2010-11/msg01193.html

was a bit confusing on this issue. While the legacy cloog is defined
as CLooG-PPL, it isn't clearly stated that CLooG-Pharma actually creates
libcloog-ppl...or has that changed to avoid a shared library naming conflict
for Debian who renamed their libcloog to libcloog-ppl?
                Jack


>
> Cheers
> Tobi
Paolo Bonzini - Nov. 12, 2010, 9:01 p.m.
On 11/12/2010 09:27 PM, Jack Howarth wrote:

> Tobi,
>     Thanks for the clarification. The original patch...
>
> http://gcc.gnu.org/ml/gcc-patches/2010-11/msg01193.html
>
> was a bit confusing on this issue. While the legacy cloog is defined
> as CLooG-PPL, it isn't clearly stated that CLooG-Pharma actually creates
> libcloog-ppl...

FWIW it's Parma (as in 
http://en.wikipedia.org/wiki/File:Prosciutto_di_Parma_-_affettato2.jpg 
:P), and in the latest patches it's --enable-cloog-backend=ppl-legacy 
vs. --enable-cloog-backend=ppl.

Paolo
Jack Howarth - Nov. 12, 2010, 9:13 p.m.
On Fri, Nov 12, 2010 at 10:01:07PM +0100, Paolo Bonzini wrote:
> On 11/12/2010 09:27 PM, Jack Howarth wrote:
>
>> Tobi,
>>     Thanks for the clarification. The original patch...
>>
>> http://gcc.gnu.org/ml/gcc-patches/2010-11/msg01193.html
>>
>> was a bit confusing on this issue. While the legacy cloog is defined
>> as CLooG-PPL, it isn't clearly stated that CLooG-Pharma actually creates
>> libcloog-ppl...
>
> FWIW it's Parma (as in  
> http://en.wikipedia.org/wiki/File:Prosciutto_di_Parma_-_affettato2.jpg  
> :P), and in the latest patches it's --enable-cloog-backend=ppl-legacy  
> vs. --enable-cloog-backend=ppl.
>
> Paolo

Paolo,
   I am puzzled by the objections expressed about 'switching the backends' in the
current configure changes. Is this objection due to the change from a PPL based
to a ISL based cloog or the switch from legacy cloog itself? As legacy cloog was always
intended to be replaced by an official CLooG.org release, it ssems like a natural
change. Certainly a switch to defaulting cloog to cloog-isl eliminates any issues
with the ppl ABI changes from 0.10.x to 0.11. The only problem would be the absence
of tarballs in gcc infrastructure for building cloog.org's cloog-isl.
          Jack
Paolo Bonzini - Nov. 12, 2010, 9:30 p.m.
On 11/12/2010 10:13 PM, Jack Howarth wrote:
>     I am puzzled by the objections expressed about 'switching the backends' in the
> current configure changes.

There's no objection to switching the backends, only to having automatic 
detection of backends so that the same configure command-line can give 
different assembly language.

In my approval to Tabis's patches, in fact, I preapproved committing the 
patch with isl active.

Paolo
Dave Korn - Nov. 12, 2010, 9:38 p.m.
On 12/11/2010 21:01, Paolo Bonzini wrote:
> On 11/12/2010 09:27 PM, Jack Howarth wrote:
> 
>> Tobi,
>>     Thanks for the clarification. The original patch...
>>
>> http://gcc.gnu.org/ml/gcc-patches/2010-11/msg01193.html
>>
>> was a bit confusing on this issue. While the legacy cloog is defined
>> as CLooG-PPL, it isn't clearly stated that CLooG-Pharma actually creates
>> libcloog-ppl...
> 
> FWIW it's Parma (as in
> http://en.wikipedia.org/wiki/File:Prosciutto_di_Parma_-_affettato2.jpg
> :P), 

  You've made me all hungry now!  *goes to get sandwich*

    cheers,
      DaveK
Tobias Grosser - Nov. 12, 2010, 9:49 p.m.
On 11/12/2010 04:30 PM, Paolo Bonzini wrote:
> On 11/12/2010 10:13 PM, Jack Howarth wrote:
>> I am puzzled by the objections expressed about 'switching the
>> backends' in the
>> current configure changes.
>
> There's no objection to switching the backends, only to having automatic
> detection of backends so that the same configure command-line can give
> different assembly language.
>
> In my approval to Tabis's patches, in fact, I preapproved committing the
> patch with isl active.

Thanks Paolo,

we are going to test polly with cloog-isl ourselves for a week or two 
(using our automated spec testers) and as soon as we do not find any 
more problems we are going to ask Sven to release a tar ball and ask for 
more testing on the mailing lists.

Cheers
Tobi
Jack Howarth - Nov. 12, 2010, 9:55 p.m.
On Fri, Nov 12, 2010 at 04:49:18PM -0500, Tobias Grosser wrote:
> On 11/12/2010 04:30 PM, Paolo Bonzini wrote:
>> On 11/12/2010 10:13 PM, Jack Howarth wrote:
>>> I am puzzled by the objections expressed about 'switching the
>>> backends' in the
>>> current configure changes.
>>
>> There's no objection to switching the backends, only to having automatic
>> detection of backends so that the same configure command-line can give
>> different assembly language.
>>
>> In my approval to Tabis's patches, in fact, I preapproved committing the
>> patch with isl active.
>
> Thanks Paolo,
>
> we are going to test polly with cloog-isl ourselves for a week or two  
> (using our automated spec testers) and as soon as we do not find any  
> more problems we are going to ask Sven to release a tar ball and ask for  
> more testing on the mailing lists.
>
> Cheers
> Tobi

Tobi,
  FYI, cloog.git from yesterday completely passes its testsuite on
x86_64-apple-darwin10 as the cloog-isl build.
              Jack

Patch

From 4ecdd356cd507dabeffc9c1b6c9f066a530624b7 Mon Sep 17 00:00:00 2001
From: Tobias Grosser <grosser@fim.uni-passau.de>
Date: Fri, 12 Nov 2010 11:22:52 -0500
Subject: [PATCH] Remove warning because of CLooG isl/ppl difference

	* graphite-cloog-util.c (oppose_constraint,
	cloog_matrix_to_ppl_constraint,
	new_Constraint_System_from_Cloog_Matrix): Explicitly cast to int
	as CLooG isl uses unsigned integers. This triggered a warning.
---
 gcc/graphite-cloog-util.c |   15 ++++++++++++---
 1 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/gcc/graphite-cloog-util.c b/gcc/graphite-cloog-util.c
index 40c6fbc..df90b83 100644
--- a/gcc/graphite-cloog-util.c
+++ b/gcc/graphite-cloog-util.c
@@ -60,7 +60,10 @@  oppose_constraint (CloogMatrix *m, int row)
   int k;
 
   /* Do not oppose the first column: it is the eq/ineq one.  */
-  for (k = 1; k < m->NbColumns; k++)
+  /* Cast needed to remove warning that is generated as CLooG isl
+     is using an unsigned int for NbColumns and CLooG PPL is
+     using a signed int for NBColumns.  */
+  for (k = 1; k < (int)m->NbColumns; k++)
     mpz_neg (m->p[row][k], m->p[row][k]);
 }
 
@@ -177,7 +180,10 @@  cloog_matrix_to_ppl_constraint (CloogMatrix *matrix, int row)
   ppl_new_Coefficient (&coef);
   ppl_new_Linear_Expression_with_dimension (&expr, dim);
 
-  for (j = 1; j < matrix->NbColumns - 1; j++)
+  /* Cast needed to remove warning that is generated as CLooG isl
+     is using an unsigned int for NbColumns and CLooG PPL is
+     using a signed int for NBColumns.  */
+  for (j = 1; j < (int)matrix->NbColumns - 1; j++)
     {
       ppl_assign_Coefficient_from_mpz_t (coef, matrix->p[row][j]);
       ppl_Linear_Expression_add_to_coefficient (expr, j - 1, coef);
@@ -207,7 +213,10 @@  new_Constraint_System_from_Cloog_Matrix (ppl_Constraint_System_t *pcs,
 
   ppl_new_Constraint_System (pcs);
 
-  for (i = 0; i < matrix->NbRows; i++)
+  /* Cast needed to remove warning that is generated as CLooG isl
+     is using an unsigned int for NbColumns and CLooG PPL is
+     using a signed int for NBColumns.  */
+  for (i = 0; i < (int)matrix->NbRows; i++)
     {
       ppl_Constraint_t c = cloog_matrix_to_ppl_constraint (matrix, i);
       ppl_Constraint_System_insert_Constraint (*pcs, c);
-- 
1.7.1