From patchwork Sat Dec 17 19:44:40 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Easwaran Raman X-Patchwork-Id: 132021 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id 0CC161007D1 for ; Sun, 18 Dec 2011 06:44:59 +1100 (EST) Received: (qmail 22325 invoked by alias); 17 Dec 2011 19:44:57 -0000 Received: (qmail 22313 invoked by uid 22791); 17 Dec 2011 19:44:56 -0000 X-SWARE-Spam-Status: No, hits=-4.5 required=5.0 tests=AWL, BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mail-pz0-f47.google.com (HELO mail-pz0-f47.google.com) (209.85.210.47) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sat, 17 Dec 2011 19:44:41 +0000 Received: by dake40 with SMTP id e40so2912708dak.20 for ; Sat, 17 Dec 2011 11:44:40 -0800 (PST) Received: by 10.68.74.69 with SMTP id r5mr26217106pbv.118.1324151080728; Sat, 17 Dec 2011 11:44:40 -0800 (PST) MIME-Version: 1.0 Received: by 10.68.74.69 with SMTP id r5mr26217074pbv.118.1324151080518; Sat, 17 Dec 2011 11:44:40 -0800 (PST) Received: by 10.142.86.16 with HTTP; Sat, 17 Dec 2011 11:44:40 -0800 (PST) In-Reply-To: <4EE88927.2040201@oracle.com> References: <4EE5FED9.7080905@oracle.com> <4EE600BA.30105@oracle.com> <4EE66714.3000002@oracle.com> <4EE88927.2040201@oracle.com> Date: Sat, 17 Dec 2011 11:44:40 -0800 Message-ID: Subject: Re: [google] Add support for delete operator that takes the size of the object as a parameter From: Easwaran Raman To: Paolo Carlini Cc: Diego Novillo , gcc-patches@gcc.gnu.org, Xinliang David Li , jason@redhat.com, Paul Pluzhnikov X-System-Of-Record: true X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Based on Paolo's comments I am dropping the changes to baseline_symbols.txt. As far as minor version, I am bumping it up to 18. Assuming that this patch will be able to go in gcc 4.8 (with minor version 18), I want to keep it at the same version in google/main and google/gcc-4_6. Bootstraps and no test regression. OK for google/main and google/gcc-4_6 branches? --------- 011-12-17 Easwaran Raman * common.opt (fsized-delete): New option. cp/ChangeLog.google-main: 2011-12-17 Easwaran Raman * decl.c (cxx_init_decl_processing): Specify a function that takes a void* and size_t for DELETE_EXPR. * call.c (build_op_delete_call): If fsized-delete is used, use the variant that takes size_t as the second parameter except when deleteting a pointer of type void *. testsuite/ChangeLog.google-main: 2011-12-17 Easwaran Raman * g++.dg/other/sized-delete-1.C: New test. libstdc++-v3/ChangeLog.google-main: 2011-12-17 Easwaran Raman * libsupc++/del_op.cc (delete): Define a new variant void operator delete(void*, std::size_t). * libsupc++/new (delete): Declare void operator delete(void*, std::size_t) throw(); * testsuite/util/testsuite_abi.cc (check_version): Add new known version GLIBCXX_3.4.18. * config/abi/pre/gnu.ver: Add new symbol _ZdlPv[jmy] at version GLIBCXX_3.4.18. On Wed, Dec 14, 2011 at 3:31 AM, Paolo Carlini wrote: > Hi, > >> On Mon, Dec 12, 2011 at 12:41 PM, Paolo Carlini >>  wrote: >>> >>> On 12/12/2011 09:37 PM, Easwaran Raman wrote: >>>> >>>> Thanks for the comments Paolo. I have attached the new patch. I have >>>> bumped the version to 3.4.18 >>> >>> You shouldn't: 4.7 is not out yet, thus no reason to increase the minor >>> version beyond the current 17. >> >> Ok, I then don't understand your comment >>  "Note that backporting the patch as-is to 4_6-branch would be very >> wrong in terms of ABI (in mainline we already have a 3.4.17)". >> My original patch added the new symbol in version 3.4.17. Since we >> don't want to add the symbol to 3.4.16 (if we have a situation where >> the new runtime is not available when running a program compiled with >> -fsized-delete) and you   said I shouldn't be using 3.4.17, I assumed >> I had to bump up the version. > > Note this is going to be an academic discussion, because it's too late for > 4.7 anyway, and I'm not even sure it's conforming to add overloads like this > for operators. > > Anyway. > > For 4.6 branch at this point the situation would be very difficult. We could > have a small time window before 4.7 is out to move **all** its new symbols > vs 4.6, currently in 3.4.17,  to a new 3.4.18 and then we could bump 4.6 to > 3.4.17. You see, after a minor is delivered you cannot add anything to it, > and also you have the general requirement that the minor version is the > minor ersion, thus irrespective whether we are talking about gcc4.6 or > gcc4.7, a specific minor version number defines which symbols are exported. > I hope now the issues are more clear.You must be always very careful. > >>>>  and used _ZdlPv[jmy] in gnu.ver.  I have >>>> also added the symbol to baseline_symbols.txt of other targets. >>> >>> You should not, just read again what I wrote. And you don't have to >>> believe >>> me: just browse the libstdc++ ChangeLogs and see if somebody ever does >>> that >>> when the linker map is touched. >> >> Sorry, I again misunderstood this as well (and still don't have a good >> picture). Is the part which adds _ZdlPv[jmy] in gnu.ver ok? > > Looks fine yes. But I didn't really check the letters. General rule of thumb > when fiddling with linker scripts: double check what you are doing with a > multilib system, like x86_64, at least you can catch errors when you are > mistakenly not accounting for the 32-bit version of the symbols. In the case > at issue, size_t can boil down be unsigned int, unsigned long, unsigned long > long, make sure the pattern includes all three. > >> I added >> that by mimicking the symbol  _Znw[jmy] found in the same file. From >> the log, it looks like the baseline_symbols.txt seems to be generated, >> but I am not sure how that is to be done. For example, r145437 says a >> bunch of these baseline_symbols.txt are regenerated, but I don't see >> any other change from which this might be generated. > > General rule of thumb: normally, if you aren't a release manager, or a > maintainer, **never** regenerate the baseline_symbols files. Just add > symbols to the linker script, that is normally fine, doesn't break anything > per se, adding is fine (subtracting/changing is not, of course). > > Paolo. Index: libstdc++-v3/libsupc++/del_op.cc =================================================================== --- libstdc++-v3/libsupc++/del_op.cc (revision 182251) +++ libstdc++-v3/libsupc++/del_op.cc (working copy) @@ -46,3 +46,11 @@ operator delete(void* ptr) throw () if (ptr) std::free(ptr); } + +_GLIBCXX_WEAK_DEFINITION void +operator delete(void* ptr, + std::size_t bytes __attribute__((__unused__))) throw () +{ + if (ptr) + std::free(ptr); +} Index: libstdc++-v3/libsupc++/new =================================================================== --- libstdc++-v3/libsupc++/new (revision 182251) +++ libstdc++-v3/libsupc++/new (working copy) @@ -93,6 +93,7 @@ namespace std void* operator new(std::size_t) throw (std::bad_alloc); void* operator new[](std::size_t) throw (std::bad_alloc); void operator delete(void*) throw(); +void operator delete(void*, std::size_t) throw(); void operator delete[](void*) throw(); void* operator new(std::size_t, const std::nothrow_t&) throw(); void* operator new[](std::size_t, const std::nothrow_t&) throw(); Index: libstdc++-v3/testsuite/util/testsuite_abi.cc =================================================================== --- libstdc++-v3/testsuite/util/testsuite_abi.cc (revision 182251) +++ libstdc++-v3/testsuite/util/testsuite_abi.cc (working copy) @@ -195,6 +195,7 @@ check_version(symbol& test, bool added) known_versions.push_back("GLIBCXX_3.4.15"); known_versions.push_back("GLIBCXX_3.4.16"); known_versions.push_back("GLIBCXX_3.4.17"); + known_versions.push_back("GLIBCXX_3.4.18"); known_versions.push_back("GLIBCXX_LDBL_3.4"); known_versions.push_back("GLIBCXX_LDBL_3.4.7"); known_versions.push_back("GLIBCXX_LDBL_3.4.10"); @@ -561,4 +562,3 @@ demangle(const std::string& mangled) } return name; } - Index: libstdc++-v3/config/abi/pre/gnu.ver =================================================================== --- libstdc++-v3/config/abi/pre/gnu.ver (revision 182251) +++ libstdc++-v3/config/abi/pre/gnu.ver (working copy) @@ -1304,6 +1304,13 @@ GLIBCXX_3.4.17 { } GLIBCXX_3.4.16; +GLIBCXX_3.4.18 { + + # operator delete(void*, , unsigned long) + _ZdlPv[jmy]; + +} GLIBCXX_3.4.17; + # Symbols in the support library (libsupc++) have their own tag. CXXABI_1.3 { Index: gcc/testsuite/g++.dg/other/sized-delete-1.C =================================================================== --- gcc/testsuite/g++.dg/other/sized-delete-1.C (revision 0) +++ gcc/testsuite/g++.dg/other/sized-delete-1.C (revision 0) @@ -0,0 +1,14 @@ +// { dg-do link} +// { dg-options "-O -fsized-delete" } +// { dg-final { scan-assembler "_ZdlPv\[mj\]" } } +struct A +{ + int a[100]; +}; + +int main(void) +{ + A *a = new A; + delete a; + return 0; +} Index: gcc/cp/decl.c =================================================================== --- gcc/cp/decl.c (revision 182251) +++ gcc/cp/decl.c (working copy) @@ -3552,6 +3552,7 @@ cxx_init_decl_processing (void) { tree void_ftype; tree void_ftype_ptr; + tree void_ftype_ptr_sizetype; /* Create all the identifiers we need. */ initialize_predefined_identifiers (); @@ -3613,8 +3614,14 @@ cxx_init_decl_processing (void) void_ftype = build_function_type_list (void_type_node, NULL_TREE); void_ftype_ptr = build_function_type_list (void_type_node, ptr_type_node, NULL_TREE); + void_ftype_ptr_sizetype = build_function_type_list (void_type_node, + ptr_type_node, + size_type_node, + NULL_TREE); void_ftype_ptr = build_exception_variant (void_ftype_ptr, empty_except_spec); + void_ftype_ptr_sizetype + = build_exception_variant (void_ftype_ptr_sizetype, empty_except_spec); /* C++ extensions */ @@ -3669,7 +3676,7 @@ cxx_init_decl_processing (void) { tree newattrs; - tree newtype, deltype; + tree newtype, deltype, deltype2; tree ptr_ftype_sizetype; tree new_eh_spec; @@ -3704,8 +3711,10 @@ cxx_init_decl_processing (void) newtype = cp_build_type_attribute_variant (ptr_ftype_sizetype, newattrs); newtype = build_exception_variant (newtype, new_eh_spec); deltype = build_exception_variant (void_ftype_ptr, empty_except_spec); + deltype2 = build_exception_variant (void_ftype_ptr_sizetype, empty_except_spec); push_cp_library_fn (NEW_EXPR, newtype); push_cp_library_fn (VEC_NEW_EXPR, newtype); + push_cp_library_fn (DELETE_EXPR, deltype2); global_delete_fndecl = push_cp_library_fn (DELETE_EXPR, deltype); push_cp_library_fn (VEC_DELETE_EXPR, deltype); Index: gcc/cp/call.c =================================================================== --- gcc/cp/call.c (revision 182251) +++ gcc/cp/call.c (working copy) @@ -5408,8 +5408,22 @@ build_op_delete_call (enum tree_code code, tree ad usual deallocation function." So (void*) beats (void*, size_t). */ - if (FUNCTION_ARG_CHAIN (fn) == void_list_node) - break; + /* If type is not void, pick (void*, size_t) version (which comes + first). */ + if (!flag_sized_delete || TREE_CODE (type) == VOID_TYPE ) + { + /* If -fsized-delete is not passed or if a void * is deleted, + prefer delete (void *) version. */ + if (FUNCTION_ARG_CHAIN (fn) == void_list_node) + break; + } + else + { + /* If -fsized-delete is passed and it is not a void *, + prefer delete (void *, size_t) version. */ + if (FUNCTION_ARG_CHAIN (fn) != void_list_node) + break; + } } } Index: gcc/common.opt =================================================================== --- gcc/common.opt (revision 182251) +++ gcc/common.opt (working copy) @@ -1927,6 +1927,10 @@ fsingle-precision-constant Common Report Var(flag_single_precision_constant) Optimization Convert floating point constants to single precision constants +fsized-delete +Common Report Var(flag_sized_delete) Optimization +Support delete operator with objetc's size as the second parameter. + fsplit-ivs-in-unroller Common Report Var(flag_split_ivs_in_unroller) Init(1) Optimization Split lifetimes of induction variables when loops are unrolled