On Wed, Nov 6, 2013 at 6:10 PM, Diego Novillo <dnovi...@google.com> wrote: > On Wed, Nov 6, 2013 at 2:04 AM, Marc Glisse <marc.gli...@inria.fr> wrote: >> On Tue, 5 Nov 2013, Diego Novillo wrote: >> >>> This is the first patch in a series of patches to cleanup tree.h to >>> reduce the exposure it has all over the compiler. >>> >>> In this patch, I'm moving functions that are used once into the files >>> that use them, and make them private to that file. These functions >>> were declared extern in tree.h and called from exactly one place. >> >> >> I am not a big fan of doing it so automatically. For instance >> widest_int_cst_value should imho remain next to int_cst_value since they >> mostly share the same implementation. Doing this also doesn't promote code >> reuse: if I am looking for a function that does some basic operation on >> trees, I won't only need to look in the file that is semantically relevant, > > Yeah, that was what I was trying to impose. Functions that > semantically make better sense in the place where they're called from. > I filtered functions that were used once but would not make much sense > to move (for instance, the init functions). > > How about this. Below is the list of functions that I took out of > tree.h. They are either not used or used only once, in which case I > moved them (and their private transitive closure) over to the file > that uses them. There are more, but these are the ones that I > originally considered to be too specific to the user file to be > globally exposed. Which ones would you folks keep global? > > I have marked some that we may decide to keep extern and one, which I > would not mind to not move at all. I also marked the functions I > removed and the ones that I just made private to their definition > file: > > Moved to builtins.c: > more_const_call_expr_args_p > expand_stack_restore > expand_stack_save
Makes sense. > Moved to cfgexpand.c: > expand_main_function > stack_protect_prologue > expand_asm_stmt > expand_computed_goto > expand_goto > expand_return Likewise. > Moved to cgraphclones.c: > build_function_decl_skip_args Likewise. > Moved to explow.c: > tree_expr_size (maybe keep extern?) Weird enough function, no need to export it. > Moved to expr.c: > fields_length Likewise. > Moved to fold-const.c: > size_low_cst Same as int_cst_value just with lack of any checking? > Moved to gimple-fold.c: > truth_type_for (maybe keep extern?) Uses a langhook so should stay here. > Moved to symtab.c: > decl_assembler_name_hash > decl_assembler_name_equal Ok. > Moved to trans-mem.c: > is_tm_safe_or_pure Ok. > Moved to tree-eh.c: > in_array_bounds_p > range_in_array_bounds_p I'm not sure ... > Moved to tree-ssa-dom.c: > iterative_hash_exprs_commutative I'm not sure - it seems that iterative_hash_expr does exactly the same thing? So instead of moving things this should be refactored (make that function re-usable from iterative_hash_expr). > Moved to tree-ssa-math-opts.c: > widest_int_cst_value (maybe keep extern?) Keep it in tree.c > Moved to tree-vrp.c: > fold_unary_to_constant (?) No. > Moved to cp/call.c > ctor_to_vec No. > Moved to cp/decl.c: > supports_one_only > chain_member No. > Moved to java/class.c: > build_method_type No. > Removed (not used anywhere) > build_type_no_quals > omp_remove_redundant_declare_simd_attrs > fold_build3_initializer_loc > real_twop > print_vec_tree > list_equal_p > ssa_name_nonnegative_p > addr_expr_of_non_mem_decl_p > save_vtable_map_decl > > Made static (only used in its defining file) > stabilize_reference_1 > tree_expr_nonzero_p > tree_invalid_nonnegative_warnv_p > tree_expr_nonzero_warnv_p > fold_builtin_snprintf_chk > validate_arglist > simple_cst_list_equal > lookup_scoped_attribute_spec > get_attribute_namespace > fini_object_sizes Those are obvious. Richard. > > Thanks. Diego.
