From patchwork Wed May 6 21:22:26 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Andrew MacLeod X-Patchwork-Id: 469145 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]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id B4812140218 for ; Thu, 7 May 2015 07:22:39 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=H4br5n44; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:date:from:mime-version:to:subject:content-type; q= dns; s=default; b=AVo69f3YRWPz/aSuAawK/DBbnbz6WRTFCx7kLGVBO70eJC jU5fG4zPgWaNtQPw9lYnwDGx2ffXmFgiunTkTq2lePLdiDaDzsAxHG1Z9KKaeBwD 94YRsFfw20MTF6MdAdnLYTTQMplqh0oQ6g8pVYgbZD7EZQuqn+fQ7P6jzlrCM= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:date:from:mime-version:to:subject:content-type; s= default; bh=A9A7LKgGHWIO4WE95d6ohVyH5aY=; b=H4br5n44/9iWvu+EQ2xd vOvf2dm/i5yI6Hkg5i38LGKp6fFh37go6+3PHBvabLzE7dglqvutTj/bjy/ZNhAR Tku2OfLcP6+VrHXa1kEt6FB/7BRsOHlLN2vA31Smv8IJIFmEjc8D7aXoKMQQHigk +kkRhX8Stazi1/gxOJhCFE8= Received: (qmail 21783 invoked by alias); 6 May 2015 21:22:32 -0000 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 Received: (qmail 21774 invoked by uid 89); 6 May 2015 21:22:32 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.0 required=5.0 tests=AWL, BAYES_50, KAM_ASCII_DIVIDERS, SPF_HELO_PASS, SPF_PASS, T_FILL_THIS_FORM_SHORT, T_RP_MATCHES_RCVD autolearn=no version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Wed, 06 May 2015 21:22:30 +0000 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) by mx1.redhat.com (Postfix) with ESMTPS id 09FEFAC7D2 for ; Wed, 6 May 2015 21:22:28 +0000 (UTC) Received: from [10.10.60.72] (vpn-60-72.rdu2.redhat.com [10.10.60.72]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t46LMRGx017016; Wed, 6 May 2015 17:22:27 -0400 Message-ID: <554A8612.6080007@redhat.com> Date: Wed, 06 May 2015 17:22:26 -0400 From: Andrew MacLeod User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.2.0 MIME-Version: 1.0 To: gcc-patches , Jonathan Wakely Subject: resurrect __attribute__((atomic)) work X-IsSubscribed: yes This patch resurrects the __attribute__((atomic)) bits from the old C11 atomic branch that were never included in trunk. Motivation was for C++ to be able to use the attribute in the atomic templates giving c++ exact compatibility with C11 _Atomic. Alignment has been an issue, and jwakley had to hack around it for this latest release. There are target hooks which allow the alignment or size of atomic objects to be changed, and that sort of thing is automatically reflected by using the attribute. That said, this is an incomplete implementation, but is somewhat functional. I changed the types to use __attribute__((atomic)) in the c++ templates and all the test pass. I left a static assert in place which checks that the alignment jwakely calculated matches the declared atomic alignment... so thats all good. It works for basic integers, ie: int __attribute__((atomic)) x; int y; void boo(int t) { y = x; x = t; } will generate atomic loads and stores for 'x'. It breaks down after that. There are bit and pieces in the frontends that I dont understand which prevent more complex things. ie struct s { char c[1000]; }; typedef struct s __attribute__((atomic)) as; produces and error: warning: ignoring attributes applied to ‘struct s’ after definition [-Wattributes] typedef struct s __attribute__((atomic)) as; The only interesting addition I've made is resolve_overloaded_builtin() has been modified to take : int __attribute__((atomic)) x; //* or _Atomic int x; */ y = __atomic_load_n (&x, __ATOMIC_SEQ_CST); and issue a VIEW_CONVERT on &x to see it as a pointer to a NON-ATOMIC x. The c++ front end was whining about how the atomic qualifier didnt match the expected "const volatile void *" parameter for the builtins. Im not sure how C11 avoided that, so perhaps there is a better way :-) And Im sure there are lots of other things that break... Fortunately, a lot of the C11 pathways through the front end are used and this patch is quite small. If someone else is interested in this, they can pick it up from here :-) Its at least better than trying to find it on the branch and get it to this point. I may poke at it again someday, but I'm not planning to for a while. It bootstraps with no new regressions on x86_64-unknown-linux-gnu... Andrew gcc * c-family/c-common.c (const struct attribute_spec c_common_att): Add "atomic" attribute. (handle_atomic_attribute): New. Deal with __attribute__((atomic)). (resolve_overloaded_builtin): If an atomic object is passed by address at an __atomic function, cast the atomic-ness away for processing. * c-family/c-pretty-print.c (pp_c_cv_qualifiers): Print attribute when not ISO c11. libstdc++-v3 * include/bits/atomic_base.h (struct __atomic_base): Make template type use attribute__((atomic)). (template __atomic_base<_PTp*>): Likewise. * include/std/atomic (template atomic<_Tp>): Likewise. * testsuite/29_atomics/atomic/60695.cc: Adjust error line number. Index: gcc/c-family/c-common.c =================================================================== *** gcc/c-family/c-common.c (revision 222852) --- gcc/c-family/c-common.c (working copy) *************** static tree handle_externally_visible_at *** 349,354 **** --- 349,355 ---- static tree handle_no_reorder_attribute (tree *, tree, tree, int, bool *); static tree handle_const_attribute (tree *, tree, tree, int, bool *); + static tree handle_atomic_attribute (tree *, tree, tree, int, bool *); static tree handle_transparent_union_attribute (tree *, tree, tree, int, bool *); static tree handle_constructor_attribute (tree *, tree, tree, int, bool *); *************** const struct attribute_spec c_common_att *** 692,697 **** --- 693,700 ---- /* The same comments as for noreturn attributes apply to const ones. */ { "const", 0, 0, true, false, false, handle_const_attribute, false }, + { "atomic", 0, 0, false, true, false, + handle_atomic_attribute, false}, { "transparent_union", 0, 0, false, false, false, handle_transparent_union_attribute, false }, { "constructor", 0, 1, true, false, false, *************** handle_const_attribute (tree *node, tree *** 7196,7201 **** --- 7199,7234 ---- return NULL_TREE; } + /* Handle an "atomic" attribute; arguments as in + struct attribute_spec.handler. */ + + static tree + handle_atomic_attribute (tree *node, tree name, tree ARG_UNUSED (args), + int ARG_UNUSED (flags), bool *no_add_attrs) + { + bool ignored = true; + if (TYPE_P (*node) && TREE_CODE (*node) != ARRAY_TYPE + && TREE_CODE (*node) != FUNCTION_TYPE) + { + tree type = *node; + + if (!TYPE_ATOMIC (type)) + { + *node = build_qualified_type (type, + (TYPE_QUALS (type) | TYPE_QUAL_ATOMIC)); + ignored = false; + } + } + + if (ignored) + { + warning (OPT_Wattributes, "%qE attribute ignored", name); + *no_add_attrs = true; + } + return NULL_TREE; + } + + /* Handle a "transparent_union" attribute; arguments as in struct attribute_spec.handler. */ *************** sync_resolve_params (location_t loc, tre *** 10589,10595 **** ptype = TREE_TYPE (TREE_TYPE ((*params)[0])); ptype = TYPE_MAIN_VARIANT (ptype); ! /* For the rest of the values, we need to cast these to FTYPE, so that we don't get warnings for passing pointer types, etc. */ parmnum = 0; while (1) --- 10622,10628 ---- ptype = TREE_TYPE (TREE_TYPE ((*params)[0])); ptype = TYPE_MAIN_VARIANT (ptype); ! /* For the rest of the values, we need to cast these to PTYPE, so that we don't get warnings for passing pointer types, etc. */ parmnum = 0; while (1) *************** resolve_overloaded_builtin (location_t l *** 11232,11238 **** case BUILT_IN_SYNC_LOCK_RELEASE_N: { int n = sync_resolve_size (function, params); ! tree new_function, first_param, result; enum built_in_function fncode; if (n == 0) --- 11265,11271 ---- case BUILT_IN_SYNC_LOCK_RELEASE_N: { int n = sync_resolve_size (function, params); ! tree new_function, first_param, result, ptype; enum built_in_function fncode; if (n == 0) *************** resolve_overloaded_builtin (location_t l *** 11240,11250 **** fncode = (enum built_in_function)((int)orig_code + exact_log2 (n) + 1); new_function = builtin_decl_explicit (fncode); if (!sync_resolve_params (loc, function, new_function, params, orig_format)) return error_mark_node; - first_param = (*params)[0]; result = build_function_call_vec (loc, vNULL, new_function, params, NULL); if (result == error_mark_node) --- 11273,11298 ---- fncode = (enum built_in_function)((int)orig_code + exact_log2 (n) + 1); new_function = builtin_decl_explicit (fncode); + + /* If the first parameter is an atomic type, cast away the atomic + attribute. This will better match the expected first param type, + and facilitiate getting the correct type for the other parameters + as well as the return type. */ + first_param = (*params)[0]; + ptype = TREE_TYPE (TREE_TYPE (first_param)); + if (TYPE_ATOMIC (ptype)) + { + ptype = build_qualified_type (ptype, + (TYPE_QUALS (ptype) & ~TYPE_QUAL_ATOMIC)); + ptype = build_pointer_type (ptype); + first_param = build1 (VIEW_CONVERT_EXPR, ptype, first_param); + (*params)[0] = first_param; + } + if (!sync_resolve_params (loc, function, new_function, params, orig_format)) return error_mark_node; result = build_function_call_vec (loc, vNULL, new_function, params, NULL); if (result == error_mark_node) Index: gcc/c-family/c-pretty-print.c =================================================================== *** gcc/c-family/c-pretty-print.c (revision 222852) --- gcc/c-family/c-pretty-print.c (working copy) *************** pp_c_cv_qualifiers (c_pretty_printer *pp *** 194,200 **** if (qualifiers & TYPE_QUAL_ATOMIC) { ! pp_c_ws_string (pp, "_Atomic"); previous = true; } --- 194,201 ---- if (qualifiers & TYPE_QUAL_ATOMIC) { ! pp_c_ws_string (pp, (flag_isoc11 ! ? "_Atomic" : "__attribute__((atomic))")); previous = true; } Index: libstdc++-v3/include/bits/atomic_base.h =================================================================== *** libstdc++-v3/include/bits/atomic_base.h (revision 222852) --- libstdc++-v3/include/bits/atomic_base.h (working copy) *************** _GLIBCXX_BEGIN_NAMESPACE_VERSION *** 238,249 **** struct __atomic_base { private: ! typedef _ITp __int_type; static constexpr int _S_alignment = sizeof(_ITp) > alignof(_ITp) ? sizeof(_ITp) : alignof(_ITp); ! alignas(_S_alignment) __int_type _M_i; public: __atomic_base() noexcept = default; --- 238,253 ---- struct __atomic_base { private: ! typedef _ITp __int_type; static constexpr int _S_alignment = sizeof(_ITp) > alignof(_ITp) ? sizeof(_ITp) : alignof(_ITp); ! typedef _ITp __attribute__((atomic)) __atomic_int_type; ! __atomic_int_type _M_i; ! ! static_assert (_S_alignment == alignof(__atomic_int_type), ! "std:atomic_base does not match expected alignment"); public: __atomic_base() noexcept = default; *************** _GLIBCXX_BEGIN_NAMESPACE_VERSION *** 566,573 **** { private: typedef _PTp* __pointer_type; ! __pointer_type _M_p; // Factored out to facilitate explicit specialization. constexpr ptrdiff_t --- 570,578 ---- { private: typedef _PTp* __pointer_type; + typedef __pointer_type __attribute__((atomic)) __atomic_pointer_type; ! __atomic_pointer_type _M_p; // Factored out to facilitate explicit specialization. constexpr ptrdiff_t Index: libstdc++-v3/include/std/atomic =================================================================== *** libstdc++-v3/include/std/atomic (revision 222852) --- libstdc++-v3/include/std/atomic (working copy) *************** _GLIBCXX_BEGIN_NAMESPACE_VERSION *** 173,186 **** static constexpr int _S_alignment = _S_min_alignment > alignof(_Tp) ? _S_min_alignment : alignof(_Tp); - alignas(_S_alignment) _Tp _M_i; - static_assert(__is_trivially_copyable(_Tp), "std::atomic requires a trivially copyable type"); static_assert(sizeof(_Tp) > 0, "Incomplete or zero-sized types are not supported"); public: atomic() noexcept = default; ~atomic() noexcept = default; --- 173,190 ---- static constexpr int _S_alignment = _S_min_alignment > alignof(_Tp) ? _S_min_alignment : alignof(_Tp); static_assert(__is_trivially_copyable(_Tp), "std::atomic requires a trivially copyable type"); static_assert(sizeof(_Tp) > 0, "Incomplete or zero-sized types are not supported"); + typedef _Tp __attribute__((atomic)) __atomic_Tp; + __atomic_Tp _M_i; + + static_assert (_S_alignment == alignof(__atomic_Tp), + "std:atomic does not match expected alignment"); + public: atomic() noexcept = default; ~atomic() noexcept = default; Index: libstdc++-v3/testsuite/29_atomics/atomic/60695.cc =================================================================== *** libstdc++-v3/testsuite/29_atomics/atomic/60695.cc (revision 222852) --- libstdc++-v3/testsuite/29_atomics/atomic/60695.cc (working copy) *************** struct X { *** 27,30 **** char stuff[0]; // GNU extension, type has zero size }; ! std::atomic a; // { dg-error "not supported" "" { target *-*-* } 181 } --- 27,30 ---- char stuff[0]; // GNU extension, type has zero size }; ! std::atomic a; // { dg-error "not supported" "" { target *-*-* } 179 }