| Message ID | CAFk3UF_+jn6bYJq+c6j88vU=K+g9w1nvtNzWcjxWF1qN9go1bg@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On Sun, 24 Jul 2011, Sebastian Pop wrote: > On Sun, Jul 24, 2011 at 05:59, Richard Guenther > <richard.guenther@gmail.com> wrote: > > For two IVs with the same precision, one signed and one unsigned you choose > > signedness of the canonical IV based on the random order of PHIs - that doesn't > > look correct. > > > > I think what you should do here is use an unsigned type if any of the IVs with > > the current max precision is unsigned. > > Attached is the updated patch. Bootstrapped and tested on amd64-linux. > Ok for trunk? for (psi = gsi_start_phis (loop->header); !gsi_end_p (psi); gsi_next (&psi)) @@ -1207,11 +1209,25 @@ canonicalize_loop_ivs (struct loop *loop, tree *nit, bool bump_in_latch) gimple phi = gsi_stmt (psi); tree res = PHI_RESULT (phi); - if (is_gimple_reg (res) && TYPE_PRECISION (TREE_TYPE (res)) > precision) - precision = TYPE_PRECISION (TREE_TYPE (res)); + if (!is_gimple_reg (res)) + continue; + + type = TREE_TYPE (res); + if (TYPE_PRECISION (type) > precision) + { + precision = TYPE_PRECISION (type); + unsigned_p = TYPE_UNSIGNED (type); + continue; + } + + if (TYPE_PRECISION (type) == precision + && TYPE_UNSIGNED (type)) + unsigned_p = true; } I think we also need to care for non-integral PHIs where TYPE_PRECISION and TYPE_UNSIGNED are not applicable (seems the original code is also buggy here?). So, sth like type = TREE_TYPE (res); if (!is_gimple_reg (res) || !INTEGRAL_TYPE_P (type) || TYPE_PRECISION (type) < precision) continue; precision = TYPE_PRECISION (type); unsigned_p |= TYPE_UNSIGNED (type); } would be ok. Ok with that change. Thanks, Richard. > > Thanks, > Sebastian >
On Mon, Jul 25, 2011 at 03:41, Richard Guenther <rguenther@suse.de> wrote: > I think we also need to care for non-integral PHIs where TYPE_PRECISION > and TYPE_UNSIGNED are not applicable (seems the original code is also > buggy here?). So, sth like > > type = TREE_TYPE (res); > if (!is_gimple_reg (res) > || !INTEGRAL_TYPE_P (type) > || TYPE_PRECISION (type) < precision) > continue; > > precision = TYPE_PRECISION (type); > unsigned_p |= TYPE_UNSIGNED (type); > } > This would not work optimally on the following sequence: unsigned char signed short as we would set the unsigned_p to true for the "unsigned char" and then we would not reset the value of unsigned_p when the precision increases. So what about doing this instead: type = TREE_TYPE (res); if (!is_gimple_reg (res) || !INTEGRAL_TYPE_P (type) || TYPE_PRECISION (type) < precision) continue; if (TYPE_PRECISION (type) > precision) unsigned_p = TYPE_UNSIGNED (type); else unsigned_p |= TYPE_UNSIGNED (type); precision = TYPE_PRECISION (type); Thanks, Sebastian
On Mon, Jul 25, 2011 at 4:56 PM, Sebastian Pop <sebpop@gmail.com> wrote: > On Mon, Jul 25, 2011 at 03:41, Richard Guenther <rguenther@suse.de> wrote: >> I think we also need to care for non-integral PHIs where TYPE_PRECISION >> and TYPE_UNSIGNED are not applicable (seems the original code is also >> buggy here?). So, sth like >> >> type = TREE_TYPE (res); >> if (!is_gimple_reg (res) >> || !INTEGRAL_TYPE_P (type) >> || TYPE_PRECISION (type) < precision) >> continue; >> >> precision = TYPE_PRECISION (type); >> unsigned_p |= TYPE_UNSIGNED (type); >> } >> > > This would not work optimally on the following sequence: > unsigned char > signed short > as we would set the unsigned_p to true for the "unsigned char" and > then we would not reset the value of unsigned_p when the precision > increases. So what about doing this instead: > > type = TREE_TYPE (res); > if (!is_gimple_reg (res) > || !INTEGRAL_TYPE_P (type) > || TYPE_PRECISION (type) < precision) > continue; > > if (TYPE_PRECISION (type) > precision) > unsigned_p = TYPE_UNSIGNED (type); > else > unsigned_p |= TYPE_UNSIGNED (type); > > precision = TYPE_PRECISION (type); Ah, indeed. Yes, fine with me. Thanks, Richard. > Thanks, > Sebastian >
From eab3e096781b0df1bda7129108eddbc73ebbfcd0 Mon Sep 17 00:00:00 2001 From: Sebastian Pop <sebpop@gmail.com> Date: Sun, 24 Jul 2011 01:52:52 -0500 Subject: [PATCH 1/2] canonicalize_loop_ivs should not generate unsigned types. 2011-07-23 Sebastian Pop <sebastian.pop@amd.com> * 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 | 6 +++++ gcc/tree-ssa-loop-manip.c | 22 +++++++++++++++++-- libgomp/ChangeLog | 5 ++++ .../testsuite/libgomp.graphite/force-parallel-1.c | 2 +- .../testsuite/libgomp.graphite/force-parallel-2.c | 2 +- 5 files changed, 32 insertions(+), 5 deletions(-) diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 31cd954..cd7c4b0 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,5 +1,11 @@ 2011-07-23 Sebastian Pop <sebastian.pop@amd.com> + * 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.pop@amd.com> + PR middle-end/47653 * graphite-scop-detection.c (graphite_can_represent_loop): Discard loops using wrapping semantics. diff --git a/gcc/tree-ssa-loop-manip.c b/gcc/tree-ssa-loop-manip.c index 8176ed8..32082cf 100644 --- a/gcc/tree-ssa-loop-manip.c +++ b/gcc/tree-ssa-loop-manip.c @@ -1200,6 +1200,8 @@ 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)) @@ -1207,11 +1209,25 @@ canonicalize_loop_ivs (struct loop *loop, tree *nit, bool bump_in_latch) gimple phi = gsi_stmt (psi); tree res = PHI_RESULT (phi); - if (is_gimple_reg (res) && TYPE_PRECISION (TREE_TYPE (res)) > precision) - precision = TYPE_PRECISION (TREE_TYPE (res)); + if (!is_gimple_reg (res)) + continue; + + type = TREE_TYPE (res); + if (TYPE_PRECISION (type) > precision) + { + precision = TYPE_PRECISION (type); + unsigned_p = TYPE_UNSIGNED (type); + continue; + } + + if (TYPE_PRECISION (type) == precision + && TYPE_UNSIGNED (type)) + unsigned_p = true; } - 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.pop@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 <ro@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 "parloops" } } */ -- 1.7.4.1