The current gcc trunk miscompiles the following code on x86_64-linux at -Os and above in both 32-bit and 64-bit modes. This is a regression from 4.9.x. $ gcc-trunk -v Using built-in specs. COLLECT_GCC=gcc-trunk COLLECT_LTO_WRAPPER=/usr/local/gcc-trunk/libexec/gcc/x86_64-unknown-linux-gnu/5.0.0/lto-wrapper Target: x86_64-unknown-linux-gnu Configured with: ../gcc-trunk/configure --prefix=/usr/local/gcc-trunk --enable-languages=c,c++ --disable-werror --enable-multilib Thread model: posix gcc version 5.0.0 20150128 (experimental) [trunk revision 220198] (GCC) $ $ gcc-trunk -O1 small.c; ./a.out $ gcc-4.9 -Os small.c; ./a.out $ $ gcc-trunk -Os small.c $ ./a.out Aborted (core dumped) $ --------------------------------- struct S { int f1; }; static struct S a = { 1 }; char b; static unsigned char *c = &b; int d, e, f; int fn1 (int p) { return 0 ? 0 : p - 1; } static int fn2 (struct S p) { int g = 200; for (e = 4; e; e = fn1 (e)) { for (; d; d++) ; *c &= p.f1 & g; g = --*c; if (f) return 0; } return 0; } int main () { fn2 (a); if (b != 0) __builtin_abort (); return 0; }
This started with r217560, the first difference is during VRP2: Visiting statement: _13 = (signed char) g_68; +Match-and-simplified (signed char) g_68 to -56(OVF) Found new range for _13: [-56, -56] and various others, leading to -_10: ~[1, 254] +_10: [255, +INF] .MEM_11: VARYING -g_12: [0, 255] -_13: VARYING +g_12: [255, 255] +_13: [-56, -1] etc. Haven't checked if the changes are correct or not.
I think the problem is that: Visiting statement: _15 = _56 & 1; Applying pattern match.pd:312, gimple-match.c:13772 Applying pattern match.pd:235, gimple-match.c:13525 Match-and-simplified _56 & 1 to 0 Intersecting [0, 0] and [0, 1] to [0, 0] Found new range for _15: [0, 0] might be reasonable during iteration, when not yet considering the backedge as executable, but we actually never visit this statement again, perhaps due to match-and-simplify looking at earlier statements. Richard, can you please have a look?
Simulating block 2 Simulating block 2 Simulating block 25 Simulating block 15 Simulating block 4 Simulating block 3 Simulating block 5 Simulating block 6 Simulating block 4 Simulating block 12 Simulating block 13 Simulating block 26 Simulating block 14 Simulating block 8 Simulating block 7 Simulating block 15 Simulating block 16 Simulating block 17 Simulating block 10 Simulating block 8 Simulating block 9 Simulating block 18 Simulating block 19 Simulating block 23 Simulating block 11 Simulating block 21 Simulating block 10 Simulating block 20 Simulating block 22 Simulating block 11 So, we indeed simulate block 5 only the first time after simulating block 15 (i.e. when edge 25 -> 15 is executable and 14 -> 15 is not), and never again (block 15 itself is simulated again with both incoming edges executable). Wonder if for match-and-simplify during the VRP folding that looks at other statements we shouldn't somehow note at which statements we've actually looked when making the decision (but how?) and mark the statement for resimulation.
--- gcc/tree-vrp.c.jj 2015-01-28 08:39:51.000000000 +0100 +++ gcc/tree-vrp.c 2015-01-29 10:44:37.395688127 +0100 @@ -7093,14 +7093,14 @@ vrp_valueize_1 (tree name) if (TREE_CODE (name) == SSA_NAME) { value_range_t *vr = get_value_range (name); - if (range_int_cst_singleton_p (vr)) - return vr->min; /* If the definition may be simulated again we cannot follow this SSA edge as the SSA propagator does not necessarily re-visit the use. */ gimple def_stmt = SSA_NAME_DEF_STMT (name); if (prop_simulate_again_p (def_stmt)) return NULL_TREE; + if (range_int_cst_singleton_p (vr)) + return vr->min; } return name; } fixes this, even when a VR is singleton, it might be singleton just because it is singleton only in the first iteration and not afterwards. That said, I'd worry that this patch might degrade VRP folding too much.
Mine. I thought I had handled this correctly with vrp_valueize_1 doing /* Return the singleton value-range for NAME if that is a constant but signal to not follow SSA edges. */ static inline tree vrp_valueize_1 (tree name) { if (TREE_CODE (name) == SSA_NAME) { value_range_t *vr = get_value_range (name); if (range_int_cst_singleton_p (vr)) return vr->min; /* If the definition may be simulated again we cannot follow this SSA edge as the SSA propagator does not necessarily re-visit the use. */ gimple def_stmt = SSA_NAME_DEF_STMT (name); if (prop_simulate_again_p (def_stmt)) ^^^ return NULL_TREE; but I guess the singleton handling needs to be guarded with the same.
I have a patch (CCP is also affected).
Sth like Index: gcc/tree-ssa-propagate.c =================================================================== --- gcc/tree-ssa-propagate.c (revision 220235) +++ gcc/tree-ssa-propagate.c (working copy) @@ -364,6 +364,7 @@ simulate_stmt (gimple stmt) FOR_EACH_EDGE (e, ei, bb->succs) add_control_edge (e); } + return; } else if (val == SSA_PROP_INTERESTING) { @@ -377,6 +378,45 @@ simulate_stmt (gimple stmt) if (taken_edge) add_control_edge (taken_edge); } + + /* If there are no SSA uses on the stmt whose defs are simulated + again then this stmt will be never visited again. */ + bool has_simulate_again_uses = false; + use_operand_p use_p; + ssa_op_iter iter; + if (gimple_code (stmt) == GIMPLE_PHI) + { + edge_iterator ei; + edge e; + tree arg; + FOR_EACH_EDGE (e, ei, gimple_bb (stmt)->preds) + if (!(e->flags & EDGE_EXECUTABLE) + || ((arg = PHI_ARG_DEF_FROM_EDGE (stmt, e)) + && TREE_CODE (arg) == SSA_NAME + && !SSA_NAME_IS_DEFAULT_DEF (arg) + && prop_simulate_again_p (SSA_NAME_DEF_STMT (arg)))) + { + has_simulate_again_uses = true; + break; + } + } + else + FOR_EACH_SSA_USE_OPERAND (use_p, stmt, iter, SSA_OP_USE) + { + gimple def_stmt = SSA_NAME_DEF_STMT (USE_FROM_PTR (use_p)); + if (!gimple_nop_p (def_stmt) + && prop_simulate_again_p (def_stmt)) + { + has_simulate_again_uses = true; + break; + } + } + if (!has_simulate_again_uses) + { + if (dump_file && (dump_flags & TDF_DETAILS)) + fprintf (dump_file, "marking stmt to be not simulated again\n"); + prop_set_simulate_again (stmt, false); + } } /* Process an SSA edge worklist. WORKLIST is the SSA edge worklist to for what we discussed on IRC. Probably makes sense to omit PHI handling first as that's definitely "interesting". I'll bootstrap that separately on the fix for this PR (without handling PHIs).
Fixed.
Author: rguenth Date: Thu Jan 29 13:50:37 2015 New Revision: 220247 URL: https://gcc.gnu.org/viewcvs?rev=220247&root=gcc&view=rev Log: 2015-01-29 Richard Biener <rguenther@suse.de> PR tree-optimization/64853 * tree-vrp.c (vrp_valueize_1): Do not return anything if the stmt will get simulated again. * tree-ssa-ccp.c (valueize_op_1): Likewise. * gcc.dg/torture/pr64853.c: New testcase. Added: trunk/gcc/testsuite/gcc.dg/torture/pr64853.c Modified: trunk/gcc/ChangeLog trunk/gcc/testsuite/ChangeLog trunk/gcc/tree-ssa-ccp.c trunk/gcc/tree-vrp.c