Hi!
As mentioned in the PR, maybe_optimize_range_tests considers basic blocks
with not just the final GIMPLE_COND (or for last_bb store feeding into PHI),
but also assign stmts that don't trap, don't have side-effects and where
the SSA_NAMEs they set are used only in their own bb.
Now, if we decide to optimize some range test, we can change some conditions
on previous bbs and that means we could execute some basic blocks that
wouldn't be executed in the original program. As the stmts don't set
anything used in other bbs, they are most likely dead after the
optimization, but the problem on the testcase is that because of the
condition changes in previous bb we end up with incorrect value range
for some SSA_NAME(s). That can result in the miscompilation of the testcase
on certain targets.
Fixed by resetting the value range info of such SSA_NAMEs. I believe it
shouldn't be a big deal, they will be mostly dead anyway.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2015-12-03 Jakub Jelinek <ja...@redhat.com>
PR tree-optimization/68671
* tree-ssa-reassoc.c (maybe_optimize_range_tests): For basic
blocks starting with the successor of first bb we've modified
and ending with last_bb, reset value ranges of all integral
SSA_NAMEs set in those basic blocks.
* gcc.dg/pr68671.c: New test.
--- gcc/tree-ssa-reassoc.c.jj 2015-11-18 11:22:51.000000000 +0100
+++ gcc/tree-ssa-reassoc.c 2015-12-03 18:12:08.915210122 +0100
@@ -3204,7 +3204,7 @@ maybe_optimize_range_tests (gimple *stmt
any_changes = optimize_range_tests (ERROR_MARK, &ops);
if (any_changes)
{
- unsigned int idx;
+ unsigned int idx, max_idx = 0;
/* update_ops relies on has_single_use predicates returning the
same values as it did during get_ops earlier. Additionally it
never removes statements, only adds new ones and it should walk
@@ -3220,6 +3220,7 @@ maybe_optimize_range_tests (gimple *stmt
{
tree new_op;
+ max_idx = idx;
stmt = last_stmt (bb);
new_op = update_ops (bbinfo[idx].op,
(enum tree_code)
@@ -3289,6 +3290,10 @@ maybe_optimize_range_tests (gimple *stmt
&& ops[bbinfo[idx].first_idx]->op != NULL_TREE)
{
gcond *cond_stmt = as_a <gcond *> (last_stmt (bb));
+
+ if (idx > max_idx)
+ max_idx = idx;
+
if (integer_zerop (ops[bbinfo[idx].first_idx]->op))
gimple_cond_make_false (cond_stmt);
else if (integer_onep (ops[bbinfo[idx].first_idx]->op))
@@ -3305,6 +3310,30 @@ maybe_optimize_range_tests (gimple *stmt
if (bb == first_bb)
break;
}
+
+ /* The above changes could result in basic blocks after the first
+ modified one, up to and including last_bb, to be executed even if
+ they would not be in the original program. If the value ranges of
+ assignment lhs' in those bbs were dependent on the conditions
+ guarding those basic blocks which now can change, the VRs might
+ be incorrect. As no_side_effect_bb should ensure those SSA_NAMEs
+ are only used within the same bb, it should be not a big deal if
+ we just reset all the VRs in those bbs. See PR68671. */
+ for (bb = last_bb, idx = 0; idx < max_idx; bb = single_pred (bb), idx++)
+ {
+ gimple_stmt_iterator gsi;
+ for (gsi = gsi_last_bb (bb); !gsi_end_p (gsi); gsi_prev (&gsi))
+ {
+ gimple *g = gsi_stmt (gsi);
+ if (!is_gimple_assign (g))
+ continue;
+ tree lhs = gimple_assign_lhs (g);
+ if (TREE_CODE (lhs) != SSA_NAME)
+ continue;
+ if (INTEGRAL_TYPE_P (TREE_TYPE (lhs)))
+ SSA_NAME_RANGE_INFO (lhs) = NULL;
+ }
+ }
}
}
--- gcc/testsuite/gcc.dg/pr68671.c.jj 2015-12-03 18:19:24.769104484 +0100
+++ gcc/testsuite/gcc.dg/pr68671.c 2015-12-03 18:19:07.000000000 +0100
@@ -0,0 +1,23 @@
+/* PR tree-optimization/68671 */
+/* { dg-do run } */
+/* { dg-options " -O2 -fno-tree-dce" } */
+
+volatile int a = -1;
+volatile int b;
+
+static inline int
+fn1 (signed char p1, int p2)
+{
+ return (p1 < 0) || (p1 > (1 >> p2)) ? 0 : (p1 << 1);
+}
+
+int
+main ()
+{
+ signed char c = a;
+ b = fn1 (c, 1);
+ c = ((128 | c) < 0 ? 1 : 0);
+ if (c != 1)
+ __builtin_abort ();
+ return 0;
+}
Jakub
