On April 8, 2016 7:16:48 PM GMT+02:00, Jakub Jelinek <ja...@redhat.com> wrote:
>Hi!
>
>The following testcase is miscompiled by tree-ssa-ifcombine.c, because
>it sees:
> if (_5 != 0)
> goto <bb 5>;
> else
> goto <bb 8>;
>
> <bb 5>:
> iftmp.0_12 = foo.part.0 (_11, _5);
> _14 = iftmp.0_12 > 0;
> _15 = (int) _14;
> if (_5 != 0)
> goto <bb 6>;
> else
> goto <bb 8>;
>and bb_no_side_effects_p says that bb5 has no side effects (which is
>incorrect) and thus optimizes it into unconditionally performing
>bb5 and only then branching.
>bb_no_side_effects_p carefully looks for various side-effects,
>including
>trap possibilities (floating point exceptions, possible integer
>division by zero), but accepts const calls (pure calls are not accepted
>because of gimple_vuse (stmt) check, other calls because of
>gimple_has_side_effects (stmt)), but my understanding is that even
>const
>calls can perform floating point arithmetics, or can contain potential
>division by zero, etc., it just shouldn't throw exceptions, affect
>global
>state etc. At least lots of const functions contain floating point
>arith,
>and our const/pure discovery also doesn't contain anything that would
>look for gimple_could_trap_p.
>Therefore IMHO it is undesirable to move const calls before their
>guarding
>condition. In the testcase, we end up with simplified:
>void foo (int x) { ... something % x; ... }
>...
> if (x)
> y = foo (x);
>and happily move the foo (x) call before the if (x) condition guarding
>it.
>
>So IMHO we should just punt on all calls in the bb,
>bootstrapped/regtested
>on x86_64-linux and i686-linux, o
Hmm, I think this means GIMPLE_has_side_effects is to be fixed then. Note that
honza had plans to compute things like 'uses FP' and 'contains arith with
undefined overflow' and propagate that alongside pure/const-ness.
Can you try to asses the impact of fixing no-side-effects?
Richard.
>Stage 1 material, if the two conditions are actually the same, it
>really
>would be better if it kept the first (outer) condition and removed the
>second (inner) condition, but that is quite a special case, the code is
>written primarily for the case when the two conditions are actually
>different.
>
>2016-04-08 Jakub Jelinek <ja...@redhat.com>
>
> PR tree-optimization/70586
> * tree-ssa-ifcombine.c (bb_no_side_effects_p): Return false
> for any calls.
>
> * gcc.c-torture/execute/pr70586.c: New test.
>
>--- gcc/tree-ssa-ifcombine.c.jj 2016-01-04 14:55:52.000000000 +0100
>+++ gcc/tree-ssa-ifcombine.c 2016-04-08 16:25:26.107238727 +0200
>@@ -125,7 +125,12 @@ bb_no_side_effects_p (basic_block bb)
> if (gimple_has_side_effects (stmt)
> || gimple_uses_undefined_value_p (stmt)
> || gimple_could_trap_p (stmt)
>- || gimple_vuse (stmt))
>+ || gimple_vuse (stmt)
>+ /* const calls don't match any of the above, yet they could
>+ still have some side-effects - they could contain
>+ gimple_could_trap_p statements, like floating point
>+ exceptions or integer division by zero. See PR70586. */
>+ || is_gimple_call (stmt))
> return false;
> }
>
>--- gcc/testsuite/gcc.c-torture/execute/pr70586.c.jj 2016-04-08
>16:29:25.417933533 +0200
>+++ gcc/testsuite/gcc.c-torture/execute/pr70586.c 2016-04-08
>16:29:06.000000000 +0200
>@@ -0,0 +1,30 @@
>+/* PR tree-optimization/70586 */
>+
>+int a, e, f;
>+short b, c, d;
>+
>+int
>+foo (int x, int y)
>+{
>+ return (y == 0 || (x && y == 1)) ? x : x % y;
>+}
>+
>+static short
>+bar (void)
>+{
>+ int i = foo (c, f);
>+ f = foo (d, 2);
>+ int g = foo (b, c);
>+ int h = foo (g > 0, c);
>+ c = (3 >= h ^ 7) <= foo (i, c);
>+ if (foo (e, 1))
>+ return a;
>+ return 0;
>+}
>+
>+int
>+main ()
>+{
>+ bar ();
>+ return 0;
>+}
>
> Jakub
