On Mon, Aug 31, 2015 at 7:49 AM, Mikhail Maltsev <malts...@gmail.com> wrote:
> Hi, all!
>
> This patch removes some conditional compilation from GCC. In this patch I
> define
> a macro CHECKING_P, which is equal to 1 when ENABLE_CHECKING is defined and 0
> otherwise. The reason for using a new name is the following: currently in GCC
> there are many places where ENABLE_CHECKING is checked using #ifdef, and it
> is a
> common way to do such checks (though #if would also work and is used in
> several
> places). If we change it in such way that it is always defined, accidentally
> using "#ifdef" instead of "#if" will lead to subtle errors: some expensive
> checks intended only for development stage will be enabled in release build
> and
> cause performance degradation.
>
> This patch removes all uses of ENABLE_CHECKING (I also tried poisoning this
> identifier in system.h, and the build succeeded, but I don't know how will
> this
> affect, e.g. merging feature branches, so I think such decisions should be
> made
> by maintainers).
I think we want to keep ENABLE_CHECKING for macro use and for some selected
cases.
> As for conditional compilation, I tried to remove it and replace #ifdef-s with
> if-s where possible, but, of course, avoided changing data structures layout.
> I
> also avoided reindenting large blocks of code. I changed some functions (and a
> couple of global variables) that were only used in "checking" build so that
> now
> they are always defined/compiled and have a DEBUG_FUNCTION (i.e. "used")
> attribute. I'll try to handle them in a more clean way: move CHECKING_P check
> inside their definitions - that will slightly reduce "visual noise" from
> conditions like
>
> if (CHECKING_P)
> {
> verify_insn_chain ();
> verify_flow_info ();
> }
>
> in their callers. But I think it is better to do this change as a separate
> patch.
>
> While working on this patch I noticed some issues worth mentioning. In
> sese.h:bb_in_region we have a check (enabled only in "checking" build):
>
> /* Check that there are no edges coming in the region: all the
> predecessors of EXIT are dominated by ENTRY. */
> FOR_EACH_EDGE (e, ei, exit->preds)
> dominated_by_p (CDI_DOMINATORS, e->src, entry);
>
> IIUC, dominated_by_p has no side effects, so it's useless. Changing it to
> "gcc_assert (dominated_by_p (...));" causes regressions. I disabled it in the
> patch and added a comment.
You should open a bugreport.
> In lra.c we have:
>
> #ifdef ENABLE_CHECKING
>
> /* Function checks RTL for correctness. If FINAL_P is true, it is
> done at the end of LRA and the check is more rigorous. */
> static void
> check_rtl (bool final_p)
> ...
> #ifdef ENABLED_CHECKING
> extract_constrain_insn (insn);
> #endif
> ...
> #endif /* #ifdef ENABLE_CHECKING */
>
> The comment for extract_constrain_insn says:
> /* Do uncached extract_insn, constrain_operands and complain about failures.
> This should be used when extracting a pre-existing constrained instruction
> if the caller wants to know which alternative was chosen. */
>
> So, as I understand, this additional check can be useful. This patch removes
> "#ifdef ENABLED_CHECKING" (without regressions).
>
> The third issue is that some gcc_checking_assert-s were guarded by #ifdef
> like:
> #ifdef ENABLE_CHECKING
> gcc_checking_assert (!is_deleted (*slot));
> #endif
> (is_deleted is defined unconditionally).
>
> Probably that is because it is not obvious from the "release" definition
> #define gcc_checking_assert(EXPR) ((void)(0 && (EXPR)))
> that EXPR is not evaluated.
>
> At least, I first decided to change it to something like
> "do { if (0) (void)(EXPR); } while(0)"
> and only then realized that the effect of "0 &&" is exactly the same. I added
> a
> comment to make it more obvious.
>
> I tested the patch on x86_64-pc-linux-gnu with --enable-checking=yes and
> "release". Likewise, built config-list.mk in both configurations. There we
> some
> minor changes since that check, but I'll need to rebase the patch anyway, so I
> will repeat the full check.
>
> Please review the patch.
Erm. Ok so if you are here... I've long wanted the heavier parts of checking
(like IL checking) to be controlled by a command-line switch (-fchecking or
even -fchecking[=...]) rather than being compile-time controlled. Obviously
that's not sth we want (or easily can get) for things like tree-checking or
asserts.
Just a suggestion which might eventually remove a whole bunch of
ENABLE_CHECKING #ifs before tackling the rest.
Thanks,
Richard.
> --
> Regards,
> Mikhail Maltsev
>
> libcpp/ChangeLog:
>
> 2015-08-24 Mikhail Maltsev <malts...@gmail.com>
>
> * include/line-map.h: Change #ifdef to #if.
> * init.c: Likewise.
> * macro.c (struct macro_arg_token_iter): Likewise.
> (set_arg_token): Remove conditional compilation.
> (macro_arg_token_iter_init, macro_arg_token_iter_forward,
> macro_arg_token_iter_get_token,
> macro_arg_token_iter_get_location): Change #ifdef to #if.
> (alloc_expanded_arg_mem, _cpp_backup_tokens): Remove conditional
> compilation.
> * system.h: Define CHECKING_P macro.
>
> gcc/lto/ChangeLog:
>
> 2015-08-24 Mikhail Maltsev <malts...@gmail.com>
>
> * lto.c (unify_scc): Remove conditional compilation.
> (lto_fixup_state, do_whole_program_analysis): Likewise.
>
> gcc/fortran/ChangeLog:
>
> 2015-08-24 Mikhail Maltsev <malts...@gmail.com>
>
> * trans-common.c (create_common): Remove conditional compilation.
> * trans.c (gfc_add_modify_loc): Likewise.
>
> gcc/cp/ChangeLog:
>
> 2015-08-24 Mikhail Maltsev <malts...@gmail.com>
>
> * call.c: Change #ifdef to #if.
> * constexpr.c (maybe_constant_value): Remove conditional compilation.
> (fold_non_dependent_expr): Likewise.
> * cp-tree.h: Change #ifdef to #if.
> * decl2.c (cxx_post_compilation_parsing_cleanups): Remove conditional
> compilation.
> * mangle.c (add_substitution): Likewise.
> * method.c (maybe_explain_implicit_delete): Likewise.
> * parser.c (cp_parser_template_argument_list): Likewise.
> * pt.c (check_unstripped_args): Change #ifdef to #if.
> (template_parm_to_arg, template_parms_to_args,
> coerce_template_parameter_pack, coerce_template_parms, tsubst_copy,
> type_unification_real, build_non_dependent_expr): Remove conditional
> compilation.
> * tree.c (build_target_expr): Likewise.
> * typeck.c (comptypes): Likewise.
> * typeck2.c (digest_init_r): Likewise.
>
> gcc/ada/ChangeLog:
>
> 2015-08-24 Mikhail Maltsev <malts...@gmail.com>
>
> * gcc-interface/decl.c (gnat_to_gnu_entity): Use gcc_checking_assert.
> * gcc-interface/trans.c (assoc_to_constructor): Remove conditional
> compilation.
> * gcc-interface/utils.c (relate_alias_sets): Likewise.
> * gcc-interface/utils2.c (build_binary_op, build_unary_op): Use
> gcc_checking_assert.
>
> gcc/java/ChangeLog:
>
> 2015-08-24 Mikhail Maltsev <malts...@gmail.com>
>
> * decl.c (java_mark_decl_local): Remove conditional compilation.
>
> gcc/ChangeLog:
>
> 2015-08-24 Mikhail Maltsev <malts...@gmail.com>
>
> * alloc-pool.h (pool_allocator::initialize, allocate, remove): Remove
> conditional compilation.
> * attribs.c (init_attributes): Change #ifdef to #if.
> * cfgcleanup.c (try_optimize_cfg): Remove conditional compilation.
> * cfgexpand.c (expand_goto, expand_debug_expr,
> pass_expand::execute): Likewise.
> * cfgrtl.c (commit_edge_insertions): Likewise.
> (fixup_reorder_chain, cfg_layout_finalize, rtl_flow_call_edges_add):
> Likewise.
> * cgraph.c (symbol_table::create_edge,
> cgraph_edge::redirect_call_stmt_to_callee): Likewise.
> * cgraph.h (cgraph_node::get): Likewise.
> * cgraphclones.c (symbol_table::materialize_all_clones): Likewise
> * cgraphunit.c (mark_functions_to_output, cgraph_node::expand_thunk,
> symbol_table::compile): Likewise.
> * config/alpha/alpha.c (alpha_function_arg): Use gcc_checking_assert.
> * config/arm/arm.c (arm_unwind_emit_sequence): Change #ifdef to #if.
> * config/bfin/bfin.c (hwloop_optimize): Remove conditional
> compilation.
> * config/i386/i386.c (ix86_print_operand_address): Change #ifdef to
> #if.
> (output_387_binary_op): Remove conditional compilation.
> * config/ia64/ia64.c (ia64_sched_init): Likewise.
> (bundling): Change #ifdef to #if.
> * config/m68k/m68k.c (m68k_sched_md_init_global): Likewise.
> * config/rs6000/rs6000.c (htm_expand_builtin): Likewise.
> (rs6000_emit_prologue): Likewise.
> * config/rs6000/rs6000.h REGNO_REG_CLASS[CHECKING_P]: Likewise
> * config/visium/visium.c (visium_setup_incoming_varargs): Likewise.
> * ddg.c (add_cross_iteration_register_deps): Likewise.
> (create_ddg_all_sccs): Likewise.
> * df-core.c (df_finish_pass): Remove some conditional compilation.
> (df_analyze): Likewise.
> * diagnostic-core.h: Use CHECKING_P
> * diagnostic.c (diagnostic_report_diagnostic): Remove conditional
> compilation.
> * dominance.c (calculate_dominance_info): Likewise.
> * dwarf2out.c (add_AT_die_ref, const_ok_for_output_1,
> mem_loc_descriptor, loc_list_from_tree, gen_lexical_block_die,
> gen_type_die_with_usage, gen_type_die, dwarf2out_decl): Likewise.
> * emit-rtl.c (verify_rtx_sharing, reorder_insns_nobb): Likewise.
> * et-forest.c: Fix comment.
> * except.c (duplicate_eh_regions): Remove conditional compilation.
> * fwprop.c (register_active_defs, update_df_init, update_uses,
> fwprop_init, fwprop_done): Likewise.
> * genautomata.c: Fix #if condition.
> * genconditions.c: Redefine CHECKING_P in generated code.
> * genextract.c: Replace #ifdef with #if.
> * gengtype.c (main): Remove conditional compilation.
> * gengtype.h: Replace #ifdef with #if.
> * ggc-page.c (ggc_grow): Remove conditional compilation.
> * gimplify.c (gimplify_body, gimplify_hasher::equal): Likewise.
> * graphite-isl-ast-to-gimple.c (graphite_verify): Likewise.
> * graphite-scop-detection.c (create_sese_edges, build_graphite_scops,
> canonicalize_loop_closed_ssa_form): Likewise.
> * graphite-sese-to-poly.c (rewrite_reductions_out_of_ssa,
> rewrite_cross_bb_scalar_deps_out_of_ssa,
> rewrite_commutative_reductions_out_of_ssa): Likewise.
> * hash-table.h (hash_table::find_empty_slot_for_expand): Likewise.
> * ifcvt.c (if_convert): Likewise.
> * ipa-cp.c (ipcp_propagate_stage): Likewise.
> * ipa-devirt.c (type_in_anonymous_namespace_p, odr_type_p,
> odr_types_equivalent_p, add_type_duplicate, get_odr_type): Likewise.
> * ipa-icf.c (sem_item_optimizer::verify_classes,
> sem_item_optimizer::traverse_congruence_split,
> sem_item_optimizer::do_congruence_step_for_index): Likewise.
> * ipa-inline-analysis.c (compute_inline_parameters): Likewise.
> * ipa-inline-transform.c (save_inline_function_body): Likewise.
> * ipa-inline.c (inline_small_functions, early_inliner): Likewise.
> * ipa-inline.h (estimate_edge_growth): Likewise.
> * ipa-visibility.c (function_and_variable_visibility): Likewise.
> * ipa.c (symbol_table::remove_unreachable_nodes,
> ipa_single_use): Likewise.
> * ira-int.h: Replace #ifdef with #if.
> * ira.c (ira): Remove conditional compilation.
> * loop-doloop.c (doloop_optimize_loops): Likewise.
> * loop-init.c (loop_optimizer_init, fix_loop_structure): Likewise.
> * loop-invariant.c (move_loop_invariants): Likewise.
> * lra-assigns.c (lra_assign): Likewise.
> * lra-constraints.c (lra_constraints): Likewise.
> * lra-eliminations.c (lra_eliminate): Likewise.
> * lra-int.h (struct lra_reg): Replace #ifdef with #if.
> * lra-lives.c (check_pseudos_live_through_calls,
> lra_create_live_ranges_1): Likewise.
> * lra-remat.c (create_remat_bb_data): Remove conditional compilation.
> * lra.c (lra_update_insn_recog_data, restore_scratches): Likewise.
> (check_rtl): Remove incorrect nested #ifdef.
> (lra): Replace #ifdef with #if.
> * lto-cgraph.c (input_cgraph_1): Remove conditional compilation.
> * lto-streamer-out.c (DFS::DFS): Likewise.
> (lto_output): Replace #ifdef with #if.
> * lto-streamer.c (lto_streamer_init): Remove conditional compilation.
> * omp-low.c (scan_omp_target, expand_omp_taskreg, expand_omp_target,
> execute_expand_omp): Likewise.
> (lower_omp_target): Replace #ifdef with #if.
> * passes.c (execute_function_todo, execute_todo,
> execute_one_pass): Remove conditional compilation.
> * predict.c (tree_estimate_probability, propagate_freq): Likewise.
> * pretty-print.c (pp_format): Likewise.
> * real.c (real_to_decimal_for_mode): Likewise.
> * recog.c (split_all_insns): Likewise.
> * regcprop.c (kill_value_one_regno): Replace #ifdef with #if.
> (copy_value): Remove conditional compilation.
> * reload.c: Fix comment.
> * sched-deps.c (CHECK): Remove unused macro definition.
> (check_dep): Remove conditional compilation.
> (add_or_update_dep_1): Likewise.
> (sd_add_dep): Likewise.
> * sel-sched-ir.c (free_regset_pool, tidy_control_flow): Likewise.
> * sel-sched.c (struct moveop_static_params,
> find_best_reg_for_expr): Replace #ifdef with #if.
> (move_cond_jump): Remove conditional compilation.
> (move_op_orig_expr_not_found, code_motion_process_successors,
> move_op): Replace #ifdef with #if.
> * sese.h (bb_in_region): Add comment, guard code with #if 0.
> * ssa-iterators.h (first_readonly_imm_use,
> next_readonly_imm_use): Remove conditional compilation.
> * store-motion.c (compute_store_table): Likewise.
> * symbol-summary.h: Likewise.
> * system.h (CHECKING_P): Define.
> * target.h (get_cumulative_args, pack_cumulative_args): Replace #ifdef
> with #if.
> * timevar.c (timer::print): Remove conditional compilation.
> * trans-mem.c (ipa_tm_execute): Likewise.
> * tree-cfg.c (move_stmt_op): Likewise.
> (move_sese_region_to_fn, gimple_flow_call_edges_add): Likewise.
> * tree-cfgcleanup.c (cleanup_tree_cfg_noloop,
> repair_loop_structures): Likewise.
> * tree-eh.c (remove_unreachable_handlers): Likewise.
> * tree-if-conv.c (pass_if_conversion::execute): Likewise.
> * tree-inline.c (expand_call_inline, optimize_inline_calls): Likewise.
> * tree-into-ssa.c (update_ssa): Replace #ifdef with #if.
> * tree-loop-distribution.c: Remove conditional compilation.
> * tree-outof-ssa.c (eliminate_useless_phis): Replace #ifdef with #if.
> (rewrite_trees): Remove conditional compilation.
> * tree-parloops.c (pass_parallelize_loops::execute): Likewise.
> * tree-predcom.c (suitable_component_p): Likewise.
> * tree-profile.c (gimple_gen_const_delta_profiler): Likewise.
> * tree-ssa-alias.c (refs_may_alias_p_1): Likewise.
> * tree-ssa-live.c (calculate_live_ranges): Replace #ifdef with #if.
> * tree-ssa-live.h (register_ssa_partition): Likewise.
> * tree-ssa-loop-ivcanon.c (tree_unroll_loops_completely): Remove
> conditional compilation.
> * tree-ssa-loop-manip.c (add_exit_phi,
> tree_transform_and_unroll_loop): Likewise.
> * tree-ssa-math-opts.c (pass_cse_reciprocals::execute): Likewise.
> * tree-ssa-operands.c (get_expr_operands): Likewise.
> * tree-ssa-propagate.c (replace_exp_1): Likewise.
> * tree-ssa-structalias.c (rewrite_constraints): Likewise.
> * tree-ssa-ter.c (free_temp_expr_table): Replace #ifdef with #if.
> * tree-ssa-threadupdate.c (duplicate_thread_path): Remove conditional
> compilation.
> * tree-ssanames.c (release_ssa_name_fn): Likewise.
> * tree-stdarg.c (expand_ifn_va_arg): Likewise.
> * tree-vect-loop-manip.c (slpeel_tree_duplicate_loop_to_edge_cfg,
> vect_do_peeling_for_loop_bound,
> vect_do_peeling_for_alignment): Likewise.
> * tree-vrp.c (supports_overflow_infinity, set_value_range): Likewise.
> * tree.c (free_lang_data_in_cgraph): Likewise.
> * value-prof.c (gimple_remove_histogram_value, free_hist): Likewise.
> * var-tracking.c (VAR_PART_OFFSET): Replace #ifdef with #if.
> (canonicalize_values_star): Remove conditional compilation.
> (compute_bb_dataflow): Replace #ifdef with #if.
> (vt_find_locations, vt_emit_notes): Likewise.
>
