diff mbox

resurrect __attribute__((atomic)) work

Message ID 554A8612.6080007@redhat.com
State New
Headers show

Commit Message

Andrew MacLeod May 6, 2015, 9:22 p.m. UTC 
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

Comments

Sandra Loosemore May 6, 2015, 10:41 p.m. UTC | #1 
On 05/06/2015 03:22 PM, Andrew MacLeod wrote:

> 	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.

I see no documentation here....

-Sandra
Andrew MacLeod May 6, 2015, 10:55 p.m. UTC | #2 
On 05/06/2015 06:41 PM, Sandra Loosemore wrote:
> On 05/06/2015 03:22 PM, Andrew MacLeod wrote:
>
>>     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.
>
> I see no documentation here....
>
> -Sandra
>
There is none as yet.. its just a WIP...  Over the past year or so I've 
had a few  people inquire into what ever happened to it and/or expressed 
an interest in finishing it...   I extracted it from the old branch and  
brought it up to a compile-able state.  Whether its extended to work 
just on types, or flushed out to be virtually the same as C11 _Atomic, 
or eventually abandoned  remains to be seen..

All c++ needs is a hook into the type and size of an atomic... maybe it 
can be tweaked instead to just provide an object with the size and 
alignment of an atomic object, but not actually be atomic. C++ doesn't 
actually need an atomic object...   That is probably the simplest thing 
to do...

we can document however  it ends up.

Andrew
diff mbox

Patch

	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<X> a;  // { dg-error "not supported" "" { target *-*-* } 181 }
--- 27,30 ----
    char stuff[0]; // GNU extension, type has zero size
  };
  
! std::atomic<X> a;  // { dg-error "not supported" "" { target *-*-* } 179 }