Re: [PATCH] Add atomic type qualifier
On 07/26/2013 07:21 PM, Joseph S. Myers wrote: On Fri, 26 Jul 2013, Andrew MacLeod wrote: This patch adds an atomic type qualifier to GCC. It can be accessed via __attribute__((atomic)) or in C11 mode via the _Atomic keyword. Why the attribute - why not just the keyword? * When C11 refers to qualified types, by default it does not include atomic types (6.2.5#27). What's your rationale for including atomic in TYPE_QUALS rather than making it separate? With either approach, a review of every reference to qualifiers in the front end is needed to determine what's correct for atomic; did you find that including atomic in qualifiers in the implementation made for fewer changes? Ive gotten the expressions mostly working (= and op= work great), but am having some issues with plain loads and identifying the correct time to emit the load... I've also hacked up a rough approximation of what stdatomic.h would need. I also think I'll revisit how atomic values are marked in the front end... now that Im doing all the real work in the front end, I don't think I want that attribute all over the place like I did before. I may well decide to remove it from the qualifiers and make it a separate type now... So don't worry about reviewing those files... I likely wont get to this until I get back from vacation in a couple of weeks however. I will post the state of things before I go, both for my own recollection when I return, and in case anyone just happens to feel really interested and wants to have a look at some of the remaining required changes. ha. I wish.Its starting to get close tho. Andrew
Re: [PATCH] Add atomic type qualifier
On 07/30/2013 09:44 AM, Andrew MacLeod wrote: On 07/30/2013 09:16 AM, Joseph S. Myers wrot THis also means that for the 3 floating point operations all we need are RTL insn patterns, no buitin. And as with the other atomics, if the pattern I think something will also be needed to specify allocation of the fenv_t temporary (whether in memory or registers). doesnt exist, we just wont emit it. we could add a warning easily enough in this case. Note there's a difference between no need to emit it, no warning should be given (soft float) and need to emit it but patterns not yet written so warning should be given (hard float but patterns not yet implemented for the architecture). In fact, the flag could be the presence of the fenv_t variable.. Null for that variable field in the builtin indicate you don't need the patterns emitted. I';ll get back to you in a bit with the actual built-in's format once I poke around the existing one and see how I can leverage it. I rewrote all that code last year and it ought to be pretty simple to add new operand support. It better be anyway :-) I worked out the built-ins and what they need to do... and you know what? I'm not sure I see the point any more. I am going to give a shot at simply expanding this code right in the front end. For the floating and complex types I'll create temps and pass them to the generic atomic routines for load and compare_exchange... I should be able to directly call the same routines that sort out what can be mapped to lock free and what can't. And in the end, the optimizers can sort out how to make things better. that way we dont need any support anywhere else. (well, we may need 3 builtins for the floating point stuff.. I don't know.. I'll worry about that later.) On a side note, after Friday, I'm off for 2 weeks, so I'll be pretty quiet until the 19th or 20th. Btw, if anyone wants to take a stab at a stdatomic.h file, that would be OK with me :-) Andrew
Re: [PATCH] Add atomic type qualifier
On Mon, 29 Jul 2013, Andrew MacLeod wrote: Ive been poking at this today, and Im wondering what you think of the idea of adding a flag to MODIFY_EXPR, #define MODIFY_EXPR_IS_COMPOUND(NODE) MODIFY_EXPR_CHECK(NODE)-base.asm_written_flag and set that in the MODIFY_EXPR node when we create it from the x op= y form in the front end. That flag seems to be free for expressions. My suggestion is that the IR generated by the front end should make it completely explicit what may need retrying with a compare-and-exchange, rather than relying on non-obvious details to reconstruct the semantics required at gimplification time - there are too many transformations (folding etc.) that may happen on existing trees and no clear way to be confident that you can still identify all the operands accurately after such transformations. That is, an ATOMIC_COMPOUND_MODIFY_EXPR or similar, whose operands are: the LHS of the assignment; a temporary variable, old in C11 footnote 113; the RHS; and the old op val expression complete with the conversion to the type of the LHS. Gimplification would then (carry out the effects of stabilize_reference on the LHS and save_expr on the RHS and) do old = LHS; followed by the do-while compare-exchange loop. A flag on the expression could indicate that the floating-point semantics are required. I'd guess back ends would need to provide three insn patterns, corresponding to feholdexcept, feclearexcept and feupdateenv, that there'd be corresponding built-in functions for these used at gimplification time, and that a target hook would give the type used for fenv_t by these built-in functions (*not* necessarily the same as the fenv_t used by any implementation of the functions in libm). The target should also be able to declare that there's no support for floating-point exceptions (e.g. for soft-float) and so floating-point cases don't need any special handling. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH] Add atomic type qualifier
On 07/30/2013 07:41 AM, Joseph S. Myers wrote: On Mon, 29 Jul 2013, Andrew MacLeod wrote: Ive been poking at this today, and Im wondering what you think of the idea of adding a flag to MODIFY_EXPR, #define MODIFY_EXPR_IS_COMPOUND(NODE) MODIFY_EXPR_CHECK(NODE)-base.asm_written_flag and set that in the MODIFY_EXPR node when we create it from the x op= y form in the front end. That flag seems to be free for expressions. My suggestion is that the IR generated by the front end should make it completely explicit what may need retrying with a compare-and-exchange, rather than relying on non-obvious details to reconstruct the semantics required at gimplification time - there are too many transformations (folding etc.) that may happen on existing trees and no clear way to be confident that you can still identify all the operands accurately after such transformations. That is, an ATOMIC_COMPOUND_MODIFY_EXPR or similar, whose operands are: the LHS of the assignment; a temporary variable, old in C11 footnote 113; the RHS; and the old op val expression complete with the conversion to the type of the LHS. Gimplification would then (carry out the effects of stabilize_reference on the LHS and save_expr on the RHS and) do old = LHS; followed by the do-while compare-exchange loop. In fact, after thinking about it overnight, I came to similar conclusions... I believe it requires new builtin(s) for these operations. Something like __atomic_compound_assign (atomic_expr, enum atomic_operation_type, blah, blah,...) A call to this builtin would be generated right from the parser when it sees the op= expression, and the built-in can then travel throughout gimple as a normal atomic built-in operation like the rest. During expansion to RTL it can be turned into whatever sequence we happen to need. This is what happens currently with the various __atomic_fetch_op and __atomic_op_fetch. In fact, they are a subset of required operations, so I should be able to combine the implementation of those with this new one. Is C++ planning to match these behaviours in the atomic library? It would need to access this builtin as well so that the C++ template code can invoke it. A flag on the expression could indicate that the floating-point semantics are required. I'd guess back ends would need to provide three insn patterns, corresponding to feholdexcept, feclearexcept and feupdateenv, that there'd be corresponding built-in functions for these used at gimplification time, and that a target hook would give the type used for fenv_t by these built-in functions (*not* necessarily the same as the fenv_t used by any implementation of the functions in libm). The target should also be able to declare that there's no support for floating-point exceptions (e.g. for soft-float) and so floating-point cases don't need any special handling. I think the fact that it requires floating point sematics should be determinable from the types of the expressions involved. If there is a floating point somewhere, then we'll need to utilize the patterns. we'll still have the types, although it would certainly be easy enough to add a flag to the builtin... and maybe thats the way to go after all. THis also means that for the 3 floating point operations all we need are RTL insn patterns, no buitin. And as with the other atomics, if the pattern doesnt exist, we just wont emit it. we could add a warning easily enough in this case. I think we're somewhere good now :-) I guess I'll do the same thing for normal references to an atomic variable... issue the atomic load or atomic store directly from the parser... Andrew
Re: [PATCH] Add atomic type qualifier
On Tue, 30 Jul 2013, Andrew MacLeod wrote: A flag on the expression could indicate that the floating-point semantics are required. I'd guess back ends would need to provide three insn patterns, corresponding to feholdexcept, feclearexcept and feupdateenv, that there'd be corresponding built-in functions for these used at gimplification time, and that a target hook would give the type used for fenv_t by these built-in functions (*not* necessarily the same as the fenv_t used by any implementation of the functions in libm). The target should also be able to declare that there's no support for floating-point exceptions (e.g. for soft-float) and so floating-point cases don't need any special handling. I think the fact that it requires floating point sematics should be determinable from the types of the expressions involved. If there is a My reasoning for such a flag is: The gimplifier shouldn't need to know the details of the semantics for operand promotions, how various arithmetic on complex numbers is carried out, and how a result is converted back to the destination type for the store. Even if it's the language-specific gimplification code rather than the language-independent gimplifier, we still want those details to be in one place (currently build_binary_op for the binary operation semantics), shared by atomic and non-atomic operations. Hence my suggestion that the operands to the new built-in function / operation do not include the unmodified RHS, and do not directly specify the operation in question. Instead, one operand is an expression of the form (convert-to-LHS-type) (old op val) where old and val are the temporary variables given in C11 footnote 113 and probably allocated by the front end (val initialized once, old initialized once but then potentially changing each time through the loop). The trees for that expression would be generated by build_binary_op and contain everything required for the semantics of e.g. complex arithmetic. It's true that in the case where floating-point issues arise, floating-point types will be involved somewhere in this expression (and otherwise, they will not be - though the initializer for val might itself have involved floating-point arithmetic), but you'd need to search recursively for them; having a flag seems simpler. THis also means that for the 3 floating point operations all we need are RTL insn patterns, no buitin. And as with the other atomics, if the pattern I think something will also be needed to specify allocation of the fenv_t temporary (whether in memory or registers). doesnt exist, we just wont emit it. we could add a warning easily enough in this case. Note there's a difference between no need to emit it, no warning should be given (soft float) and need to emit it but patterns not yet written so warning should be given (hard float but patterns not yet implemented for the architecture). -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH] Add atomic type qualifier
On 07/30/2013 09:16 AM, Joseph S. Myers wrot My reasoning for such a flag is: Hence my suggestion that the operands to the new built-in function / operation do not include the unmodified RHS, and do not directly specify the operation in question. Instead, one operand is an expression of the form (convert-to-LHS-type) (old op val) where old and val are the temporary variables given in C11 footnote 113 and probably allocated by the front end (val initialized once, old initialized once but then potentially changing each time through the loop). The trees for that expression would be generated by build_binary_op and contain everything required for the semantics of e.g. complex arithmetic. It's true that in the case where floating-point issues arise, floating-point types will be involved somewhere in this expression (and otherwise, they will not be - though the initializer for val might itself have involved floating-point arithmetic), but you'd need to search recursively for them; having a flag seems simpler. I agree. THis also means that for the 3 floating point operations all we need are RTL insn patterns, no buitin. And as with the other atomics, if the pattern I think something will also be needed to specify allocation of the fenv_t temporary (whether in memory or registers). doesnt exist, we just wont emit it. we could add a warning easily enough in this case. Note there's a difference between no need to emit it, no warning should be given (soft float) and need to emit it but patterns not yet written so warning should be given (hard float but patterns not yet implemented for the architecture). In fact, the flag could be the presence of the fenv_t variable.. Null for that variable field in the builtin indicate you don't need the patterns emitted. I';ll get back to you in a bit with the actual built-in's format once I poke around the existing one and see how I can leverage it. I rewrote all that code last year and it ought to be pretty simple to add new operand support. It better be anyway :-) Amndrew
Re: [PATCH] Add atomic type qualifier
On 07/28/2013 03:34 PM, Joseph S. Myers wrote: On Fri, 26 Jul 2013, Andrew MacLeod wrote: What it doesn't do: * It doesn't implement the stdatomic.h header - do you intend that to be provided by GCC or glibc? (Substantive review of the full patch still to come.) I figured gcc would provide it... but I hadn't given it a ton of thought on whether that was good or bad. again, that sort of thing isn't really my strong suit :-) * It doesn't implement the C11 expression expansion into atomic built-ins. ie, you can't write: _Atomic int x; x = 0; and have the result be an atomic operation calling __atomic_store (x, 0). That will be in a follow on patch. So none of the expression expansion from C11 is included yet. This just enables that. The hardest part will probably be compound assignment to an atomic object where either operand of the assignment has floating-point type - see C11 footnote 113, but you can't actually use feholdexcept or feclearexcept or feupdateenv here because that would introduce libm dependencies, so back ends will need to provide appropriate insn patterns to do those operations inline. Blick. What were they smoking the night before... I guess we'll probably need to enhance the current atomic patterns in RTL...We should be able to figure out that its floating point and invoke the appropriate RTL pattern during expansion rather than an existing one.OR just frigging call libatomic and let it deal with it. :-) I guess there wouldnt be any other fallback available. Actually, thats a mess... no way for the librtary to know its floating point unless you tell it somehow with new entry points or somesuch.. very lame. I planned to do the work in gimplification... let the atomic decls through, and during gimplification, loads or stores of an atomic decl would be converted to the appropriate load or store builtin, and at the same time recognize the 'decl = decl op value' expression and replace those as appropriate with atomic_op_fetch operations. I had discussed this at some length with Lawrence crowl and Jeffrey Yasskin some time ago.. At gimplification time we no longer know whether the original form was decl op= val or decl = decl op val;, but the decision was that it is ok to recognize decl = decl op val and make that atomic.. it would still satisfy the language requirements.. Andrew
Re: [PATCH] Add atomic type qualifier
On Mon, 29 Jul 2013, Andrew MacLeod wrote: Blick. What were they smoking the night before... I guess we'll probably need to enhance the current atomic patterns in RTL...We should be able to figure out that its floating point and invoke the appropriate RTL pattern during expansion rather than an existing one.OR just frigging call libatomic and let it deal with it. :-) I guess there wouldnt be any other fallback available. Actually, thats a mess... no way for the librtary to know its floating point unless you tell it somehow with new entry points or somesuch.. very lame. Note that you only need *one* of the types to be floating-point for this issue to apply. If you have _Atomic char c; float f; c /= f; then all the same requirements apply; there may be exceptions to discard not just from the division, but from the conversion of a float division result to char. I planned to do the work in gimplification... let the atomic decls through, and during gimplification, loads or stores of an atomic decl would be converted to the appropriate load or store builtin, and at the same time recognize the 'decl = decl op value' expression and replace those as appropriate with atomic_op_fetch operations. I had discussed this at some length with Lawrence crowl and Jeffrey Yasskin some time ago.. At gimplification time we no longer know whether the original form was decl op= val or decl = decl op val;, but the decision was that it is ok to recognize decl = decl op val and make that atomic.. it would still satisfy the language requirements.. I think that's probably OK (though, is this a theorem of the formal modelling work that has been done on the memory model?), but note it's not just a decl but an arbitrary pointer dereference (the address of the lvalue is only evaluated once, no matter how many compare-and-exchange operations are needed), and the operation may end up looking like *ptr = (convert to type of *ptr) ((promote) *ptr op (promote) val) rather than a simple decl = decl op val. Or something more complicated if the operation involves complex numbers - look at what gets for mixed real / complex arithmetic, for example. Given _Atomic _Complex float f; double d; f += d; the atomicity is for the whole complex number (and so the compare-and-exchange needs to work on the whole number) although only the real part is modified by the addition. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH] Add atomic type qualifier
On 07/29/2013 10:59 AM, Andrew MacLeod wrote: Blick. What were they smoking the night before... I guess we'll probably need to enhance the current atomic patterns in RTL...We should be able to figure out that its floating point and invoke the appropriate RTL pattern during expansion rather than an existing one.OR just frigging call libatomic and let it deal with it. :-) I guess there wouldnt be any other fallback available. Actually, thats a mess... no way for the librtary to know its floating point unless you tell it somehow with new entry points or somesuch.. very lame. Actually, in hind sight, we will need new atomic builtins It lists other operators we need to support that we currently do not: They list *, / , % + -^ | we currently don't support *,/, modulus, right or left shift. Maybe the simple thing is simply to emit the compare_exchange loop for all these, and not bother libatomic with them at all.We can eventually add rtl patterns for them if we want better performance for these new bits. As for the floating point operations, emit the same loop, and if there is a target pattern emit it, otherwise issue a warning saying those footnote items are not properly protected or whatever. Also, If I read this right, arithmetic type also includes complex type does it not? which means we need to support this for complex as well... Does C++14(or onward) intend to support these additions as well? Andrew
Re: [PATCH] Add atomic type qualifier
On 07/29/2013 12:06 PM, Joseph S. Myers wrote: On Mon, 29 Jul 2013, Andrew MacLeod wrote: Blick. What were they smoking the night before... I guess we'll probably need to enhance the current atomic patterns in RTL...We should be able to figure out that its floating point and invoke the appropriate RTL pattern during expansion rather than an existing one.OR just frigging call libatomic and let it deal with it. :-) I guess there wouldnt be any other fallback available. Actually, thats a mess... no way for the librtary to know its floating point unless you tell it somehow with new entry points or somesuch.. very lame. Note that you only need *one* of the types to be floating-point for this issue to apply. If you have _Atomic char c; float f; c /= f; then all the same requirements apply; there may be exceptions to discard not just from the division, but from the conversion of a float division result to char. yes, unfortunately. but gimplification should turn this to: t0 = c t1 =(float) t0 t2 = t0 / f t3 = (char) t2 c = t3 so we simply see a floating point division bracketted by load and store from a TYPE_ATOMIC expression. so we'd should be able to recognize the need for the floating point extra stuff based on the type of the operation . and just wrap the whole blasted thing. I must say, I'm not a fan :-) I planned to do the work in gimplification... let the atomic decls through, and during gimplification, loads or stores of an atomic decl would be converted to the appropriate load or store builtin, and at the same time recognize the 'decl = decl op value' expression and replace those as appropriate with atomic_op_fetch operations. I had discussed this at some length with Lawrence crowl and Jeffrey Yasskin some time ago.. At gimplification time we no longer know whether the original form was decl op= val or decl = decl op val;, but the decision was that it is ok to recognize decl = decl op val and make that atomic.. it would still satisfy the language requirements.. I think that's probably OK (though, is this a theorem of the formal modelling work that has been done on the memory model?), but note it's not I have no idea if its a theorem or not. just a decl but an arbitrary pointer dereference (the address of the lvalue is only evaluated once, no matter how many compare-and-exchange operations are needed), and the operation may end up looking like *ptr = (convert to type of *ptr) ((promote) *ptr op (promote) val) rather than a simple decl = decl op val. Or something more complicated if I think gimplification takes care of that as well since all assignments have to be in the form DECL = VALUE op VALUE.. it constructs the sequence so that something like *op_expr += 1 is properly transformed to t0 = op_expr t1 = *t0 t2 = t1 + 1 *t0 = t2 With the TYPE_ATOMIC attribute set and rippled through the op_expr expression, we know that *t0 is atomic in nature, so t1 is an atomic load, *t0 is an atomic store, and looking back at t2 = t1 + 1 can see that this is an atomic += 1. same thing with a normal load of an expression... the TYPE_ATOMIC gimple attribute *ought* to tell us everything we need. the operation involves complex numbers - look at what gets for mixed real / complex arithmetic, for example. Given _Atomic _Complex float f; double d; f += d; the atomicity is for the whole complex number (and so the compare-and-exchange needs to work on the whole number) although only the real part is modified by the addition. complex I hadn't thought about until just now, I'll have to look. I know we can deal with parts on complex sometimes. Hopefully at gimplification time we still have the whole complex reference and if we just take care of that with the atomic builtins, we'll maintain the entire thing as we need. Andrew
Re: [PATCH] Add atomic type qualifier
On Mon, 29 Jul 2013, Andrew MacLeod wrote: complex I hadn't thought about until just now, I'll have to look. I know we can deal with parts on complex sometimes. Hopefully at gimplification time we still have the whole complex reference and if we just take care of that with the atomic builtins, we'll maintain the entire thing as we need. You have things in a fairly complicated form, building up COMPLEX_EXPRs out of operations on the individual parts of the complex operands. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH] Add atomic type qualifier
On 07/29/2013 12:42 PM, Joseph S. Myers wrote:
On Mon, 29 Jul 2013, Andrew MacLeod wrote:
complex I hadn't thought about until just now, I'll have to look. I know we
can deal with parts on complex sometimes. Hopefully at gimplification time
we still have the whole complex reference and if we just take care of that
with the atomic builtins, we'll maintain the entire thing as we need.
You have things in a fairly complicated form, building up COMPLEX_EXPRs
out of operations on the individual parts of the complex operands.
I tried:
__complex__ double d;
int main (void)
{
d = 0;
d = d + 5;
}
and it seems to break it into:
d = __complex__ (0.0, 0.0);
and
d.1 = d;
d.0 = d.1;
D.1723 = REALPART_EXPR d.0;
D.1724 = D.1723 + 5.0e+0;
D.1725 = IMAGPART_EXPR d.0;
d.2 = COMPLEX_EXPR D.1724, D.1725;
d = d.2;
so again the loads and stores to (D) appear to be completely wrap the
entire complex operation, so this should be handle-able the same way...
So you really should be able to key into the atomic load from D followed
by the store to D and look at whats in between.
I thini is is straightforward in the gimplifier, we ought to have the d
=d op V expression at some point and be able to detect that d is the
same and atomic, and check op. but if it turns out not to be, then I
could simply turn those atomic loads and stores into atomic loads and
stores in the gimplifier and stop there., Then have a very early
always-run SSA pass pattern match looking for atomic stores fed from
atomic loads, and examine the operations in between, looking for
patterns that match d = d op v, and then turning the loads/store and
intermediate bits into the specified compare_exchange loops...
I'll get to this shortly.
Andrew
Re: [PATCH] Add atomic type qualifier
On 07/29/2013 12:06 PM, Joseph S. Myers wrote: On Mon, 29 Jul 2013, Andrew MacLeod wrote: I planned to do the work in gimplification... let the atomic decls through, and during gimplification, loads or stores of an atomic decl would be converted to the appropriate load or store builtin, and at the same time recognize the 'decl = decl op value' expression and replace those as appropriate with atomic_op_fetch operations. I had discussed this at some length with Lawrence crowl and Jeffrey Yasskin some time ago.. At gimplification time we no longer know whether the original form was decl op= val or decl = decl op val;, but the decision was that it is ok to recognize decl = decl op val and make that atomic.. it would still satisfy the language requirements.. I think that's probably OK (though, is this a theorem of the formal modelling work that has been done on the memory model?), but note it's not just a decl but an arbitrary pointer dereference (the address of the lvalue is only evaluated once, no matter how many compare-and-exchange operations are needed), and the operation may end up looking like *ptr = (convert to type of *ptr) ((promote) *ptr op (promote) val) rather than a simple decl = decl op val. Or something more complicated if the operation involves complex numbers - look at what gets for mixed real / complex arithmetic, for example. Given _Atomic _Complex float f; double d; f += d; the atomicity is for the whole complex number (and so the compare-and-exchange needs to work on the whole number) although only the real part is modified by the addition. Ive been poking at this today, and Im wondering what you think of the idea of adding a flag to MODIFY_EXPR, #define MODIFY_EXPR_IS_COMPOUND(NODE) MODIFY_EXPR_CHECK(NODE)-base.asm_written_flag and set that in the MODIFY_EXPR node when we create it from the x op= y form in the front end. That flag seems to be free for expressions. It will then be trivial to locate these expressions and issue a builtin or the wrapper compare_exchange code during gimplification. We just check if MODIFY_EXPR_IS_COMPOUND() is true and TYPE_ATOMIC() is set on the expression type. (Ive already confirmed the atomic type is set as the attribute ripples up to the MODIFY_EXPR node's type.) then we know all the important bits from the MODIFY_EXPR to perform the operation. Otherwise, it looks like it can get a bit hairy... What do you think? As a side effect, we also only get it for the actual statements we care about as well. Andrew
Re: [PATCH] Add atomic type qualifier
On Fri, 26 Jul 2013, Andrew MacLeod wrote: What it doesn't do: * It doesn't implement the stdatomic.h header - do you intend that to be provided by GCC or glibc? (Substantive review of the full patch still to come.) * It doesn't implement the C11 expression expansion into atomic built-ins. ie, you can't write: _Atomic int x; x = 0; and have the result be an atomic operation calling __atomic_store (x, 0). That will be in a follow on patch. So none of the expression expansion from C11 is included yet. This just enables that. The hardest part will probably be compound assignment to an atomic object where either operand of the assignment has floating-point type - see C11 footnote 113, but you can't actually use feholdexcept or feclearexcept or feupdateenv here because that would introduce libm dependencies, so back ends will need to provide appropriate insn patterns to do those operations inline. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH] Add atomic type qualifier
Andrew MacLeod amacl...@redhat.com writes: What it doesn't do: * It doesn't implement the C11 expression expansion into atomic built-ins. ie, you can't write: _Atomic int x; x = 0; and have the result be an atomic operation calling __atomic_store (x, 0). How would this work if you want a different memory order? -Andi -- a...@linux.intel.com -- Speaking for myself only
Re: [PATCH] Add atomic type qualifier
On 07/26/2013 03:01 PM, Andi Kleen wrote: Andrew MacLeod amacl...@redhat.com writes: What it doesn't do: * It doesn't implement the C11 expression expansion into atomic built-ins. ie, you can't write: _Atomic int x; x = 0; and have the result be an atomic operation calling __atomic_store (x, 0). How would this work if you want a different memory order? The way the standard is defined, any implicit operation like that is seq_cst. If you want something other than seq-cst, you have to explicitly call atomic_store (x, 0, model). C11 also provides all those same atomic routines that c++11 provides for this reason. They just decided to try to implement the c++ atomic templates in the language along the way :-P so it feels very much like c++ atomics... Andrew
Re: [PATCH] Add atomic type qualifier
On Fri, 26 Jul 2013, Andrew MacLeod wrote: This patch adds an atomic type qualifier to GCC. It can be accessed via __attribute__((atomic)) or in C11 mode via the _Atomic keyword. HP, you might want to give this a try and see if you can get the alignment correct for the cris port finally :-) Looks like the means to that end are there now. Thanks! Though I won't be able to look into it for a while. (Looking at two more weeks of vacation and likely a hectic period after that.) Note to self (mostly): also implement target-specific warning when the layout of a composite type including an atomic (naturally aligned) type would differ from normal (unaligned; packed) layout. brgds, H-P
Re: [PATCH] Add atomic type qualifier
Andrew MacLeod amacl...@redhat.com writes: The way the standard is defined, any implicit operation like that is seq_cst. If you want something other than seq-cst, you have to explicitly call atomic_store (x, 0, model). Thanks. This doesn't sound like a good default for x86. seq_cst requires somewhat expensive extra barriers that most people most likely don't need. -Andi -- a...@linux.intel.com -- Speaking for myself only
Re: [PATCH] Add atomic type qualifier
On 07/26/2013 05:13 PM, Andi Kleen wrote: Andrew MacLeod amacl...@redhat.com writes: The way the standard is defined, any implicit operation like that is seq_cst. If you want something other than seq-cst, you have to explicitly call atomic_store (x, 0, model). Thanks. This doesn't sound like a good default for x86. seq_cst requires somewhat expensive extra barriers that most people most likely don't need. Its the only truly safe default for a multi-threaded environment. These are the same defaults you get with C++11 as well if you don't explicitly use a memory model in your atomic operations: ie __int_type load(memory_order __m = memory_order_seq_cst) const noexcept So if you don't want seq-cst, you need to specify exactly what you do want. Andrew
Re: [PATCH] Add atomic type qualifier
On Fri, 26 Jul 2013, Andrew MacLeod wrote: This patch adds an atomic type qualifier to GCC. It can be accessed via __attribute__((atomic)) or in C11 mode via the _Atomic keyword. Why the attribute - why not just the keyword? I'll review the patch in detail later (which will involve checking each reference to atomic or qualified in the language part of C11, checking for appropriate implementation and complaining if it appears to be missing, and checking for appropriate testcases and complaining if those appear to be missing). But some comments now: * pedwarns for using a C11 feature in previous standard modes should as per usual practice be pedwarns-if-pedantic. * I don't see anything obvious in the parser changes to implement the _Atomic ( type-name ) version of the syntax for atomic types (6.7.2.4). * When C11 refers to qualified types, by default it does not include atomic types (6.2.5#27). What's your rationale for including atomic in TYPE_QUALS rather than making it separate? With either approach, a review of every reference to qualifiers in the front end is needed to determine what's correct for atomic; did you find that including atomic in qualifiers in the implementation made for fewer changes? -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH] Add atomic type qualifier
On 07/26/2013 07:21 PM, Joseph S. Myers wrote: On Fri, 26 Jul 2013, Andrew MacLeod wrote: This patch adds an atomic type qualifier to GCC. It can be accessed via __attribute__((atomic)) or in C11 mode via the _Atomic keyword. Why the attribute - why not just the keyword? 2 reasons: 1 - we currently have at least one target who cannot express their alignment requirements for an atomic value (ie, a short with 4 byte alignment). Use of__attribute((atomic)) will allow proper expression and usage within the atomic built-ins without using C11. 2 - compatibility with C11 and C++11. Ultimately, the atomic object can be upsized (ie,a 6 byte object sized up to be 8 bytes). _Atomic would do that for C11, but we cant really use _Atomic within the c++11 template files, so __attribute__((atomic)) seemed pretty natural. As well, this will allow c++ atomic templates to work on aforementioned target(s). I'll review the patch in detail later (which will involve checking each reference to atomic or qualified in the language part of C11, checking for appropriate implementation and complaining if it appears to be missing, and checking for appropriate testcases and complaining if those appear to be missing). But some comments now: * pedwarns for using a C11 feature in previous standard modes should as per usual practice be pedwarns-if-pedantic. * I don't see anything obvious in the parser changes to implement the _Atomic ( type-name ) version of the syntax for atomic types (6.7.2.4). * When C11 refers to qualified types, by default it does not include atomic types (6.2.5#27). What's your rationale for including atomic in TYPE_QUALS rather than making it separate? With either approach, a review of every reference to qualifiers in the front end is needed to determine what's correct for atomic; did you find that including atomic in qualifiers in the implementation made for fewer changes? I'm not good at front ends... don't really know them at all. Parsing and syntax and semantics scare me. This was my initial attempt to enable _Atomic and hoped for help with the rest from those who do :-) observation and changes welcome. Andrew
