On 9/2/21 10:28 AM, Jeff Law via Gcc-patches wrote:
On 8/30/2021 2:03 PM, Martin Sebor via Gcc-patches wrote:
The predicate analysis subset of the tree-ssa-uninit pass isn't
necessarily specific to the detection of uninitialized reads.
Suitably parameterized, the same core logic could be used in
other warning passes to improve their S/N ratio, or issue more
nuanced diagnostics (e.g., when an invalid access cannot be
ruled out but also need not in reality be unavoidable, issue
a "may be invalid" type of warning rather than "is invalid").
Separating the predicate analysis logic from the uninitialized
pass and exposing a narrow API should also make it easier to
understand and evolve each part independently of the other,
or replace one with a better implementation without modifying
the other.(*)
As the first step in this direction, the attached patch extracts
the predicate analysis logic out of the pass, turns the interface
into public class members, and hides the internals in either
private members or static functions defined in a new source file.
(**)
The changes should have no externally observable effect (i.e.,
should cause no changes in warnings), except on the contents of
the uninitialized dump. While making the changes I enhanced
the dumps to help me follow the logic. Turning some previously
free-standing functions into members involved changing their
signatures and adjusting their callers. While making these
changes I also renamed some of them as well some variables for
improved clarity. Finally, I moved declarations of locals
closer to their point of initialization.
Tested on x86_64-linux. Besides the usual bootstrap/regtest
I also tentatively verified the generality of the new class
interfaces by making use of it in -Warray-bounds. Besides there,
I'd like to make use of it in the new gimple-ssa-warn-access pass
and, longer term, any other flow-sensitive warnings that might
benefit from it.
Martin
[*] A review of open -Wuninitialized bugs I did while working
on this project made me aware of a number of opportunities to
improve the analyzer to reduce the number of false positives
-Wmaybe-uninitiailzed suffers from.
[**] The class isn't fully general and, like the uninit pass,
only works with PHI nodes. I plan to generalize it to compute
the set of predicates between any two basic blocks.
gcc-predanal.diff
Factor predidacte analysis out of tree-ssa-uninit.c into its own module.
gcc/ChangeLog:
* Makefile.in (OBJS): Add gimple-predicate-analysis.o.
* tree-ssa-uninit.c (max_phi_args): Move to
gimple-predicate-analysis.
(MASK_SET_BIT, MASK_TEST_BIT, MASK_EMPTY): Same.
(check_defs):
(can_skip_redundant_opnd):
(compute_uninit_opnds_pos): Adjust to namespace change.
(find_pdom): Move to gimple-predicate-analysis.cc.
(find_dom): Same.
(struct uninit_undef_val_t): New.
(is_non_loop_exit_postdominating): Move to
gimple-predicate-analysis.cc.
(find_control_equiv_block): Same.
(MAX_NUM_CHAINS, MAX_CHAIN_LEN, MAX_POSTDOM_CHECK): Same.
(MAX_SWITCH_CASES): Same.
(compute_control_dep_chain): Same.
(find_uninit_use): Use predicate analyzer.
(struct pred_info): Move to gimple-predicate-analysis.
(convert_control_dep_chain_into_preds): Same.
(find_predicates): Same.
(collect_phi_def_edges): Same.
(warn_uninitialized_phi): Use predicate analyzer.
(find_def_preds): Move to gimple-predicate-analysis.
(dump_pred_info): Same.
(dump_pred_chain): Same.
(dump_predicates): Same.
(destroy_predicate_vecs): Remove.
(execute_late_warn_uninitialized): New.
(get_cmp_code): Move to gimple-predicate-analysis.
(is_value_included_in): Same.
(value_sat_pred_p): Same.
(find_matching_predicate_in_rest_chains): Same.
(is_use_properly_guarded): Same.
(prune_uninit_phi_opnds): Same.
(find_var_cmp_const): Same.
(use_pred_not_overlap_with_undef_path_pred): Same.
(pred_equal_p): Same.
(is_neq_relop_p): Same.
(is_neq_zero_form_p): Same.
(pred_expr_equal_p): Same.
(is_pred_expr_subset_of): Same.
(is_pred_chain_subset_of): Same.
(is_included_in): Same.
(is_superset_of): Same.
(pred_neg_p): Same.
(simplify_pred): Same.
(simplify_preds_2): Same.
(simplify_preds_3): Same.
(simplify_preds_4): Same.
(simplify_preds): Same.
(push_pred): Same.
(push_to_worklist): Same.
(get_pred_info_from_cmp): Same.
(is_degenerated_phi): Same.
(normalize_one_pred_1): Same.
(normalize_one_pred): Same.
(normalize_one_pred_chain): Same.
(normalize_preds): Same.
(can_one_predicate_be_invalidated_p): Same.
(can_chain_union_be_invalidated_p): Same.
(uninit_uses_cannot_happen): Same.
(pass_late_warn_uninitialized::execute): Define.
* gimple-predicate-analysis.cc: New file.
* gimple-predicate-analysis.h: New file.
Thanks for tackling this. It's something I think we've needed for a
long time.
I've only done a cursory review since this is primarily moving the
analysis engine to its own file and class-ifying it.
As we build additional consumers I won't be surprised if we have to
adjust the exposed class API, but that's fine. The first goal is to
get the analysis disentangled from the consumer(s).
OK. Looking forward to seeing the analysis used elsewhere.
Thanks, I just pushed it.
As a heads up, I've been having some weird email problems lately and
didn't find your approval until today, in my Trash folder. Just in
case there's fallout that I'm not responsive to, please either ping
me on IRC or email my @redhat account.
Martin
