Re: [PATCH,GRAPHITE] Fix for P1 bug 58028
On Fri, Feb 28, 2014 at 8:37 PM, Mircea Namolaru mircea.namol...@inria.fr wrote: Hi, Thanks. Here is the updated patch. Boostrapped / tested on x86_64-unknown-linux-gnu and applied. Thanks, Richard. 2014-02-26 Tobias Grosser tob...@grosser.es Mircea Namolaru mircea.namol...@inria.fr PR tree-optimization/58028 * graphite-clast-to-gimple.c (set_cloog_options): Don't remove scalar dimensions. Index: gcc/graphite-clast-to-gimple.c === --- gcc/graphite-clast-to-gimple.c (revision 207298) +++ gcc/graphite-clast-to-gimple.c (working copy) @@ -1522,6 +1522,13 @@ variables. */ options-save_domains = 1; + /* Do not remove scalar dimensions. CLooG by default removes scalar + dimensions very early from the input schedule. However, they are + necessary to correctly derive from the saved domains + (options-save_domains) the relationship between the generated loops + and the schedule dimensions they are generated from. */ + options-noscalars = 1; + /* Disable optimizations and make cloog generate source code closer to the input. This is useful for debugging, but later we want the optimized code. Mircea
Re: [PATCH,GRAPHITE] Fix for P1 bug 58028
On 03/03/2014 12:39 PM, Richard Biener wrote: On Fri, Feb 28, 2014 at 8:37 PM, Mircea Namolaru mircea.namol...@inria.fr wrote: Hi, Thanks. Here is the updated patch. Boostrapped / tested on x86_64-unknown-linux-gnu and applied. Thanks Richard! Tobias
Re: [PATCH,GRAPHITE] Fix for P1 bug 58028
Hi, Thanks. Here is the updated patch. 2014-02-26 Tobias Grosser tob...@grosser.es Mircea Namolaru mircea.namol...@inria.fr PR tree-optimization/58028 * graphite-clast-to-gimple.c (set_cloog_options): Don't remove scalar dimensions. Index: gcc/graphite-clast-to-gimple.c === --- gcc/graphite-clast-to-gimple.c (revision 207298) +++ gcc/graphite-clast-to-gimple.c (working copy) @@ -1522,6 +1522,13 @@ variables. */ options-save_domains = 1; + /* Do not remove scalar dimensions. CLooG by default removes scalar + dimensions very early from the input schedule. However, they are + necessary to correctly derive from the saved domains + (options-save_domains) the relationship between the generated loops + and the schedule dimensions they are generated from. */ + options-noscalars = 1; + /* Disable optimizations and make cloog generate source code closer to the input. This is useful for debugging, but later we want the optimized code. Mircea
Re: [PATCH,GRAPHITE] Fix for P1 bug 58028
On Wed, Feb 26, 2014 at 10:13 PM, Tobias Grosser tob...@grosser.es wrote: On 02/26/2014 10:09 PM, Mircea Namolaru wrote: This patch fixes the libgomp problems: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58028 2014-02-26 Tobias Grosser tob...@grosser.es Mircea Namolaru mnamo...@inria.fr Fix for bug 58028 * graphite-clast-to-gimple.c (set_cloog_options): Don't remove scalar dimensions Index: gcc/graphite-clast-to-gimple.c === --- gcc/graphite-clast-to-gimple.c (revision 207298) +++ gcc/graphite-clast-to-gimple.c (working copy) @@ -1522,6 +1522,13 @@ variables. */ options-save_domains = 1; + /* Do not remove scalar dimensions. Cloog be default removes scalar + dimensions very early from the input schedule. However, they are + necessary to correctly derive from the saved domains + (options-save_domains) the relationship between the generated loops + and the schedule dimensions they are generated from. */ + options-noscalars = 1; + /* Disable optimizations and make cloog generate source code closer to the input. This is useful for debugging, but later we want the optimized code. Tested on x86-64 Linux, no regressions for c/c++/fortran. Got a notification failure from gcc-patches for the initial submission of this patch. However, Tobias (on cc) received it and already sent his comments, (see http://gcc.gnu.org/ml/gcc-patches/2014-02/msg01540.html), that are addressed in this re-submission. Yes, very nice. Thanks Mircea for reacting so quickly. This patch look good to me. @Release-managers: Is this patch OK for trunk? It fixes a P1 regression, so yes. Richard. Cheers, Tobias
Re: [PATCH,GRAPHITE] Fix for P1 bug 58028
Hi Tobias and Mircea, On 02/26/2014 10:09 PM, Mircea Namolaru wrote: + /* Do not remove scalar dimensions. Cloog be default removes scalar + dimensions very early from the input schedule. However, they are Small nit: Instead of 'be default' it should be 'by default' Tobias
Re: [PATCH,GRAPHITE] Fix for P1 bug 58028
Tobias Burnus tobias.bur...@physik.fu-berlin.de writes: On 02/26/2014 10:09 PM, Mircea Namolaru wrote: + /* Do not remove scalar dimensions. Cloog be default removes scalar + dimensions very early from the input schedule. However, they are Small nit: Instead of 'be default' it should be 'by default' ... and two spaces after the full stops. While you're at it, please fix the CLooG capitalization ;-) Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University
Re: [PATCH,GRAPHITE] Fix for P1 bug 58028
Thanks for comments - updated the patch (fixed my e-mail address too :-)). 2014-02-26 Tobias Grosser tob...@grosser.es Mircea Namolaru mircea.namol...@inria.fr Fix for bug 58028 * graphite-clast-to-gimple.c (set_cloog_options): Don't remove scalar dimensions. Index: gcc/graphite-clast-to-gimple.c === --- gcc/graphite-clast-to-gimple.c (revision 207298) +++ gcc/graphite-clast-to-gimple.c (working copy) @@ -1522,6 +1522,13 @@ variables. */ options-save_domains = 1; + /* Do not remove scalar dimensions. CLooG by default removes scalar + dimensions very early from the input schedule. However, they are + necessary to correctly derive from the saved domains + (options-save_domains) the relationship between the generated loops + and the schedule dimensions they are generated from. */ + options-noscalars = 1; + /* Disable optimizations and make cloog generate source code closer to the input. This is useful for debugging, but later we want the optimized code. - Mail original - De: Rainer Orth r...@cebitec.uni-bielefeld.de À: Tobias Burnus tobias.bur...@physik.fu-berlin.de Cc: Tobias Grosser tob...@grosser.es, Mircea Namolaru mircea.namol...@inria.fr, gcc-patches@gcc.gnu.org Envoyé: Jeudi 27 Février 2014 11:14:30 Objet: Re: [PATCH,GRAPHITE] Fix for P1 bug 58028 Tobias Burnus tobias.bur...@physik.fu-berlin.de writes: On 02/26/2014 10:09 PM, Mircea Namolaru wrote: + /* Do not remove scalar dimensions. Cloog be default removes scalar + dimensions very early from the input schedule. However, they are Small nit: Instead of 'be default' it should be 'by default' ... and two spaces after the full stops. While you're at it, please fix the CLooG capitalization ;-) Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University
Re: [PATCH,GRAPHITE] Fix for P1 bug 58028
Hi Mircea, two last nits: 2014-02-26 Tobias Grosser tob...@grosser.es Mircea Namolaru mircea.namol...@inria.fr Fix for bug 58028 Please write this as PR tree-optimization/58028 instead. This way, the commit triggers an update to the bugzilla bug. * graphite-clast-to-gimple.c (set_cloog_options): Don't remove scalar dimensions. Don't insert a line break after the colon, but continue on the same line, letting it wrap at pos. 75 (Emacs' fill-column). Thanks. Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University
Re: [PATCH,GRAPHiTE] Fix for P1 bug 58028
On 02/26/2014 03:30 PM, Mircea Namolaru wrote: This patch fixes the libgomp problems: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58028 2014-02-26 Tobies Grosser tob...@grosser.es Mircea Namolaru mnamo...@inria.fr Hi Mircea, the patch is correct. Fix for bug 58028 * graphite-clast-to-gimple.c (set_cloog_options): Don't remove scalar dimensions Index: gcc/graphite-clast-to-gimple.c === --- gcc/graphite-clast-to-gimple.c (revision 207298) +++ gcc/graphite-clast-to-gimple.c (working copy) @@ -1522,6 +1522,10 @@ variables. */ options-save_domains = 1; + /* Don't remove scalars dimensions - original number of dimensions required + for loop-can_be_parallel check. */ + options-noscalars = 1; Maybe we can provide a little bit more information. What about the following: Do not remove scalar dimensions. Cloog be default removes scalar dimensions very early from the input schedule. However, they are necessary to correctly derive from the saved domains (options-save_domains) the relationship between the generated loops and the schedule dimensions they are generated from. As this patch fixes a regression it should be fine at this stage. However, let's wait for the explicit OK from a release manager. Tobias Btw.: Do you have commit access?
Re: [PATCH,GRAPHITE] Fix for P1 bug 58028
On 02/26/2014 10:09 PM, Mircea Namolaru wrote: This patch fixes the libgomp problems: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58028 2014-02-26 Tobias Grosser tob...@grosser.es Mircea Namolaru mnamo...@inria.fr Fix for bug 58028 * graphite-clast-to-gimple.c (set_cloog_options): Don't remove scalar dimensions Index: gcc/graphite-clast-to-gimple.c === --- gcc/graphite-clast-to-gimple.c (revision 207298) +++ gcc/graphite-clast-to-gimple.c (working copy) @@ -1522,6 +1522,13 @@ variables. */ options-save_domains = 1; + /* Do not remove scalar dimensions. Cloog be default removes scalar + dimensions very early from the input schedule. However, they are + necessary to correctly derive from the saved domains + (options-save_domains) the relationship between the generated loops + and the schedule dimensions they are generated from. */ + options-noscalars = 1; + /* Disable optimizations and make cloog generate source code closer to the input. This is useful for debugging, but later we want the optimized code. Tested on x86-64 Linux, no regressions for c/c++/fortran. Got a notification failure from gcc-patches for the initial submission of this patch. However, Tobias (on cc) received it and already sent his comments, (see http://gcc.gnu.org/ml/gcc-patches/2014-02/msg01540.html), that are addressed in this re-submission. Yes, very nice. Thanks Mircea for reacting so quickly. This patch look good to me. @Release-managers: Is this patch OK for trunk? Cheers, Tobias
