Re: [patch] Fix PR tree-optimization/49471
2011/7/30 Zdenek Dvorak rakd...@kam.mff.cuni.cz: Hi, This patch fixes the build failure of cactusADM and dealII spec2006 benchmarks when autopar is enabled. (for powerpc they fail only when -m32 is additionally enabled) The problem originated in canonicalize_loop_ivs, where we iterate the header's phis in order to base all the induction variables on a single control variable. We use the largest precision of the loop's ivs in order to determine the type of the control variable. Since iterating the loop's phis takes into account not only the loop's ivs, but also reduction variables, we got precision values like 80 for x86, or 128 for ppc. The compilers failed to create proper types for these sizes (respectively). The proper behavior for determining the control variable's type is to take into account only the loop's ivs, which is what this patch does. Bootstrap and testsuite pass successfully (as autopar is not enabled by default). No new regressions when the testsuite is run with autopar enabled. No new regressions for the run of spec2006 with autopar enabled, cactusADM and dealII benchmarks now pass successfully with autopar on powerpc and x86. Thanks to Zdenek who helped me figure out the failure/fix. OK for trunk? It'll collide with Sebastians patch in that area. I suggested a INTEGRAL_TYPE_P check instead of the simple_iv one, it should be cheaper. Zdenek, do you think it will be incorrect in some cases? well, it does not make much sense -- reductions of integral type would be taken into consideration for determining the size of the canonical variable. However, it is not a big issue (the choice of the type is more or less arbitrary, as long as the number of iterations fits into it; selecting the type based on another existing iv is just to avoid unnecessary extensions), Hm, ok. Shouldn't we then simply choose a signed type that can hold niter based on the fact that we know this IV won't overflow? Choosing the biggest of all IVs precision looks indeed odd if we just need to count from zero to niter. Richard. Zdenek Thanks, Richard. Thanks, Razya ChangeLog: PR tree-optimization/49471 * tree-vect-loop-manip.c (canonicalize_loop_ivs): Add condition to ignore reduction variables when iterating the loop header's phis.
Re: [patch] Fix PR tree-optimization/49471
Hi, It'll collide with Sebastians patch in that area. I suggested a INTEGRAL_TYPE_P check instead of the simple_iv one, it should be cheaper. Zdenek, do you think it will be incorrect in some cases? well, it does not make much sense -- reductions of integral type would be taken into consideration for determining the size of the canonical variable. However, it is not a big issue (the choice of the type is more or less arbitrary, as long as the number of iterations fits into it; selecting the type based on another existing iv is just to avoid unnecessary extensions), Hm, ok. Shouldn't we then simply choose a signed type that can hold niter based on the fact that we know this IV won't overflow? Choosing the biggest of all IVs precision looks indeed odd if we just need to count from zero to niter. we often cannot use a signed type (as long as the initial value is zero), since we could not guarantee that it will not overflow (if the number of iterations is more than half of the range of the type), Zdenek
Re: [patch] Fix PR tree-optimization/49471
Hi, This patch fixes the build failure of cactusADM and dealII spec2006 benchmarks when autopar is enabled. (for powerpc they fail only when -m32 is additionally enabled) The problem originated in canonicalize_loop_ivs, where we iterate the header's phis in order to base all the induction variables on a single control variable. We use the largest precision of the loop's ivs in order to determine the type of the control variable. Since iterating the loop's phis takes into account not only the loop's ivs, but also reduction variables, we got precision values like 80 for x86, or 128 for ppc. The compilers failed to create proper types for these sizes (respectively). The proper behavior for determining the control variable's type is to take into account only the loop's ivs, which is what this patch does. Bootstrap and testsuite pass successfully (as autopar is not enabled by default). No new regressions when the testsuite is run with autopar enabled. No new regressions for the run of spec2006 with autopar enabled, cactusADM and dealII benchmarks now pass successfully with autopar on powerpc and x86. Thanks to Zdenek who helped me figure out the failure/fix. OK for trunk? It'll collide with Sebastians patch in that area. I suggested a INTEGRAL_TYPE_P check instead of the simple_iv one, it should be cheaper. Zdenek, do you think it will be incorrect in some cases? well, it does not make much sense -- reductions of integral type would be taken into consideration for determining the size of the canonical variable. However, it is not a big issue (the choice of the type is more or less arbitrary, as long as the number of iterations fits into it; selecting the type based on another existing iv is just to avoid unnecessary extensions), Zdenek Thanks, Richard. Thanks, Razya ChangeLog: PR tree-optimization/49471 * tree-vect-loop-manip.c (canonicalize_loop_ivs): Add condition to ignore reduction variables when iterating the loop header's phis.
Re: [patch] Fix PR tree-optimization/49471
On Tue, Jul 26, 2011 at 8:07 PM, Sebastian Pop seb...@gmail.com wrote: On Tue, Jul 26, 2011 at 08:30, Richard Guenther richard.guent...@gmail.com wrote: I suppose we also need to allow POINTER_TYPE_P here (but then treat it like an unsigned variable of the same width). Updated patch. Ok for trunk after regstrap? + uns = POINTER_TYPE_P (type) | TYPE_UNSIGNED (type); + unsigned_p = TYPE_PRECISION (type) precision ? uns : unsigned_p | uns; Um, operator precedence? Ok if written with an if stmt. Thanks, Richard. Thanks, Sebastian
Re: [patch] Fix PR tree-optimization/49471
Richard Guenther richard.guent...@gmail.com wrote on 25/07/2011 05:54:28 PM: From: Richard Guenther richard.guent...@gmail.com To: Razya Ladelsky/Haifa/IBM@IBMIL Cc: gcc-patches@gcc.gnu.org, Zdenek Dvorak rakd...@kam.mff.cuni.cz, Sebastian Pop s...@gcc.gnu.org Date: 25/07/2011 05:54 PM Subject: Re: [patch] Fix PR tree-optimization/49471 On Mon, Jul 25, 2011 at 4:47 PM, Razya Ladelsky ra...@il.ibm.com wrote: Hi, This patch fixes the build failure of cactusADM and dealII spec2006 benchmarks when autopar is enabled. (for powerpc they fail only when -m32 is additionally enabled) The problem originated in canonicalize_loop_ivs, where we iterate the header's phis in order to base all the induction variables on a single control variable. We use the largest precision of the loop's ivs in order to determine the type of the control variable. Since iterating the loop's phis takes into account not only the loop's ivs, but also reduction variables, we got precision values like 80 for x86, or 128 for ppc. The compilers failed to create proper types for these sizes (respectively). The proper behavior for determining the control variable's type is to take into account only the loop's ivs, which is what this patch does. Bootstrap and testsuite pass successfully (as autopar is not enabled by default). No new regressions when the testsuite is run with autopar enabled. No new regressions for the run of spec2006 with autopar enabled, cactusADM and dealII benchmarks now pass successfully with autopar on powerpc and x86. Thanks to Zdenek who helped me figure out the failure/fix. OK for trunk? It'll collide with Sebastians patch in that area. I suggested a INTEGRAL_TYPE_P check instead of the simple_iv one, it should be cheaper. Zdenek, do you think it will be incorrect in some cases? The INTEGRAL_TYPE_P check does work for cactusADM and dealII, but I'm not sure about the general case. Razya Thanks, Richard. Thanks, Razya ChangeLog: PR tree-optimization/49471 * tree-vect-loop-manip.c (canonicalize_loop_ivs): Add condition to ignore reduction variables when iterating the loop header's phis.
Re: [patch] Fix PR tree-optimization/49471
On Tue, Jul 26, 2011 at 2:59 PM, Razya Ladelsky ra...@il.ibm.com wrote: Richard Guenther richard.guent...@gmail.com wrote on 25/07/2011 05:54:28 PM: From: Richard Guenther richard.guent...@gmail.com To: Razya Ladelsky/Haifa/IBM@IBMIL Cc: gcc-patches@gcc.gnu.org, Zdenek Dvorak rakd...@kam.mff.cuni.cz, Sebastian Pop s...@gcc.gnu.org Date: 25/07/2011 05:54 PM Subject: Re: [patch] Fix PR tree-optimization/49471 On Mon, Jul 25, 2011 at 4:47 PM, Razya Ladelsky ra...@il.ibm.com wrote: Hi, This patch fixes the build failure of cactusADM and dealII spec2006 benchmarks when autopar is enabled. (for powerpc they fail only when -m32 is additionally enabled) The problem originated in canonicalize_loop_ivs, where we iterate the header's phis in order to base all the induction variables on a single control variable. We use the largest precision of the loop's ivs in order to determine the type of the control variable. Since iterating the loop's phis takes into account not only the loop's ivs, but also reduction variables, we got precision values like 80 for x86, or 128 for ppc. The compilers failed to create proper types for these sizes (respectively). The proper behavior for determining the control variable's type is to take into account only the loop's ivs, which is what this patch does. Bootstrap and testsuite pass successfully (as autopar is not enabled by default). No new regressions when the testsuite is run with autopar enabled. No new regressions for the run of spec2006 with autopar enabled, cactusADM and dealII benchmarks now pass successfully with autopar on powerpc and x86. Thanks to Zdenek who helped me figure out the failure/fix. OK for trunk? It'll collide with Sebastians patch in that area. I suggested a INTEGRAL_TYPE_P check instead of the simple_iv one, it should be cheaper. Zdenek, do you think it will be incorrect in some cases? The INTEGRAL_TYPE_P check does work for cactusADM and dealII, but I'm not sure about the general case. I suppose we also need to allow POINTER_TYPE_P here (but then treat it like an unsigned variable of the same width). Richard. Razya Thanks, Richard. Thanks, Razya ChangeLog: PR tree-optimization/49471 * tree-vect-loop-manip.c (canonicalize_loop_ivs): Add condition to ignore reduction variables when iterating the loop header's phis.
Re: [patch] Fix PR tree-optimization/49471
On Tue, Jul 26, 2011 at 08:30, Richard Guenther
richard.guent...@gmail.com wrote:
I suppose we also need to allow POINTER_TYPE_P here (but then
treat it like an unsigned variable of the same width).
Updated patch. Ok for trunk after regstrap?
Thanks,
Sebastian
From 3e8f8cfd0c4298b6b5e88c8bc7ba81a01e7cd815 Mon Sep 17 00:00:00 2001
From: Sebastian Pop seb...@gmail.com
Date: Sun, 24 Jul 2011 01:52:52 -0500
Subject: [PATCH] Fix PR49471: canonicalize_loop_ivs should not generate unsigned types.
2011-07-23 Sebastian Pop sebastian@amd.com
PR tree-optimization/49471
* tree-ssa-loop-manip.c (canonicalize_loop_ivs): Build an unsigned
iv only when the largest type is unsigned. Do not call
lang_hooks.types.type_for_size.
* testsuite/libgomp.graphite/force-parallel-1.c: Un-xfail.
* testsuite/libgomp.graphite/force-parallel-2.c: Adjust pattern.
---
gcc/ChangeLog |7 +++
gcc/tree-ssa-loop-manip.c | 19 ---
libgomp/ChangeLog |5 +
.../testsuite/libgomp.graphite/force-parallel-1.c |2 +-
.../testsuite/libgomp.graphite/force-parallel-2.c |2 +-
5 files changed, 30 insertions(+), 5 deletions(-)
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 27d4001..17358a8 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,5 +1,12 @@
2011-07-23 Sebastian Pop sebastian@amd.com
+ PR tree-optimization/49471
+ * tree-ssa-loop-manip.c (canonicalize_loop_ivs): Build an unsigned
+ iv only when the largest type is unsigned. Do not call
+ lang_hooks.types.type_for_size.
+
+2011-07-23 Sebastian Pop sebastian@amd.com
+
PR middle-end/47691
* graphite-clast-to-gimple.c (translate_clast_user): Update use of
copy_bb_and_scalar_dependences.
diff --git a/gcc/tree-ssa-loop-manip.c b/gcc/tree-ssa-loop-manip.c
index 8176ed8..f3392e6 100644
--- a/gcc/tree-ssa-loop-manip.c
+++ b/gcc/tree-ssa-loop-manip.c
@@ -1200,18 +1200,31 @@ canonicalize_loop_ivs (struct loop *loop, tree *nit, bool bump_in_latch)
gimple stmt;
edge exit = single_dom_exit (loop);
gimple_seq stmts;
+ enum machine_mode mode;
+ bool unsigned_p = false;
for (psi = gsi_start_phis (loop-header);
!gsi_end_p (psi); gsi_next (psi))
{
gimple phi = gsi_stmt (psi);
tree res = PHI_RESULT (phi);
+ bool uns;
- if (is_gimple_reg (res) TYPE_PRECISION (TREE_TYPE (res)) precision)
- precision = TYPE_PRECISION (TREE_TYPE (res));
+ type = TREE_TYPE (res);
+ if (!is_gimple_reg (res)
+ || (!INTEGRAL_TYPE_P (type)
+ !POINTER_TYPE_P (type))
+ || TYPE_PRECISION (type) precision)
+ continue;
+
+ uns = POINTER_TYPE_P (type) | TYPE_UNSIGNED (type);
+ unsigned_p = TYPE_PRECISION (type) precision ? uns : unsigned_p | uns;
+ precision = TYPE_PRECISION (type);
}
- type = lang_hooks.types.type_for_size (precision, 1);
+ mode = smallest_mode_for_size (precision, MODE_INT);
+ precision = GET_MODE_PRECISION (mode);
+ type = build_nonstandard_integer_type (precision, unsigned_p);
if (original_precision != precision)
{
diff --git a/libgomp/ChangeLog b/libgomp/ChangeLog
index 9225401..d5cd94d 100644
--- a/libgomp/ChangeLog
+++ b/libgomp/ChangeLog
@@ -1,3 +1,8 @@
+2011-07-23 Sebastian Pop sebastian@amd.com
+
+ * testsuite/libgomp.graphite/force-parallel-1.c: Un-xfail.
+ * testsuite/libgomp.graphite/force-parallel-2.c: Adjust pattern.
+
2011-07-18 Rainer Orth r...@cebitec.uni-bielefeld.de
PR target/49541
diff --git a/libgomp/testsuite/libgomp.graphite/force-parallel-1.c b/libgomp/testsuite/libgomp.graphite/force-parallel-1.c
index 71ed332..7f043d8 100644
--- a/libgomp/testsuite/libgomp.graphite/force-parallel-1.c
+++ b/libgomp/testsuite/libgomp.graphite/force-parallel-1.c
@@ -23,7 +23,7 @@ int main(void)
}
/* Check that parallel code generation part make the right answer. */
-/* { dg-final { scan-tree-dump-times 1 loops carried no dependency 2 graphite { xfail *-*-* } } } */
+/* { dg-final { scan-tree-dump-times 1 loops carried no dependency 2 graphite } } */
/* { dg-final { cleanup-tree-dump graphite } } */
/* { dg-final { scan-tree-dump-times loopfn 5 optimized } } */
/* { dg-final { cleanup-tree-dump parloops } } */
diff --git a/libgomp/testsuite/libgomp.graphite/force-parallel-2.c b/libgomp/testsuite/libgomp.graphite/force-parallel-2.c
index 1ce0feb..03d8236 100644
--- a/libgomp/testsuite/libgomp.graphite/force-parallel-2.c
+++ b/libgomp/testsuite/libgomp.graphite/force-parallel-2.c
@@ -23,7 +23,7 @@ int main(void)
}
/* Check that parallel code generation part make the right answer. */
-/* { dg-final { scan-tree-dump-times 2 loops carried no dependency 1 graphite } } */
+/* { dg-final { scan-tree-dump-times 2 loops carried no dependency 2 graphite } } */
/* { dg-final { cleanup-tree-dump graphite } } */
/* { dg-final { scan-tree-dump-times loopfn 5 optimized } } */
/* { dg-final { cleanup-tree-dump
Re: [patch] Fix PR tree-optimization/49471
Razya Ladelsky/Haifa/IBM wrote on 25/07/2011 05:44:02 PM: From: Razya Ladelsky/Haifa/IBM To: gcc-patches@gcc.gnu.org Cc: Zdenek Dvorak rakd...@kam.mff.cuni.cz, Richard Guenther richard.guent...@gmail.com Date: 25/07/2011 05:44 PM Subject: [patch] Fix PR tree-optimization/49471 Hi, This patch fixes the build failure of cactusADM and dealII spec2006 benchmarks when autopar is enabled. (for powerpc they fail only when -m32 is additionally enabled) The problem originated in canonicalize_loop_ivs, where we iterate the header's phis in order to base all the induction variables on a single control variable. We use the largest precision of the loop's ivs in order to determine the type of the control variable. Since iterating the loop's phis takes into account not only the loop's ivs, but also reduction variables, we got precision values like 80 for x86, or 128 for ppc. The compilers failed to create proper types for these sizes (respectively). The proper behavior for determining the control variable's type is to take into account only the loop's ivs, which is what this patch does. Bootstrap and testsuite pass successfully (as autopar is not enabled by default). No new regressions when the testsuite is run with autopar enabled. No new regressions for the run of spec2006 with autopar enabled, cactusADM and dealII benchmarks now pass successfully with autopar on powerpc and x86. Thanks to Zdenek who helped me figure out the failure/fix. OK for trunk? Thanks, Razya ChangeLog: PR tree-optimization/49471 * tree-vect-loop-manip.c (canonicalize_loop_ivs): Add condition to ignore reduction variables when iterating the loop header's phis. I have an error in the ChangeLog: the change is in tree-ssa-loop-manip.c instead of tree-vect-loop-manip.c Sorry, Razya [attachment cactus_dealII_patch.txt deleted by Razya Ladelsky/Haifa/IBM]
