Re: [patch] Uniquization of constants at the Tree level

[Richard Guenther <richard dot guenther at gmail dot com>]

On Sun, Apr 25, 2010 at 1:40 PM, Eric Botcazou <ebotcazou@adacore.com> wrote:
> Hi,
>
> this is something we've been using in our 4.x compilers for quite some time.
> Constant objects (*_CST and CONSTRUCTOR) are uniquized at the RTL level by
> means of output_constant_def but there is no equivalent at the Tree level so,
> if a constant is commited to read-only memory at this level, it might be
> unnecessarily duplicated like in the following artificial C example:
>
> int lookup1 (int i)
> {
> ?int a[] = { 0, 1, 2, 3, 4, 5, 6, 7 };
> ?return a[i];
> }
>
> int lookup2 (int i)
> {
> ?int a[] = { 0, 1, 2, 3, 4, 5, 6, 7 };
> ?return a[i+1];
> }
>
> Compiled at -Os for i686, the assembly file contains:
>
> ? ? ? ?.section ? ? ? ?.rodata
> ? ? ? ?.align 4
> ? ? ? ?.type ? C.0.1957, @object
> ? ? ? ?.size ? C.0.1957, 32
> C.0.1957:
> ? ? ? ?.long ? 0
> ? ? ? ?.long ? 1
> ? ? ? ?.long ? 2
> ? ? ? ?.long ? 3
> ? ? ? ?.long ? 4
> ? ? ? ?.long ? 5
> ? ? ? ?.long ? 6
> ? ? ? ?.long ? 7
> ? ? ? ?.align 4
> ? ? ? ?.type ? C.1.1961, @object
> ? ? ? ?.size ? C.1.1961, 32
> C.1.1961:
> ? ? ? ?.long ? 0
> ? ? ? ?.long ? 1
> ? ? ? ?.long ? 2
> ? ? ? ?.long ? 3
> ? ? ? ?.long ? 4
> ? ? ? ?.long ? 5
> ? ? ? ?.long ? 6
> ? ? ? ?.long ? 7
>
>
> The patch introduces the counterpart of output_constant_def at the Tree level,
> namely tree_output_constant_def, which returns something directly usable in
> the GENERIC/GIMPLE IL (a VAR_DECL) when passed a valid constant, doing the
> uniquization and output work behind the scene automatically by means of the
> varasm.c uniquization machinery and the varpool.
>
> The new feature is used in gimplify_init_constructor, which eliminates the
> duplication for the above C example, as well as in gnat_gimplify_expr when an
> address of a constant constructor is gimplified (this happens often in Ada).
> Other front-ends could presumably use it as well in equivalent cases.
>
> The implementation is straightforward, modulo the following nits:
> ?- the varasm.c uniquization machinery wasn't updated for VECTOR_CSTs so the
> patch does that,
> ?- same for CONSTANT_ALIGNMENT of VECTOR_TYPEs for most architectures so the
> patch uses DATA_ALIGNMENT as well,
> ?- config/arm/unknown-elf.h duplicates a macro of varasm.c and someone thought
> it was clever to undefine the instance in varasm.c... so the patch untangles
> that (no functional changes for the ARM target because of that).
>
> Tested on x86_64-suse-linux and i586-suse-linux with mainline, but a backport
> to our 4.3-based compiler is in use on a wide range of platforms without any
> problems as of this writing. ?OK for mainline?

I doubt that this will work with LTO as-is.  Why can't we use
a file-scope shared CONST_DECL instead?

Most of the fixes regarding to vector types look ok - can you
split them out?  The IN_NAMED_SECTION parts look obvious
if split out.

Can you add a LTO testcase that has your C example in one
file and a main() in another verifying correctness of the
resulting initializer?

Thanks,
Richard.

>
> 2010-04-25 ?Eric Botcazou ?<ebotcazou@adacore.com>
>
> ? ? ? ?Uniquization of constants at the Tree level
> ? ? ? ?* tree.h (tree_output_constant_def): Declare.
> ? ? ? ?* gimplify.c (gimplify_init_constructor): When using block copy,
> ? ? ? ?uniquize the constant constructor on the RHS.
> ? ? ? ?* varasm.c (IN_NAMED_SECTION): Remove guard.
> ? ? ? ?(tree_constant_section_name): New static variable.
> ? ? ? ?(DECL_TREE_LEVEL_CONSTANT_P): New macro.
> ? ? ? ?(make_decl_rtl): Deal with Tree level constants.
> ? ? ? ?(assemble_variable): If the underlying symbol is attached to a
> ? ? ? ?uniquized constant, output the constant.
> ? ? ? ?(decode_addr_const): Handle special case of INDIRECT_REF.
> ? ? ? ?(const_hash_1) <VECTOR_CST>: New case.
> ? ? ? ?(compare_constant) <VECTOR_CST>: Likewise.
> ? ? ? ?<ADDR_EXPR>: Deal with LABEL_REFs.
> ? ? ? ?(copy_constant) <VECTOR_CST>: New case.
> ? ? ? ?(get_constant_alignment): Delete.
> ? ? ? ?(get_constant_section): Add ALIGN parameter and simplify.
> ? ? ? ?(build_constant_desc): Build a VAR_DECL and attach it to the symbol.
> ? ? ? ?(assemble_constant_contents): Use the expression of the VAR_DECL.
> ? ? ? ?(output_constant_def_contents): Use the alignment of the VAR_DECL.
> ? ? ? ?(tree_output_constant_def): New global function.
> ? ? ? ?(mark_constant): Use the expression of the VAR_DECL.
> ? ? ? ?(init_varasm_once): Initialize tree_constant_section_name.
> ? ? ? ?(place_block_symbol): Use the alignment of the VAR_DECL and the size
> ? ? ? ?of its expression.
> ? ? ? ?(output_object_block): Likewise and assemble the expression.
> ? ? ? ?* config/arm/unknown-elf.h (IN_NAMED_SECTION): Rename to...
> ? ? ? ?(IN_NAMED_SECTION_P): ...this.
> ? ? ? ?(ASM_OUTPUT_ALIGNED_BSS): Adjust for above renaming.
> ? ? ? ?(ASM_OUTPUT_ALIGNED_DECL_LOCAL): Likewise.
> ada/
> ? ? ? ?* gcc-interface/trans.c (gnat_gimplify_expr) <ADDR_EXPR>: Uniquize
> ? ? ? ?constant constructors before taking their address.
>
>
> --
> Eric Botcazou
>