* [PATCH] Fix PR56466
@ 2013-02-27 14:48 Marek Polacek
2013-02-27 15:21 ` Richard Biener
2013-02-28 5:32 ` Miles Bader
0 siblings, 2 replies; 4+ messages in thread
From: Marek Polacek @ 2013-02-27 14:48 UTC (permalink / raw)
To: GCC Patches; +Cc: Richard Biener
This patch does two things: it fixes PR56466 by calling fix_loop_structure
in case we're changing a loop, and secondly, it makes verifiyng
loops in the unroller cheaper: there's no need to verify each loop,
we can do it once after all transformations (verify_loop_structure is
being called via fix_loop_structure).
Regtested/bootstrapped on x86_64-linux, ok for trunk?
2013-02-27 Marek Polacek <polacek@redhat.com>
PR rtl-optimization/56466
* loop-unroll.c (unroll_and_peel_loops): Call fix_loop_structure
if we're changing a loop.
(peel_loops_completely): Likewise.
* gcc.dg/pr56466.c: New test.
--- gcc/loop-unroll.c.mp 2013-02-27 11:13:01.915430193 +0100
+++ gcc/loop-unroll.c 2013-02-27 14:57:09.873168342 +0100
@@ -207,7 +207,7 @@ void
unroll_and_peel_loops (int flags)
{
struct loop *loop;
- bool check;
+ bool changed = false;
loop_iterator li;
/* First perform complete loop peeling (it is almost surely a win,
@@ -220,7 +220,6 @@ unroll_and_peel_loops (int flags)
/* Scan the loops, inner ones first. */
FOR_EACH_LOOP (li, loop, LI_FROM_INNERMOST)
{
- check = true;
/* And perform the appropriate transformations. */
switch (loop->lpt_decision.decision)
{
@@ -229,30 +228,33 @@ unroll_and_peel_loops (int flags)
gcc_unreachable ();
case LPT_PEEL_SIMPLE:
peel_loop_simple (loop);
+ changed |= true;
break;
case LPT_UNROLL_CONSTANT:
unroll_loop_constant_iterations (loop);
+ changed |= true;
break;
case LPT_UNROLL_RUNTIME:
unroll_loop_runtime_iterations (loop);
+ changed |= true;
break;
case LPT_UNROLL_STUPID:
unroll_loop_stupid (loop);
+ changed |= true;
break;
case LPT_NONE:
- check = false;
break;
default:
gcc_unreachable ();
}
- if (check)
- {
-#ifdef ENABLE_CHECKING
- verify_loop_structure ();
-#endif
- }
}
+ if (changed)
+ {
+ calculate_dominance_info (CDI_DOMINATORS);
+ fix_loop_structure (NULL);
+ }
+
iv_analysis_done ();
}
@@ -283,6 +285,7 @@ peel_loops_completely (int flags)
{
struct loop *loop;
loop_iterator li;
+ bool changed = false;
/* Scan the loops, the inner ones first. */
FOR_EACH_LOOP (li, loop, LI_FROM_INNERMOST)
@@ -306,11 +309,15 @@ peel_loops_completely (int flags)
{
report_unroll_peel (loop, locus);
peel_loop_completely (loop);
-#ifdef ENABLE_CHECKING
- verify_loop_structure ();
-#endif
+ changed = true;
}
}
+
+ if (changed)
+ {
+ calculate_dominance_info (CDI_DOMINATORS);
+ fix_loop_structure (NULL);
+ }
}
/* Decide whether unroll or peel loops (depending on FLAGS) and how much. */
--- gcc/testsuite/gcc.dg/pr56466.c.mp 2013-02-27 12:43:44.468886756 +0100
+++ gcc/testsuite/gcc.dg/pr56466.c 2013-02-27 12:46:36.385390735 +0100
@@ -0,0 +1,31 @@
+/* PR rtl-optimization/56466 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -w -funroll-loops" } */
+
+int a, b, c;
+
+void
+f (void)
+{
+ for (; b; b++)
+ {
+ if (0)
+ for (; b < 0; b++)
+ if (1 % 0)
+ {
+ while (1)
+ {
+ a = 0;
+ lbl1:
+ c++;
+ }
+ lbl2:
+ ;
+ }
+
+ goto lbl1;
+ }
+
+ a = 0;
+ goto lbl2;
+}
Marek
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Fix PR56466
2013-02-27 14:48 [PATCH] Fix PR56466 Marek Polacek
@ 2013-02-27 15:21 ` Richard Biener
2013-02-28 5:32 ` Miles Bader
1 sibling, 0 replies; 4+ messages in thread
From: Richard Biener @ 2013-02-27 15:21 UTC (permalink / raw)
To: Marek Polacek; +Cc: GCC Patches
On Wed, 27 Feb 2013, Marek Polacek wrote:
> This patch does two things: it fixes PR56466 by calling fix_loop_structure
> in case we're changing a loop, and secondly, it makes verifiyng
> loops in the unroller cheaper: there's no need to verify each loop,
> we can do it once after all transformations (verify_loop_structure is
> being called via fix_loop_structure).
>
> Regtested/bootstrapped on x86_64-linux, ok for trunk?
Ok.
Thanks,
Richard.
> 2013-02-27 Marek Polacek <polacek@redhat.com>
>
> PR rtl-optimization/56466
> * loop-unroll.c (unroll_and_peel_loops): Call fix_loop_structure
> if we're changing a loop.
> (peel_loops_completely): Likewise.
>
> * gcc.dg/pr56466.c: New test.
>
> --- gcc/loop-unroll.c.mp 2013-02-27 11:13:01.915430193 +0100
> +++ gcc/loop-unroll.c 2013-02-27 14:57:09.873168342 +0100
> @@ -207,7 +207,7 @@ void
> unroll_and_peel_loops (int flags)
> {
> struct loop *loop;
> - bool check;
> + bool changed = false;
> loop_iterator li;
>
> /* First perform complete loop peeling (it is almost surely a win,
> @@ -220,7 +220,6 @@ unroll_and_peel_loops (int flags)
> /* Scan the loops, inner ones first. */
> FOR_EACH_LOOP (li, loop, LI_FROM_INNERMOST)
> {
> - check = true;
> /* And perform the appropriate transformations. */
> switch (loop->lpt_decision.decision)
> {
> @@ -229,30 +228,33 @@ unroll_and_peel_loops (int flags)
> gcc_unreachable ();
> case LPT_PEEL_SIMPLE:
> peel_loop_simple (loop);
> + changed |= true;
> break;
> case LPT_UNROLL_CONSTANT:
> unroll_loop_constant_iterations (loop);
> + changed |= true;
> break;
> case LPT_UNROLL_RUNTIME:
> unroll_loop_runtime_iterations (loop);
> + changed |= true;
> break;
> case LPT_UNROLL_STUPID:
> unroll_loop_stupid (loop);
> + changed |= true;
> break;
> case LPT_NONE:
> - check = false;
> break;
> default:
> gcc_unreachable ();
> }
> - if (check)
> - {
> -#ifdef ENABLE_CHECKING
> - verify_loop_structure ();
> -#endif
> - }
> }
>
> + if (changed)
> + {
> + calculate_dominance_info (CDI_DOMINATORS);
> + fix_loop_structure (NULL);
> + }
> +
> iv_analysis_done ();
> }
>
> @@ -283,6 +285,7 @@ peel_loops_completely (int flags)
> {
> struct loop *loop;
> loop_iterator li;
> + bool changed = false;
>
> /* Scan the loops, the inner ones first. */
> FOR_EACH_LOOP (li, loop, LI_FROM_INNERMOST)
> @@ -306,11 +309,15 @@ peel_loops_completely (int flags)
> {
> report_unroll_peel (loop, locus);
> peel_loop_completely (loop);
> -#ifdef ENABLE_CHECKING
> - verify_loop_structure ();
> -#endif
> + changed = true;
> }
> }
> +
> + if (changed)
> + {
> + calculate_dominance_info (CDI_DOMINATORS);
> + fix_loop_structure (NULL);
> + }
> }
>
> /* Decide whether unroll or peel loops (depending on FLAGS) and how much. */
> --- gcc/testsuite/gcc.dg/pr56466.c.mp 2013-02-27 12:43:44.468886756 +0100
> +++ gcc/testsuite/gcc.dg/pr56466.c 2013-02-27 12:46:36.385390735 +0100
> @@ -0,0 +1,31 @@
> +/* PR rtl-optimization/56466 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -w -funroll-loops" } */
> +
> +int a, b, c;
> +
> +void
> +f (void)
> +{
> + for (; b; b++)
> + {
> + if (0)
> + for (; b < 0; b++)
> + if (1 % 0)
> + {
> + while (1)
> + {
> + a = 0;
> + lbl1:
> + c++;
> + }
> + lbl2:
> + ;
> + }
> +
> + goto lbl1;
> + }
> +
> + a = 0;
> + goto lbl2;
> +}
>
> Marek
>
>
--
Richard Biener <rguenther@suse.de>
SUSE / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
GF: Jeff Hawn, Jennifer Guild, Felix Imend
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Fix PR56466
2013-02-27 14:48 [PATCH] Fix PR56466 Marek Polacek
2013-02-27 15:21 ` Richard Biener
@ 2013-02-28 5:32 ` Miles Bader
2013-02-28 9:49 ` Marek Polacek
1 sibling, 1 reply; 4+ messages in thread
From: Miles Bader @ 2013-02-28 5:32 UTC (permalink / raw)
To: Marek Polacek; +Cc: GCC Patches, Richard Biener
Marek Polacek <polacek@redhat.com> writes:
> + bool changed = false;
> + changed |= true;
> + changed |= true;
> + changed |= true;
> + changed |= true;
> + if (changed)
Why do you use "|=" ...? Isn't it equivalent to just "=" (which is
more clear) for a boolean?
Thanks,
-miles
--
永日の 澄んだ紺から 永遠へ
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Fix PR56466
2013-02-28 5:32 ` Miles Bader
@ 2013-02-28 9:49 ` Marek Polacek
0 siblings, 0 replies; 4+ messages in thread
From: Marek Polacek @ 2013-02-28 9:49 UTC (permalink / raw)
To: Miles Bader; +Cc: GCC Patches, Richard Biener
On Thu, Feb 28, 2013 at 02:32:02PM +0900, Miles Bader wrote:
> Marek Polacek <polacek@redhat.com> writes:
> > + bool changed = false;
> > + changed |= true;
> > + changed |= true;
> > + changed |= true;
> > + changed |= true;
> > + if (changed)
>
> Why do you use "|=" ...? Isn't it equivalent to just "=" (which is
> more clear) for a boolean?
Ah, yeah, it's a remnant; I had "changed = false;" there as well.
Will adjust, thanks,
Marek
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-02-28 9:49 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-02-27 14:48 [PATCH] Fix PR56466 Marek Polacek
2013-02-27 15:21 ` Richard Biener
2013-02-28 5:32 ` Miles Bader
2013-02-28 9:49 ` Marek Polacek
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).