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