I've figured that while the uninit diagnostic is intended to diagnose all cases where we cannot prove the must-uninit paths are not reaching any use the actual implementation errors on the wrong side in two places. First the recently added use_cannot_happen computes the must-uninit predicate using compute_control_dep_chain which errors on the side making the covered domain too small (pruning OR components), second, init_use_preds which computes the predicate that holds when the use is executed also uses the same function. In both cases the computational limits implemented in compute_control_dep_chain will cause false negatives. The following corrects this by using the recently introduced simple_control_dep_chain instead which errors on the side of making the domain larger (pruning AND components).
I shall note that I didn't yet find a testcase that requires use_cannot_happen which does the same as the init_from_phi_def and superset_of combination. The only difference is that init_from_phi_def computes conjunction of the edge predicates covering the not must-uninitialized control flow, also handling the case of PHI cascading and verifying the use predicate is a subset of this while use_cannot_happen computes whether each predicate on the uninitialized edges (may-uninit for the case of PHI cascading) has an empty intersection with the use predicate. Due to the PHI cascading handling the original should be superior but since both implementations apply the computational limit of compute_control_dep_chain in different ways and have different false negative issues it may have catched some extra bits at times. A patch to remove use_cannot_happen passes bootstrap and regtest with all languages on x86_64, my preference would be to axe it instead of fixing it. Note I am yet not sure why compute_control_dep_chain goes the length of computing all (well, only some) paths from the root to the PHI edge source rather than walking dominators as simple_control_dep_chain or using control dependences (and pruning them appropriately). The patch as-is causes FAIL: g++.dg/warn/uninit-pr55288.C (test for bogus messages, line 16) the testcase was fixed by r0-125162-g5c2961cf38a69f, the issue here is that simple_control_dep_chain does not handle A || A with the same predicate on different paths. XPASS: gcc.dg/uninit-pred-7_a.c bogus warning (test for bogus messages, line 26) FAIL: gcc.dg/uninit-pred-9_b.c pr101674 (test for bogus messages, line 23) where the FAIL is PR101674. I've commented extensively in the PR. Bootstrapped on x86_64-unknown-linux-gnu, full testing is still in progress. * gimple-predicate-analysis.cc (dump_dep_chains): Don't use auto_vec[] arguments. (uninit_analysis::use_cannot_happen): Only use simple_control_dep_chain. (uninit_analysis::init_use_preds): Likewise. (uninit_analysis::init_from_phi_def): Add comment why using compute_control_dep_chain here is OK. --- gcc/gimple-predicate-analysis.cc | 72 ++++++++++++-------------------- 1 file changed, 27 insertions(+), 45 deletions(-) diff --git a/gcc/gimple-predicate-analysis.cc b/gcc/gimple-predicate-analysis.cc index 079e06009fd..44f00507d83 100644 --- a/gcc/gimple-predicate-analysis.cc +++ b/gcc/gimple-predicate-analysis.cc @@ -238,14 +238,14 @@ dump_predicates (gimple *stmt, const pred_chain_union &preds, const char *msg) /* Dump the first NCHAINS elements of the DEP_CHAINS array into DUMP_FILE. */ static void -dump_dep_chains (const auto_vec<edge> dep_chains[], unsigned nchains) +dump_dep_chains (const vec<edge> dep_chains[], unsigned nchains) { if (!dump_file) return; for (unsigned i = 0; i != nchains; ++i) { - const auto_vec<edge> &v = dep_chains[i]; + const vec<edge> &v = dep_chains[i]; unsigned n = v.length (); for (unsigned j = 0; j != n; ++j) { @@ -1309,41 +1309,27 @@ uninit_analysis::use_cannot_happen (gphi *phi, unsigned opnds, const predicate & continue; edge e = gimple_phi_arg_edge (phi, i); - auto_vec<edge> dep_chains[MAX_NUM_CHAINS]; - auto_vec<edge, MAX_CHAIN_LEN + 1> cur_chain; - unsigned num_chains = 0; - unsigned num_calls = 0; - /* Build the control dependency chain for the PHI argument... */ - if (!compute_control_dep_chain (cd_root, - e->src, dep_chains, &num_chains, - cur_chain, &num_calls)) - { - gcc_assert (num_chains == 0); - /* If compute_control_dep_chain bailed out due to limits - build a partial sparse path using dominators. Collect - only edges whose predicates are always true when reaching E. */ - simple_control_dep_chain (dep_chains[0], cd_root, e); - num_chains++; - } - /* Update the chains with the phi operand edge. */ - else if (EDGE_COUNT (e->src->succs) > 1) - { - for (unsigned j = 0; j < num_chains; j++) - dep_chains[j].safe_push (e); - } + /* We cannot use compute_control_dep_chain here because that does + not give us a conservative answer for proving must initialized + since the domain is equal or less when it runs into its + computational limits. simple_control_dep_chain errors on + the other side so we can use that here to compute the + must-uninitialized predicate. */ + auto_vec<edge, MAX_CHAIN_LEN + 1> cur_chain; + simple_control_dep_chain (cur_chain, cd_root, e); if (DEBUG_PREDICATE_ANALYZER && dump_file) { fprintf (dump_file, "predicate::use_cannot_happen (...) " "dep_chains for arg %u:\n\t", i); - dump_dep_chains (dep_chains, num_chains); + dump_dep_chains (&cur_chain, 1); } /* ...and convert it into a set of predicates guarding its definition. */ predicate def_preds; - def_preds.init_from_control_deps (dep_chains, num_chains); + def_preds.init_from_control_deps (&cur_chain, 1); if (def_preds.is_empty ()) /* If there's no predicate there's no basis to rule the use out. */ return false; @@ -2147,36 +2133,27 @@ uninit_analysis::init_use_preds (predicate &use_preds, basic_block def_bb, break; } - /* Set DEP_CHAINS to the set of edges between CD_ROOT and USE_BB. - Each DEP_CHAINS element is a series of edges whose conditions - are logical conjunctions. Together, the DEP_CHAINS vector is - used below to initialize an OR expression of the conjunctions. */ - unsigned num_calls = 0; - unsigned num_chains = 0; - auto_vec<edge> dep_chains[MAX_NUM_CHAINS]; + /* compute_control_dep_chain, when it runs into computational + limits, errors on the side of making the domain smaller which + is not OK for the use predicate computation since our desire is + to diagnose uses that cannot be proved must-initialized. + Use simple_control_dep_chain instead. */ auto_vec<edge, MAX_CHAIN_LEN + 1> cur_chain; - - if (!compute_control_dep_chain (cd_root, use_bb, dep_chains, &num_chains, - cur_chain, &num_calls)) - { - gcc_assert (num_chains == 0); - simple_control_dep_chain (dep_chains[0], cd_root, use_bb); - num_chains++; - } + simple_control_dep_chain (cur_chain, cd_root, use_bb); if (DEBUG_PREDICATE_ANALYZER && dump_file) { fprintf (dump_file, "predicate::predicate (def_bb = %u, use_bb = %u, func_t) " - "initialized from %u dep_chains:\n\t", - def_bb->index, use_bb->index, num_chains); - dump_dep_chains (dep_chains, num_chains); + "initialized from dep_chain:\n\t", + def_bb->index, use_bb->index); + dump_dep_chains (&cur_chain, 1); } /* From the set of edges computed above initialize *THIS as the OR condition under which the definition in DEF_BB is used in USE_BB. Each OR subexpression is represented by one element of DEP_CHAINS, where each element consists of a series of AND subexpressions. */ - use_preds.init_from_control_deps (dep_chains, num_chains); + use_preds.init_from_control_deps (&cur_chain, 1); return !use_preds.is_empty (); } @@ -2246,6 +2223,11 @@ uninit_analysis::init_from_phi_def (gphi *phi) edge e = def_edges[i]; unsigned num_calls = 0; unsigned prev_nc = num_chains; + /* This computes the domain for must-initialized of the PHI + def based on the edge predicates. That errors on the + conservative side, making the must-initialized domain + smaller, when failing to consider paths if compute_control_dep_chain + runs into its computational limits. */ compute_control_dep_chain (cd_root, e->src, dep_chains, &num_chains, cur_chain, &num_calls); -- 2.35.3