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