This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] Fix ifcombine (PR tree-optimization/70586)


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, ok for trunk?

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  <jakub@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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]