Re: [PATCH: PR /3187] C++ space optimization: declone delete destructor

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

On Thu, Sep 3, 2009 at 4:04 PM, Carrot Wei<carrot@google.com> wrote:
> This patch fixes a small part of
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=3187
>
> The delete destructor(D0) always contains the content of complete
> desturctor(D1) followed by a function call to delete. So instead of cloning
> the abstract destructor function body to the delete destructor(D0), we
> can generate a function call to complete destructor(D1) followed by a function
> call to delete.
>
> This method doesn't break the ABI or linker compatibility.
>
> With this patch we can reduce 2.59% instructions for an open source 2d library
> skia on arm. Some function call to complete destructor(D1) will be inlined in
> later pass. So more instructions can be removed if we can do better inlining.
>
> Test:
> Bootstrapped on linux-x86.
> gcc regression tests without new failure.
>
>
> ChangeLog:
> 2009-09-03 ?Wei Guozhi ?<carrot@google.com>
>
> ? ? ? ?PR /3187
> ? ? ? ?* cp/optimize.c (build_delete_destructor_body): New function.
> ? ? ? ?(maybe_clone_body): Call build_delete_destructor_body if optimize for
> ? ? ? ?size.
>
> thanks
> Guozhi
>
>
>
> Index: optimize.c
> ===================================================================
> --- optimize.c ?(revision 151143)
> +++ optimize.c ?(working copy)
> @@ -106,6 +106,41 @@ clone_body (tree clone, tree fn, void *a
> ? append_to_statement_list_force (stmts, &DECL_SAVED_TREE (clone));
> ?}
>
> +/* DELETE_DTOR is a delete destructor whose body will be build.
> + ? COMPLETE_DTOR is the corresponding complete destructor. ?*/
> +
> +static void
> +build_delete_destructor_body (tree delete_dtor, tree complete_dtor)
> +{
> + ?tree call_dtor, call_delete;
> + ?tree parm = DECL_ARGUMENTS (delete_dtor);
> + ?tree virtual_size = cxx_sizeof (current_class_type);
> +
> + ?/* Call the corresponding complete destructor. ?*/
> + ?gcc_assert (complete_dtor);
> + ?call_dtor = build_cxx_call (complete_dtor, 1, &parm);
> + ?add_stmt (call_dtor);
> +
> + ?add_stmt (build_stmt (0, LABEL_EXPR, cdtor_label));
> +
> + ?/* Call the delete function. ?*/
> + ?call_delete = build_op_delete_call(DELETE_EXPR, current_class_ptr,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? virtual_size,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? /*global_p=*/false,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? /*placement=*/NULL_TREE,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? /*alloc_fn=*/NULL_TREE);
> + ?add_stmt (call_delete);
> +
> + ?/* Return the address of the object. ?*/
> + ?if (targetm.cxx.cdtor_returns_this ())
> + ? ?{
> + ? ? ?tree val = DECL_ARGUMENTS (delete_dtor);
> + ? ? ?val = build2 (MODIFY_EXPR, TREE_TYPE (val),
> + ? ? ? ? ? ? ? ? ? ?DECL_RESULT (delete_dtor), val);
> + ? ? ?add_stmt (build_stmt (0, RETURN_EXPR, val));
> + ? ?}
> +}
> +
> ?/* FN is a function that has a complete body. ?Clone the body as
> ? ?necessary. ?Returns nonzero if there's no longer any need to
> ? ?process the main body. ?*/
> @@ -114,6 +149,7 @@ bool
> ?maybe_clone_body (tree fn)
> ?{
> ? tree clone;
> + ?tree complete_dtor = NULL_TREE;
> ? bool first = true;
>
> ? /* We only clone constructors and destructors. ?*/
> @@ -124,6 +160,15 @@ maybe_clone_body (tree fn)
> ? /* Emit the DWARF1 abstract instance. ?*/
> ? (*debug_hooks->deferred_inline_function) (fn);
>
> + ?/* Look for the complete destructor which may be used to build the
> + ? ? delete destructor. ?*/
> + ?FOR_EACH_CLONE (clone, fn)
> + ? ?if (DECL_NAME (clone) == complete_dtor_identifier)
> + ? ? ?{
> + ? ? ? ?complete_dtor = clone;
> + ? ? ? ?break;
> + ? ? ?}
> +
> ? /* We know that any clones immediately follow FN in the TYPE_METHODS
> ? ? ?list. ?*/
> ? push_to_top_level ();
> @@ -224,8 +269,15 @@ maybe_clone_body (tree fn)
> ? ? ? ? ?clone_parm = DECL_RESULT (clone);
> ? ? ? ? ?*pointer_map_insert (decl_map, parm) = clone_parm;
> ? ? ? ?}
> - ? ? ?/* Clone the body. ?*/
> - ? ? ?clone_body (clone, fn, decl_map);
> +
> + ? ? ?/* If we are optimizing for size, we can build the delete destructor
> + ? ? ? ? by calling complete destructor and delete function. ?*/
> + ? ? ?if ((DECL_NAME (clone) == deleting_dtor_identifier)
> + ? ? ? ? ?&& optimize_function_for_size_p (DECL_STRUCT_FUNCTION (clone)))
> + ? ? ? ?build_delete_destructor_body (clone, complete_dtor);

Using optimize_function_for_size_p doens't make sense in the frontends.
Just use optimize_size here.  In fact I would suggest to unconditionally
go with the build_delete_destructor_body variant.

In future we might want to have a way to mark this call as always binding
locally to avoid a relocation here and allow better optimization.

Richard.

> + ? ? ?else
> + ? ? ? ?/* Clone the body. ?*/
> + ? ? ? ?clone_body (clone, fn, decl_map);
>
> ? ? ? /* Clean up. ?*/
> ? ? ? pointer_map_destroy (decl_map);
>