Re-factor tree.h - Part 1
Richard Biener
richard.guenther@gmail.com
Thu Nov 7 10:41:00 GMT 2013
On Wed, Nov 6, 2013 at 6:10 PM, Diego Novillo <dnovillo@google.com> wrote:
> On Wed, Nov 6, 2013 at 2:04 AM, Marc Glisse <marc.glisse@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.
More information about the Gcc-patches
mailing list