This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


On Mon, Sep 7, 2009 at 5:32 PM, Richard
Guenther<richard.guenther@gmail.com> wrote:
> On Mon, Sep 7, 2009 at 10:03 AM, Carrot Wei<carrot@google.com> wrote:
>> Thank you Richard, unconditionally construct deleting destructor this way is
>> also a good idea, since it can still get inlined later. I've modified the
>> patch according to your suggestion. Could you take another look?
>>
>> I've run the boot-strap and gcc regression tests again, and without any new
>> failure.
>
> It looks good but I can't approve it. ?I think we need to make sure we
> have a way
> of forcing that call to bind locally (otherwise inlining wouldn't work as well).
> Do we ever have weak destructors? ?OTOH they need to follow ODR so they'd
> be comdat at most - can you verify the call is indeed inlined?
>
Yes, I did notice inlining occurred. That makes up another 1% code
size opportunity for skia on arm. Since I didn't change any function
attributes, it should not violate ODR.

thanks
Wei Guozhi

> Thanks,
> Richard.
>
>> thanks
>> Wei 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,14 @@ 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);
>> +
>> + ? ? ?/* Build the delete destructor by calling complete destructor
>> + ? ? ? ? and delete function. ?*/
>> + ? ? ?if (DECL_NAME (clone) == deleting_dtor_identifier)
>> + ? ? ? ?build_delete_destructor_body (clone, complete_dtor);
>> + ? ? ?else
>> + ? ? ? ?/* Clone the body. ?*/
>> + ? ? ? ?clone_body (clone, fn, decl_map);
>>
>> ? ? ? /* Clean up. ?*/
>> ? ? ? pointer_map_destroy (decl_map);
>>
>>
>> On Thu, Sep 3, 2009 at 10:20 PM, Richard
>> Guenther<richard.guenther@gmail.com> wrote:
>>> 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);
>>>>
>>>
>>
>


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]