| Message ID | 9fc51a58cc354a934ff5a5e2b232dae520090e2b.1459485314.git.segher@kernel.crashing.org |
|---|---|
| State | New |
| Headers | show |
On Fri, Apr 1, 2016 at 6:54 AM, Segher Boessenkool <segher@kernel.crashing.org> wrote: > Sometimes people write loops that they do not want optimized away, even > when the compiler can replace those loops by a simple expression (or > nothing). For such people, this patch adds a compiler option. > > Bootstrapped on powerpc64-linux; regression check still in progress > (with Init(1) to actually test anything). -fno-tree-scev-cprop? -O0? A new compiler option for this is complete overkill (and it's implementation is gross ;)). Semantics are also unclear, your patch would only make sure to preserve an empty loop with the asm in the latch, it wouldn't disallow replacing the overall effect with a computation. Your patch would also miss a few testcases. Richard. > > Segher > > > 2016-04-01 Segher Boessenkool <segher@kernel.crashing.org> > > * loop-init.c: Include some more stuff that really doesn't belong > here, oh well. > (loop_optimizer_init): Add empty asm statements in all gimple loops, > if asked to. > * common.opt: Add new option. > > --- > gcc/common.opt | 4 ++++ > gcc/loop-init.c | 15 +++++++++++++++ > 2 files changed, 19 insertions(+) > > diff --git a/gcc/loop-init.c b/gcc/loop-init.c > index 8634591..7c5dc24 100644 > --- a/gcc/loop-init.c > +++ b/gcc/loop-init.c > @@ -24,6 +24,8 @@ along with GCC; see the file COPYING3. If not see > #include "target.h" > #include "rtl.h" > #include "tree.h" > +#include "gimple.h" > +#include "gimple-iterator.h" > #include "cfghooks.h" > #include "df.h" > #include "regs.h" > @@ -91,6 +93,19 @@ loop_optimizer_init (unsigned flags) > > /* Find the loops. */ > current_loops = flow_loops_find (NULL); > + > + if (flag_never_gonna_give_you_up && current_ir_type () == IR_GIMPLE) > + { > + struct loop *loop; > + FOR_EACH_LOOP (loop, 0) > + if (loop->latch) > + { > + gasm *p = gimple_build_asm_vec ("", 0, 0, 0, 0); > + gimple_asm_set_volatile (p, true); > + gimple_stmt_iterator bsi = gsi_after_labels (loop->latch); > + gsi_insert_before (&bsi, p, GSI_SAME_STMT); > + } > + } > } > else > { > diff --git a/gcc/common.opt b/gcc/common.opt > index 0f3bb4e..b7c0a6a 100644 > --- a/gcc/common.opt > +++ b/gcc/common.opt > @@ -2002,6 +2002,10 @@ frerun-loop-opt > Common Ignore > Does nothing. Preserved for backward compatibility. > > +frickroll-all-loops > +Common Report Var(flag_never_gonna_give_you_up) Init(0) Optimization > +You know the rules, and so do I. > + > frounding-math > Common Report Var(flag_rounding_math) Optimization SetByCombined > Disable optimizations that assume default FP rounding behavior. > -- > 1.9.3 >
On Fri, Apr 01, 2016 at 09:36:49AM +0200, Richard Biener wrote: > On Fri, Apr 1, 2016 at 6:54 AM, Segher Boessenkool > <segher@kernel.crashing.org> wrote: > > Sometimes people write loops that they do not want optimized away, even > > when the compiler can replace those loops by a simple expression (or > > nothing). For such people, this patch adds a compiler option. > > > > Bootstrapped on powerpc64-linux; regression check still in progress > > (with Init(1) to actually test anything). > > -fno-tree-scev-cprop? -O0? There are other cases where GCC can delete loops, for example cddce1. > A new compiler option for this is complete overkill (and it's implementation > is gross ;)). Semantics are also unclear, your patch would only make sure > to preserve an empty loop with the asm in the latch, it wouldn't disallow > replacing the overall effect with a computation. That's right, and the loop can even still be unrolled, even fully unrolled (which is good, not only should we not desert the loop but we also shouldn't run around so much). > Your patch would also miss a few testcases. It already makes ~2000 (mainly vectorisation) testcases fail, is that not enough coverage? :-) Cheers, Segher
On Thu, Mar 31, 2016 at 9:54 PM, Segher Boessenkool <segher@kernel.crashing.org> wrote: > Sometimes people write loops that they do not want optimized away, even > when the compiler can replace those loops by a simple expression (or > nothing). For such people, this patch adds a compiler option. The Linux kernel has a nice workaround for this case, at least for the divide case. Thanks, ANdrew > > Bootstrapped on powerpc64-linux; regression check still in progress > (with Init(1) to actually test anything). > > > Segher > > > 2016-04-01 Segher Boessenkool <segher@kernel.crashing.org> > > * loop-init.c: Include some more stuff that really doesn't belong > here, oh well. > (loop_optimizer_init): Add empty asm statements in all gimple loops, > if asked to. > * common.opt: Add new option. > > --- > gcc/common.opt | 4 ++++ > gcc/loop-init.c | 15 +++++++++++++++ > 2 files changed, 19 insertions(+) > > diff --git a/gcc/loop-init.c b/gcc/loop-init.c > index 8634591..7c5dc24 100644 > --- a/gcc/loop-init.c > +++ b/gcc/loop-init.c > @@ -24,6 +24,8 @@ along with GCC; see the file COPYING3. If not see > #include "target.h" > #include "rtl.h" > #include "tree.h" > +#include "gimple.h" > +#include "gimple-iterator.h" > #include "cfghooks.h" > #include "df.h" > #include "regs.h" > @@ -91,6 +93,19 @@ loop_optimizer_init (unsigned flags) > > /* Find the loops. */ > current_loops = flow_loops_find (NULL); > + > + if (flag_never_gonna_give_you_up && current_ir_type () == IR_GIMPLE) > + { > + struct loop *loop; > + FOR_EACH_LOOP (loop, 0) > + if (loop->latch) > + { > + gasm *p = gimple_build_asm_vec ("", 0, 0, 0, 0); > + gimple_asm_set_volatile (p, true); > + gimple_stmt_iterator bsi = gsi_after_labels (loop->latch); > + gsi_insert_before (&bsi, p, GSI_SAME_STMT); > + } > + } > } > else > { > diff --git a/gcc/common.opt b/gcc/common.opt > index 0f3bb4e..b7c0a6a 100644 > --- a/gcc/common.opt > +++ b/gcc/common.opt > @@ -2002,6 +2002,10 @@ frerun-loop-opt > Common Ignore > Does nothing. Preserved for backward compatibility. > > +frickroll-all-loops > +Common Report Var(flag_never_gonna_give_you_up) Init(0) Optimization > +You know the rules, and so do I. > + > frounding-math > Common Report Var(flag_rounding_math) Optimization SetByCombined > Disable optimizations that assume default FP rounding behavior. > -- > 1.9.3 >
On Fri, Apr 01, 2016 at 08:32:28AM -0700, Andrew Pinski wrote: > On Thu, Mar 31, 2016 at 9:54 PM, Segher Boessenkool > <segher@kernel.crashing.org> wrote: > > Sometimes people write loops that they do not want optimized away, even > > when the compiler can replace those loops by a simple expression (or > > nothing). For such people, this patch adds a compiler option. > > The Linux kernel has a nice workaround for this case, at least for the > divide case. I know, I wrote that code :-) It has a slightly different purpose (and semantics) though: it makes _sure_ that "inside, we know what is going on" does not hold (that's what "+rm" does). But yes, it is playing a similar game. Segher
On April 1, 2016 5:18:10 PM GMT+02:00, Segher Boessenkool <segher@kernel.crashing.org> wrote: >On Fri, Apr 01, 2016 at 09:36:49AM +0200, Richard Biener wrote: >> On Fri, Apr 1, 2016 at 6:54 AM, Segher Boessenkool >> <segher@kernel.crashing.org> wrote: >> > Sometimes people write loops that they do not want optimized away, >even >> > when the compiler can replace those loops by a simple expression >(or >> > nothing). For such people, this patch adds a compiler option. >> > >> > Bootstrapped on powerpc64-linux; regression check still in progress >> > (with Init(1) to actually test anything). >> >> -fno-tree-scev-cprop? -O0? > >There are other cases where GCC can delete loops, for example cddce1. > >> A new compiler option for this is complete overkill (and it's >implementation >> is gross ;)). Semantics are also unclear, your patch would only make >sure >> to preserve an empty loop with the asm in the latch, it wouldn't >disallow >> replacing the overall effect with a computation. > >That's right, and the loop can even still be unrolled, even fully >unrolled (which is good, not only should we not desert the loop but >we also shouldn't run around so much). > >> Your patch would also miss a few testcases. > >It already makes ~2000 (mainly vectorisation) testcases fail, is that >not enough coverage? :-) For an April 1st joke yes, otherwise no :) Richard. >Cheers, > > >Segher
diff --git a/gcc/loop-init.c b/gcc/loop-init.c index 8634591..7c5dc24 100644 --- a/gcc/loop-init.c +++ b/gcc/loop-init.c @@ -24,6 +24,8 @@ along with GCC; see the file COPYING3. If not see #include "target.h" #include "rtl.h" #include "tree.h" +#include "gimple.h" +#include "gimple-iterator.h" #include "cfghooks.h" #include "df.h" #include "regs.h" @@ -91,6 +93,19 @@ loop_optimizer_init (unsigned flags) /* Find the loops. */ current_loops = flow_loops_find (NULL); + + if (flag_never_gonna_give_you_up && current_ir_type () == IR_GIMPLE) + { + struct loop *loop; + FOR_EACH_LOOP (loop, 0) + if (loop->latch) + { + gasm *p = gimple_build_asm_vec ("", 0, 0, 0, 0); + gimple_asm_set_volatile (p, true); + gimple_stmt_iterator bsi = gsi_after_labels (loop->latch); + gsi_insert_before (&bsi, p, GSI_SAME_STMT); + } + } } else { diff --git a/gcc/common.opt b/gcc/common.opt index 0f3bb4e..b7c0a6a 100644 --- a/gcc/common.opt +++ b/gcc/common.opt @@ -2002,6 +2002,10 @@ frerun-loop-opt Common Ignore Does nothing. Preserved for backward compatibility. +frickroll-all-loops +Common Report Var(flag_never_gonna_give_you_up) Init(0) Optimization +You know the rules, and so do I. + frounding-math Common Report Var(flag_rounding_math) Optimization SetByCombined Disable optimizations that assume default FP rounding behavior.