Re: Implement C11 _Atomic

[Andrew MacLeod]

On 11/21/2013 06:40 PM, Hans-Peter Nilsson wrote:

On Thu, 21 Nov 2013, Hans-Peter Nilsson wrote:

with this/these patches
at least I'll be able to tell people that _Atomic for C11 works.

Oh right, gcc still doesn't remove target-introduced manual
alignment checks (when expanding atomic intrinsics), but at
least gcc makes sure it's aligned on stack, when options doesn't
say it's aligned.  And a.c:plugh2 doesn't seem to perform an
atomic assignment, but just assignment through an
_Atomic-aligned stack temporary.  Might be my C11-ignorance
showing.

(Without the patches layout and alignment is all broken.)

brgds, H-P

The target hook patch is checked into mainline, revision 205273.

Andrew

Re: Implement C11 _Atomic

[Hans-Peter Nilsson]

On Fri, 22 Nov 2013, Andrew MacLeod wrote:
 The target hook patch is checked into mainline, revision 205273.

Thanks!

The target patch is there too now; tested with the previous
version of the hook-patch.  I'm confident my autotester will
yell at me if I goofed.

gcc:
* config/cris/cris.c (cris_atomic_align_for_mode): New function.
(TARGET_ATOMIC_ALIGN_FOR_MODE): Define.

Index: config/cris/cris.c
===
--- config/cris/cris.c  (revision 205225)
+++ config/cris/cris.c  (working copy)
@@ -93,6 +93,8 @@ static int cris_reg_overlap_mentioned_p
 static enum machine_mode cris_promote_function_mode (const_tree, enum 
machine_mode,
 int *, const_tree, int);

+static unsigned int cris_atomic_align_for_mode (enum machine_mode);
+
 static void cris_print_base (rtx, FILE *);

 static void cris_print_index (rtx, FILE *);
@@ -227,6 +229,9 @@ int cris_cpu_version = CRIS_DEFAULT_CPU_
 #undef TARGET_PROMOTE_FUNCTION_MODE
 #define TARGET_PROMOTE_FUNCTION_MODE cris_promote_function_mode

+#undef TARGET_ATOMIC_ALIGN_FOR_MODE
+#define TARGET_ATOMIC_ALIGN_FOR_MODE cris_atomic_align_for_mode
+
 #undef TARGET_STRUCT_VALUE_RTX
 #define TARGET_STRUCT_VALUE_RTX cris_struct_value_rtx
 #undef TARGET_SETUP_INCOMING_VARARGS
@@ -4018,6 +4023,14 @@ cris_promote_function_mode (const_tree t
 return mode;
   return CRIS_PROMOTED_MODE (mode, *punsignedp, type);
 }
+
+/* Atomic types require alignment to be at least their natural size.  */
+
+static unsigned int
+cris_atomic_align_for_mode (enum machine_mode mode)
+{
+  return GET_MODE_BITSIZE (mode);
+}

 /* Let's assume all functions return in r[CRIS_FIRST_ARG_REG] for the
time being.  */

brgds, H-P

Re: Implement C11 _Atomic

[Hans-Peter Nilsson]

On Tue, 5 Nov 2013, Joseph S. Myers wrote:

Thanks for doing this!  However, without examples I have trouble
reading out the bits I need as a target maintainer, and I can't
read out the answers from the patch, so pardon a few questions.

 This patch, relative to trunk and based on work done on the C11-atomic
 branch, adds support for C11 _Atomic.  It is intended to include all
 the required language support.

 It does not include the stdatomic.h header; there's a version on the
 branch, but it needs further review against the standard and test
 coverage adding to the testsuite before I can propose it for mainline.

 Support for atomic types having bigger alignment than the
 corresponding non-atomic types is limited: it includes the code to
 increase the alignment of types whose size is exactly 1, 2, 4, 8 or 16
 to that of the corresponding integer type [*], but not anything for
 target-specific alignment increases.

Target-maintainer perspective here: do I read that correctly,
that by default adding _Atomic raises the alignment of that type
to the natural one, for all targets?

To wit,

 There's code for target-specific
 alignment on the branch (and I intend to merge trunk back to the
 branch once this patch is on trunk, so it's easy to tell what the
 changes still left on the branch are), should any target maintainers
 wish to have such alignment.

...is that part needed for alignment that is only
target-specific and other-than-natural?  For example, 8-byte
aligment where required for atomic 4-byte types?

Or is that part also required for
anything-other-than-ordinary-C-type alignment for the target;
say, natural 4-byte alignment of 4-byte-types for targets where
alignment is otherwise packed; where only 1-byte alignment of
the basic type is ABI-mandated?

brgds, H-P

Re: Implement C11 _Atomic

[Hans-Peter Nilsson]

On Thu, 21 Nov 2013, Andrew MacLeod wrote:
  Or is that part also required for
  anything-other-than-ordinary-C-type alignment for the target;
  say, natural 4-byte alignment of 4-byte-types for targets where
  alignment is otherwise packed; where only 1-byte alignment of
  the basic type is ABI-mandated?
 
 If I understand correctly, yes, it would be needed there as well if the
 compiler thinks alignof (int) is 1, then I believe it will set alignof(_Atomic
 int) to 1 as well, and that's probably not good.

Right.

 Basically, atomic_types are given their own type nodes in tree.c:
   atomicQI_type_node = build_atomic_base (unsigned_intQI_type_node);
   atomicHI_type_node = build_atomic_base (unsigned_intHI_type_node);
   atomicSI_type_node = build_atomic_base (unsigned_intSI_type_node);
   atomicDI_type_node = build_atomic_base (unsigned_intDI_type_node);
   atomicTI_type_node = build_atomic_base (unsigned_intTI_type_node);

It sounds like I should be able to hack that from the port in
some other tree initializer hook?

I'm trying to avoid ABI breakage of course.  I'd rather not have
to ask people not to use _Atomic with 4.9 for CRIS ports using
official releases or have ABI breakage with the next release.
Maybe there's one other port in the same situation...

 and on the branch that code instead looks something like:

 #define SET_ATOMIC_TYPE_NODE(TYPE, MODE, DEFAULT)   \
  (TYPE) = build_atomic_base (DEFAULT, targetm.atomic_align_for_mode (MODE));

   SET_ATOMIC_TYPE_NODE (atomicQI_type_node, QImode, unsigned_intQI_type_node);
 ...

 which provides a target hook to override the default values and a target can
 set them to whatever it deems necessary.

Yah, that'd be nice.  Doesn't sound like more than the target
hook and the patched lines above left for that to happen,
though?  Or perhaps that's a too-naive assumption. I guess I
should have a look.

 There was insufficient time to test and fully flush this out, so it hasn't
 made it into mainline.  Its only thanks to Josephs heroic efforts we have C11
 :-)

 I don't think its a lot of code if you wanted to fool with it for your port.

So, what would be needed in terms of testing and coding to get
that part into 4.9?

brgds, H-P

Re: Implement C11 _Atomic

[Hans-Peter Nilsson]

On Thu, 21 Nov 2013, Andrew MacLeod wrote:
 I can bootstrap and check this on x86 to make sure it doesnt affect anything,
 and you can fool with it and see if you can get your desired results with your
 port.

Success!

For the record, tested together with the attached patch for the
CRIS ports, both regularly (not finished, but done with the C
testsuite part and no regressions there), as well as manually
for the attached test-programs, compiling and inspecting output
for different sub-targets and checking that data layout,
alignment and size is as intended.

Too bad about the libstdc++ atomics, but with this/these patches
at least I'll be able to tell people that _Atomic for C11 works.

Thanks to the both of you!

brgds, H-PIndex: config/cris/cris.c
===
--- config/cris/cris.c  (revision 205225)
+++ config/cris/cris.c  (working copy)
@@ -93,6 +93,8 @@ static int cris_reg_overlap_mentioned_p 
 static enum machine_mode cris_promote_function_mode (const_tree, enum 
machine_mode,
 int *, const_tree, int);
 
+static unsigned int cris_atomic_align_for_mode (enum machine_mode);
+
 static void cris_print_base (rtx, FILE *);
 
 static void cris_print_index (rtx, FILE *);
@@ -227,6 +229,9 @@ int cris_cpu_version = CRIS_DEFAULT_CPU_
 #undef TARGET_PROMOTE_FUNCTION_MODE
 #define TARGET_PROMOTE_FUNCTION_MODE cris_promote_function_mode
 
+#undef TARGET_ATOMIC_ALIGN_FOR_MODE
+#define TARGET_ATOMIC_ALIGN_FOR_MODE cris_atomic_align_for_mode
+
 #undef TARGET_STRUCT_VALUE_RTX
 #define TARGET_STRUCT_VALUE_RTX cris_struct_value_rtx
 #undef TARGET_SETUP_INCOMING_VARARGS
@@ -4018,6 +4023,14 @@ cris_promote_function_mode (const_tree t
 return mode;
   return CRIS_PROMOTED_MODE (mode, *punsignedp, type);
 } 
+
+/* Atomic types require alignment to be at least the natural size. */
+
+static unsigned int
+cris_atomic_align_for_mode (enum machine_mode mode)
+{
+  return GET_MODE_BITSIZE (mode);
+}
 
 /* Let's assume all functions return in r[CRIS_FIRST_ARG_REG] for the
time being.  */
struct r0 {
  char a;
  int b;
};

struct l0 {
  char a;
  int b __attribute__((__aligned__(4)));
};

struct a0 {
  char a;
  _Atomic int b;
};

#define test(name, cond) typedef int test_ ## name [-!(cond)]

struct a0 ag;
struct l0 lg;
struct r0 rg;

struct a0 ai = {0, 1};
struct l0 li = {1, 2};
struct r0 ri = {2, 3};

extern struct a0 ae;
extern struct l0 le;
extern struct r0 re;

void foo(void)
{
  struct a0 al;
  struct l0 ll;
  struct r0 rl;
  test(a_al_ge_4, __alignof__(al) = 4);
  test(a_al_ge_ll, __alignof__(al) = __alignof__(ll));
  test(a_al_ge_rl, __alignof__(al) = __alignof__(rl));
  test(a_ag_ge_4, __alignof__(ag) = 4);
  test(a_ag_ge_lg, __alignof__(ag) = __alignof__(lg));
  test(a_ag_ge_rg, __alignof__(ag) = __alignof__(rg));
  test(a_ai_ge_4, __alignof__(ai) = 4);
  test(a_ai_ge_li, __alignof__(ai) = __alignof__(li));
  test(a_ai_ge_ri, __alignof__(ai) = __alignof__(ri));
  test(a_ae_ge_4, __alignof__(ae) = 4);
  test(a_ae_ge_le, __alignof__(ae) = __alignof__(le));
  test(a_ae_ge_re, __alignof__(ae) = __alignof__(re));

  test(s_al_ge_4, sizeof(al) = 8);
  test(s_al_ge_ll, sizeof(al) = sizeof(ll));
  test(s_al_ge_rl, sizeof(al) = sizeof(rl));
  test(s_ag_ge_4, sizeof(ag) = 8);
  test(s_ag_ge_lg, sizeof(ag) = sizeof(lg));
  test(s_ag_ge_rg, sizeof(ag) = sizeof(rg));
  test(s_ai_ge_4, sizeof(ai) = 8);
  test(s_ai_ge_lg, sizeof(ai) = sizeof(li));
  test(s_ai_ge_rg, sizeof(ai) = sizeof(ri));
  test(s_ae_ge_4, sizeof(ae) = 8);
  test(s_ae_ge_le, sizeof(ae) = sizeof(le));
  test(s_ae_ge_re, sizeof(ae) = sizeof(re));

  test(ab_eq_4, __builtin_offsetof(struct a0, b) == 4);
  al = ai;
  al.b++;
  ae = al;
}
_Atomic int foo;
int bar;
void baz(void)
{
  foo++;
}
void foobar(void)
{
  bar++;
}

void xyzzy(int *x)
{
  (*x)++;
}

void plugh(_Atomic int *x)
{
  (*x)++;
}

void xyzzy1(int *x)
{
  int y = *x;
  *x = y+1;
}

void plugh2(_Atomic int *x)
{
  _Atomic int y = *x;
  *x = y+1;
}

Re: Implement C11 _Atomic

[Andrew MacLeod]

On 11/21/2013 06:23 PM, Hans-Peter Nilsson wrote:

On Thu, 21 Nov 2013, Andrew MacLeod wrote:

I can bootstrap and check this on x86 to make sure it doesnt affect anything,
and you can fool with it and see if you can get your desired results with your
port.

Success!

For the record, tested together with the attached patch for the
CRIS ports, both regularly (not finished, but done with the C
testsuite part and no regressions there), as well as manually
for the attached test-programs, compiling and inspecting output
for different sub-targets and checking that data layout,
alignment and size is as intended.

Too bad about the libstdc++ atomics, but with this/these patches
at least I'll be able to tell people that _Atomic for C11 works.

Thanks to the both of you!


All we need is a way to communicate the atomic property to the type 
within the libstdc++ template...  We cant use _Atomic there :-P


 I originally had created an __attribute__ ((atomic)) whch you could 
apply to the atomic template type and get the same behaviour  as 
_Atomic, Im not sure if there is another way or not.  The atomic 
template is generic, so I couldn't think of any special macro wizardry 
we could define...


Andrew

Re: Implement C11 _Atomic

[Hans-Peter Nilsson]

On Thu, 21 Nov 2013, Hans-Peter Nilsson wrote:
 with this/these patches
 at least I'll be able to tell people that _Atomic for C11 works.

Oh right, gcc still doesn't remove target-introduced manual
alignment checks (when expanding atomic intrinsics), but at
least gcc makes sure it's aligned on stack, when options doesn't
say it's aligned.  And a.c:plugh2 doesn't seem to perform an
atomic assignment, but just assignment through an
_Atomic-aligned stack temporary.  Might be my C11-ignorance
showing.

(Without the patches layout and alignment is all broken.)

brgds, H-P

Re: Implement C11 _Atomic

[Joseph S. Myers]

On Thu, 21 Nov 2013, Hans-Peter Nilsson wrote:

 Oh right, gcc still doesn't remove target-introduced manual
 alignment checks (when expanding atomic intrinsics), but at
 least gcc makes sure it's aligned on stack, when options doesn't
 say it's aligned.  And a.c:plugh2 doesn't seem to perform an
 atomic assignment, but just assignment through an
 _Atomic-aligned stack temporary.  Might be my C11-ignorance
 showing.

It appears to me on x86_64 to produce an __atomic_store_4 to *x (in the 
GIMPLE dumps, what happens after that is a matter for the back end).

Note that atomic variable initialization is *not* atomic (see 7.17.2.1 - 
in general ATOMIC_VAR_INIT needs using with the initializer, or the 
initialization needs to be carried out with atomic_init, though GCC 
doesn't require that).  (In C11, the effect of a plain initialization 
without ATOMIC_VAR_INIT is I think that the initializer is evaluated for 
its side effects, but if the variable gets used as either rvalue or lvalue 
without one of the special forms of initialization being used first then 
the behavior is undefined.  The idea is to support implementations with 
extra bits in atomic objects used for locking purposes.)  So no atomic 
store to y is expected - although there are atomic loads from y.

-- 
Joseph S. Myers
jos...@codesourcery.com

Re: Implement C11 _Atomic

[Andrew MacLeod]

On 11/21/2013 06:05 AM, Hans-Peter Nilsson wrote:

On Tue, 5 Nov 2013, Joseph S. Myers wrote:

Thanks for doing this!  However, without examples I have trouble
reading out the bits I need as a target maintainer, and I can't
read out the answers from the patch, so pardon a few questions.



Support for atomic types having bigger alignment than the
corresponding non-atomic types is limited: it includes the code to
increase the alignment of types whose size is exactly 1, 2, 4, 8 or 16
to that of the corresponding integer type [*], but not anything for
target-specific alignment increases.

Target-maintainer perspective here: do I read that correctly,
that by default adding _Atomic raises the alignment of that type
to the natural one, for all targets?

To wit,


It sets up the default alignment for all atomic integral types to be 
that of the natural alignment of their integer counterpart. ie 
alignof(_Atomic short) = alignof (short)


  For non-integral types, if sizeof (type) == sizeof (integral-type), 
then it makes sure that  alignof (type) is at least as large as alignof 
(_Atomic integral-type), and overrides it if necessary, allowing the 
lock-free routines to work the same for non-integral types.






There's code for target-specific
alignment on the branch (and I intend to merge trunk back to the
branch once this patch is on trunk, so it's easy to tell what the
changes still left on the branch are), should any target maintainers
wish to have such alignment.

...is that part needed for alignment that is only
target-specific and other-than-natural?  For example, 8-byte
aligment where required for atomic 4-byte types?
Yes, I believe the override code is still on the branch. There is a 
target hook which allows the alignment for an atomic type to be changed.


So, the cris port would be able to override the alignment of the 
atomicHI_type_node and make it 4 bytes aligned,  which would then cause 
any variable declared as _Atomic short to automatically be 4 byte aligned.





Or is that part also required for
anything-other-than-ordinary-C-type alignment for the target;
say, natural 4-byte alignment of 4-byte-types for targets where
alignment is otherwise packed; where only 1-byte alignment of
the basic type is ABI-mandated?

If I understand correctly, yes, it would be needed there as well if 
the compiler thinks alignof (int) is 1, then I believe it will set 
alignof(_Atomic int) to 1 as well, and that's probably not good.


Basically, atomic_types are given their own type nodes in tree.c:
  atomicQI_type_node = build_atomic_base (unsigned_intQI_type_node);
  atomicHI_type_node = build_atomic_base (unsigned_intHI_type_node);
  atomicSI_type_node = build_atomic_base (unsigned_intSI_type_node);
  atomicDI_type_node = build_atomic_base (unsigned_intDI_type_node);
  atomicTI_type_node = build_atomic_base (unsigned_intTI_type_node);

and on the branch that code instead looks something like:

#define SET_ATOMIC_TYPE_NODE(TYPE, MODE, DEFAULT)   \
 (TYPE) = build_atomic_base (DEFAULT, targetm.atomic_align_for_mode 
(MODE));


  SET_ATOMIC_TYPE_NODE (atomicQI_type_node, QImode, 
unsigned_intQI_type_node);

...

which provides a target hook to override the default values and a target 
can set them to whatever it deems necessary.


There was insufficient time to test and fully flush this out, so it 
hasn't made it into mainline.  Its only thanks to Josephs heroic efforts 
we have C11 :-)


I don't think its a lot of code if you wanted to fool with it for your port.

Andrew

Re: Implement C11 _Atomic

[Andrew MacLeod]

On 11/21/2013 10:20 AM, Hans-Peter Nilsson wrote:

On Thu, 21 Nov 2013, Andrew MacLeod wrote:

Or is that part also required for
anything-other-than-ordinary-C-type alignment for the target;
say, natural 4-byte alignment of 4-byte-types for targets where
alignment is otherwise packed; where only 1-byte alignment of
the basic type is ABI-mandated?


If I understand correctly, yes, it would be needed there as well if the
compiler thinks alignof (int) is 1, then I believe it will set alignof(_Atomic
int) to 1 as well, and that's probably not good.

Right.


Basically, atomic_types are given their own type nodes in tree.c:
   atomicQI_type_node = build_atomic_base (unsigned_intQI_type_node);
   atomicHI_type_node = build_atomic_base (unsigned_intHI_type_node);
   atomicSI_type_node = build_atomic_base (unsigned_intSI_type_node);
   atomicDI_type_node = build_atomic_base (unsigned_intDI_type_node);
   atomicTI_type_node = build_atomic_base (unsigned_intTI_type_node);

It sounds like I should be able to hack that from the port in
some other tree initializer hook?

I'm trying to avoid ABI breakage of course.  I'd rather not have
to ask people not to use _Atomic with 4.9 for CRIS ports using
official releases or have ABI breakage with the next release.
Maybe there's one other port in the same situation...


None I am aware of, but that doesn't mean much :-)

and on the branch that code instead looks something like:

#define SET_ATOMIC_TYPE_NODE(TYPE, MODE, DEFAULT)   \
  (TYPE) = build_atomic_base (DEFAULT, targetm.atomic_align_for_mode (MODE));

   SET_ATOMIC_TYPE_NODE (atomicQI_type_node, QImode, unsigned_intQI_type_node);
...

which provides a target hook to override the default values and a target can
set them to whatever it deems necessary.

Yah, that'd be nice.  Doesn't sound like more than the target
hook and the patched lines above left for that to happen,
though?  Or perhaps that's a too-naive assumption. I guess I
should have a look.


That's all that is in theory needed, but it hasnt been tested very well.


There was insufficient time to test and fully flush this out, so it hasn't
made it into mainline.  Its only thanks to Josephs heroic efforts we have C11
:-)

I don't think its a lot of code if you wanted to fool with it for your port.

So, what would be needed in terms of testing and coding to get
that part into 4.9?




If we add the hook for atomic_align_for_mode, and change the initalizers 
in tree.c, any target which doesnt need/use the hook should be 
unaffected. So everything remains as it is today.


So Putting the hook in shouldn't be an issue.  Then you can experiment 
with it on your port and see if you get the desired effect...


I've attached what I think the remaining bits are regarding the 
alignment.All untested since I wrote it of course :-)  In any case,  
you should just need to provide a function to override the alignment for 
whatever mode(s)  you need with this...


I can bootstrap and check this on x86 to make sure it doesnt affect 
anything, and you can fool with it and see if you can get your desired 
results with your port.


Andrew


Index: doc/tm.texi
===
*** doc/tm.texi	(revision 205220)
--- doc/tm.texi	(working copy)
*** The support includes the assembler, link
*** 11500,11505 
--- 11500,11509 
  The default value of this hook is based on target's libc.
  @end deftypefn
  
+ @deftypefn {Target Hook} {unsigned int} TARGET_ATOMIC_ALIGN_FOR_MODE (enum machine_mode @var{mode})
+ If defined, this function returns an appropriate alignment in bits for an atomic object of machine_mode @var{mode}.  If 0 is returned then the default alignment for the specified mode is used. 
+ @end deftypefn
+ 
  @deftypefn {Target Hook} void TARGET_ATOMIC_ASSIGN_EXPAND_FENV (tree *@var{hold}, tree *@var{clear}, tree *@var{update})
  ISO C11 requires atomic compound assignments that may raise floating-point exceptions to raise exceptions corresponding to the arithmetic operation whose result was successfully stored in a compare-and-exchange sequence.  This requires code equivalent to calls to @code{feholdexcept}, @code{feclearexcept} and @code{feupdateenv} to be generated at appropriate points in the compare-and-exchange sequence.  This hook should set @code{*@var{hold}} to an expression equivalent to the call to @code{feholdexcept}, @code{*@var{clear}} to an expression equivalent to the call to @code{feclearexcept} and @code{*@var{update}} to an expression equivalent to the call to @code{feupdateenv}.  The three expressions are @code{NULL_TREE} on entry to the hook and may be left as @code{NULL_TREE} if no code is required in a particular place.  The default implementation leaves all three expressions as @code{NULL_TREE}.  The @code{__atomic_feraiseexcept} function from @code{libatomic} may be of use as part of the code generated in @code{*@var{update}}.
  @end deftypefn
Index: doc/tm.texi.in

Re: Implement C11 _Atomic

[Joseph S. Myers]

On Thu, 21 Nov 2013, Andrew MacLeod wrote:

  I'm trying to avoid ABI breakage of course.  I'd rather not have
  to ask people not to use _Atomic with 4.9 for CRIS ports using
  official releases or have ABI breakage with the next release.
  Maybe there's one other port in the same situation...
 
 None I am aware of, but that doesn't mean much :-)

It has been suggested that hppa may have interesting issues for atomics - 
but I'd think atomics support for it in GCC would mainly be for GNU/Linux, 
where you have kernel helpers that may avoid the issues with the 
underlying architecture.

Note that if you want libstdc++ atomics to be ABI compatible with C 
atomics then there may be more work to do on that side (again, there are 
pieces on the branch).

-- 
Joseph S. Myers
jos...@codesourcery.com

Re: Implement C11 _Atomic

[Andrew MacLeod]

On 11/21/2013 11:32 AM, Joseph S. Myers wrote:

On Thu, 21 Nov 2013, Andrew MacLeod wrote:


I'm trying to avoid ABI breakage of course.  I'd rather not have
to ask people not to use _Atomic with 4.9 for CRIS ports using
official releases or have ABI breakage with the next release.
Maybe there's one other port in the same situation...

None I am aware of, but that doesn't mean much :-)

It has been suggested that hppa may have interesting issues for atomics -
but I'd think atomics support for it in GCC would mainly be for GNU/Linux,
where you have kernel helpers that may avoid the issues with the
underlying architecture.

Note that if you want libstdc++ atomics to be ABI compatible with C
atomics then there may be more work to do on that side (again, there are
pieces on the branch).

I was just about to point out that  :-)that would be a 
shortcoming... 4.9 C++11 atomics do not use the same mechanism yet, so 
they would still behave the same as 4.8 C++ atomics, even with the 
override present C11 _Atomic variables would be the only beneficiary 
at this point of the override.


Andrew

Re: Implement C11 _Atomic

[Hans-Peter Nilsson]

On Thu, 21 Nov 2013, Andrew MacLeod wrote:
 If we add the hook for atomic_align_for_mode, and change the initalizers in
 tree.c, any target which doesnt need/use the hook should be unaffected. So
 everything remains as it is today.

 So Putting the hook in shouldn't be an issue.  Then you can experiment with it
 on your port and see if you get the desired effect...

 I've attached what I think the remaining bits are regarding the alignment.

Right, that's about what I expected.  Nice.

Of course, I'd argue that a better default for the atomic
alignment is the max of the default alignment and the natural
alignment, since *you can't have atomic support without extra
machinery for misaligned data* - straddling a page or cache
boundary has dire consequences.

But then the patch would not have the nice property of
obviously being a NOP elsewhere.  Then again, where it isn't a
NOP, you have breakage anyway.  Bah.

 All untested since I wrote it of course :-)  In any case,  you should just
 need to provide a function to override the alignment for whatever mode(s)  you
 need with this...

 I can bootstrap and check this on x86 to make sure it doesnt affect anything,
 and you can fool with it and see if you can get your desired results with your
 port.

Thanks!  I'll play with it and get back.

brgds, H-P

Re: Implement C11 _Atomic

[Uros Bizjak]

On Thu, Nov 7, 2013 at 7:26 PM, Joseph S. Myers jos...@codesourcery.com wrote:

 Please note that following code form fenv.c won't generate overflow
 exception on x87:

   if (excepts  FP_EX_OVERFLOW)
 {
   volatile float max = __FLT_MAX__;
   r = max * max;
 }

 r being volatile is intended to ensure that the result does get stored
 back to memory, and so in particular that a result computed with excess
 precision gets converted back to float and the exception is raised.

Can we introduce a target-dependant source here, in the same way as
gfortran has different config/fpu-*.c files? I would like to propose
the code from (recently improved) libgcc/config/i386/sfp-exceptions.c
to generate exceptions on x86 targets. The main benefits of this code
are:
- precision: it generates exactly the exception it is supposed to
generate at the exact spot (x87 and SSE)
- the code doesn't do arithemtics with denormal results
- can also generate non-standard denormal exception

Your proposed code can be considered a generic fallback code then. I'm
sure other targets have their own, perhaps better ways to generate FP
exceptions.

Uros.

Re: Implement C11 _Atomic

[Dominique Dhumieres]

The tests introduced in revision 204544 fail with -m32
(see http://gcc.gnu.org/ml/gcc-regression/2013-11/msg00213.html
or http://gcc.gnu.org/ml/gcc-testresults/2013-11/msg00526.html ).
This revision may also causes the failures of
gfortran.dg/typebound_operator_9.f03 and 
FAIL: gfortran.fortran-torture/execute/forall_1.f90.

On x86_64-apple-darwin* typebound_operator_9.f03 gives the following ICE with 
-O3

/opt/gcc/work/gcc/testsuite/gfortran.dg/typebound_operator_9.f03: In function 
'nabla2_cart2d':
/opt/gcc/work/gcc/testsuite/gfortran.dg/typebound_operator_9.f03:272:0: 
internal compiler error: tree check: expected integer_cst, have plus_expr in 
tree_int_cst_lt, at tree.c:7083
   function nabla2_cart2d (obj)

and the failures of c11-atomic-exec-*.exe are of the kind

Executing on host: /opt/gcc/build_w/gcc/xgcc -B/opt/gcc/build_w/gcc/ 
/opt/gcc/work/gcc/testsuite/gcc.dg/atomic/c11-atomic-exec-4.c   
-B/opt/gcc/build_w/x86_64-apple-darwin13.0.0/i386/libatomic/  
-L/opt/gcc/build_w/x86_64-apple-darwin13.0.0/i386/libatomic/.libs -latomic  
-fno-diagnostics-show-caret -fdiagnostics-color=never   -Os  -std=c11 
-pedantic-errors -pthread -D_POSIX_C_SOURCE=200809L  -lm   -m32 -o 
./c11-atomic-exec-4.exe(timeout = 300)
spawn /opt/gcc/build_w/gcc/xgcc -B/opt/gcc/build_w/gcc/ 
/opt/gcc/work/gcc/testsuite/gcc.dg/atomic/c11-atomic-exec-4.c 
-B/opt/gcc/build_w/x86_64-apple-darwin13.0.0/i386/libatomic/ 
-L/opt/gcc/build_w/x86_64-apple-darwin13.0.0/i386/libatomic/.libs -latomic 
-fno-diagnostics-show-caret -fdiagnostics-color=never -Os -std=c11 
-pedantic-errors -pthread -D_POSIX_C_SOURCE=200809L -lm -m32 -o 
./c11-atomic-exec-4.exe
Undefined symbols for architecture i386:
  ___atomic_compare_exchange_16, referenced from:
  _test_main_long_double_add in ccBbKO9A.o
  _test_thread_long_double_add in ccBbKO9A.o
  _test_main_complex_double_add in ccBbKO9A.o
  _test_thread_complex_double_add in ccBbKO9A.o
  _test_main_long_double_postinc in ccBbKO9A.o
  _test_thread_long_double_postinc in ccBbKO9A.o
  _test_main_long_double_preinc in ccBbKO9A.o
  ...^M
  ___atomic_load_16, referenced from:
  _test_main_long_double_add in ccBbKO9A.o
  _test_thread_long_double_add in ccBbKO9A.o
  _test_main_complex_double_add in ccBbKO9A.o
  _test_thread_complex_double_add in ccBbKO9A.o
  _test_main_long_double_postinc in ccBbKO9A.o
  _test_thread_long_double_postinc in ccBbKO9A.o
  _test_main_long_double_preinc in ccBbKO9A.o
  ...^M
ld: symbol(s) not found for architecture i386

Dominique

Re: Implement C11 _Atomic

[Joseph S. Myers]

On Fri, 8 Nov 2013, Uros Bizjak wrote:

 Can we introduce a target-dependant source here, in the same way as

Sure, that seems a reasonable thing to do.  I think putting a file fenv.c 
in an appropriate subdirectory of libatomic/config will result in it being 
found automatically by the existing search path logic, but you'll need to 
test that.

The present code essentially follows what glibc's feraiseexcept does for 
lots of architectures, but with generic C code where the glibc code tends 
to use asms to control the exact instructions used (and thereby avoid the 
need for volatile, I suppose).

-- 
Joseph S. Myers
jos...@codesourcery.com

Re: Implement C11 _Atomic

[Joseph S. Myers]

On Fri, 8 Nov 2013, Dominique Dhumieres wrote:

 This revision may also causes the failures of
 gfortran.dg/typebound_operator_9.f03 and 
 FAIL: gfortran.fortran-torture/execute/forall_1.f90.

I doubt the patch affects Fortran (there are language-independent changes, 
but they are fairly small and shouldn't affect code not using _Atomic 
qualifiers).

 Undefined symbols for architecture i386:
   ___atomic_compare_exchange_16, referenced from:

   ___atomic_load_16, referenced from:

Either those functions should be in libatomic (using locking where 
necessary), or the built-in functions should be expanding to use the 
size-generic libatomic function rather than the _16 one.  Richard or 
Andrew should be able to advise on which the design was for systems that 
don't have lock-free atomics for all of the standard sizes (1, 2, 4, 8, 
16).

My guess is that it would be hard to provide the _16 functions in 
libatomic when TImode types aren't supported, and so the bug is that _16 
function calls are generated when not supported.  That is, c-common.c 
functions resolve_overloaded_atomic_exchange, 
resolve_overloaded_atomic_compare_exchange, 
resolve_overloaded_atomic_load, resolve_overloaded_atomic_store, where 
they check n != 16, should do something like (n != 16 || 
!targetm.scalar_mode_supported_p (TImode)).  (Better, refactor the (n != 1 
 n != 2  n != 4  n != 8  n != 16) into an atomic_size_supported_p 
or similar function.)

-- 
Joseph S. Myers
jos...@codesourcery.com

Re: Implement C11 _Atomic

[Dominique Dhumieres]

 I doubt the patch affects Fortran (there are language-independent changes,
 but they are fairly small and shouldn't affect code not using _Atomic 
 qualifiers).

The Fortran failures seem related to revision 204538.

Dominique

Re: Implement C11 _Atomic

[Uros Bizjak]

On Wed, Nov 6, 2013 at 12:21 AM, Joseph S. Myers
jos...@codesourcery.com wrote:
 This patch, relative to trunk and based on work done on the C11-atomic
 branch, adds support for C11 _Atomic.  It is intended to include all
 the required language support.

 It does not include the stdatomic.h header; there's a version on the
 branch, but it needs further review against the standard and test
 coverage adding to the testsuite before I can propose it for mainline.

 Support for atomic types having bigger alignment than the
 corresponding non-atomic types is limited: it includes the code to
 increase the alignment of types whose size is exactly 1, 2, 4, 8 or 16
 to that of the corresponding integer type [*], but not anything for
 target-specific alignment increases.  There's code for target-specific
 alignment on the branch (and I intend to merge trunk back to the
 branch once this patch is on trunk, so it's easy to tell what the
 changes still left on the branch are), should any target maintainers
 wish to have such alignment.  Note however that ideally libstdc++
 atomics would be ABI-compatible with C atomics, requiring them to get
 the same alignment; the branch has an atomic attribute for that
 purpose, but I think further work on the C++ front-end changes would
 be needed for them to be ready for mainline.

 [*] The c-common.c code to resolve size-generic atomic built-in
 functions to size-specific ones assumes that types are naturally
 aligned (and so resolves to calls to the size-specific functions that
 require natural alignment) without checking it.  If users should be
 able to use the size-generic functions on types with lesser alignment,
 e.g. _Complex double (8-byte rather than 16-byte aligned), rather than
 just on their _Atomic variants, this is of course a bug in the code
 resolving those built-in functions.  (The libatomic functions properly
 check for alignment at runtime.)  But it seems reasonable enough
 anyway to make _Atomic _Complex double 16-byte aligned.

 Full use of _Atomic requires linking with libatomic, if you're doing
 any atomic operations that aren't fully expanded inline; arguably
 libatomic should be linked in by default with --as-needed (given that
 previously ordinary C code didn't require the user to link any
 libraries explicitly unless they directly called library functions),
 but that's independent of this patch.  Correct handling of atomic
 compound assignment for floating-point types also requires built-in
 support for floating-point environment manipulation operations roughly
 equivalent to feholdexcept, feclearexcept and feupdateenv (built-ins
 are used to avoid introducing libm dependencies, which generic C code
 not actually calling libm library functions shouldn't require); this
 patch includes such support for x86 [*], and I expect other
 architectures to be simpler.  If you don't have a libatomic port, the
 execution tests for _Atomic simply won't be run; if you have libatomic
 but not the floating-point support, the tests will be run but
 c11-atomic-exec-5.c will fail (assuming the library features are
 present for that test to run at all - it also requires pthreads).

 [*] This could be optimized if desired by passing the types in
 question to the target hook so it can generate code for only one of
 x87 and SSE in most cases, much like an optimization present in glibc
 when it internally does feholdexcept / fesetenv / feupdateenv
 sequences.

 _Atomic support is currently disabled for Objective-C and OpenMP.  For
 both (but mainly OpenMP), the relevant parser code needs checking to
 determine where convert_lvalue_to_rvalue calls need inserting to
 ensure that accesses to atomic variables involve atomic loads.  For
 Objective-C, there are also various special cases of compound
 assignment that need special handling for atomics just as standard C
 compound assignment is handled differently for atomics, as well as
 some TYPE_MAIN_VARIANT calls to check for correctness for atomics; see
 the comment on the relevant sorry () call for details.  OpenMP should
 also have TYPE_MAIN_VARIANT uses checked as well as a use of
 TYPE_QUALS_NO_ADDR_SPACE for a diagnostic in
 c_parser_omp_declare_reduction (where the diagnostic refers to a
 particular list of qualifiers).

 Bootstrapped with no regressions on x86_64-unknown-linux-gnu.  OK to
 commit (the various non-front-end pieces)?

 gcc:
 2013-11-05  Andrew MacLeod  amacl...@redhat.com
 Joseph Myers  jos...@codesourcery.com

 * tree-core.h (enum cv_qualifier): Add TYPE_QUAL_ATOMIC.
 (enum tree_index): Add TI_ATOMICQI_TYPE, TI_ATOMICHI_TYPE,
 TI_ATOMICSI_TYPE, TI_ATOMICDI_TYPE and TI_ATOMICTI_TYPE.
 (struct tree_base): Add atomic_flag field.
 * tree.h (TYPE_ATOMIC): New accessor macro.
 (TYPE_QUALS, TYPE_QUALS_NO_ADDR_SPACE): Add TYPE_QUAL_ATOMIC.
 (TYPE_QUALS_NO_ADDR_SPACE_NO_ATOMIC): New macro.
 (atomicQI_type_node, atomicHI_type_node, 

Re: Implement C11 _Atomic

[Jakub Jelinek]

On Tue, Nov 05, 2013 at 11:21:56PM +, Joseph S. Myers wrote:
 2013-11-05  Andrew MacLeod  amacl...@redhat.com
   Joseph Myers  jos...@codesourcery.com
 
   * tree-core.h (enum cv_qualifier): Add TYPE_QUAL_ATOMIC.
   (enum tree_index): Add TI_ATOMICQI_TYPE, TI_ATOMICHI_TYPE,
   TI_ATOMICSI_TYPE, TI_ATOMICDI_TYPE and TI_ATOMICTI_TYPE.
   (struct tree_base): Add atomic_flag field.
   * tree.h (TYPE_ATOMIC): New accessor macro.
   (TYPE_QUALS, TYPE_QUALS_NO_ADDR_SPACE): Add TYPE_QUAL_ATOMIC.
   (TYPE_QUALS_NO_ADDR_SPACE_NO_ATOMIC): New macro.
   (atomicQI_type_node, atomicHI_type_node, atomicSI_type_node)
   (atomicDI_type_node, atomicTI_type_node): New macros for type
   nodes.
   * tree.c (set_type_quals): Set TYPE_ATOMIC.
   (find_atomic_core_type): New function.
   (build_qualified_type): Adjust alignment for qualified types.
   (build_atomic_base): New function
   (build_common_tree_nodes): Build atomicQI_type_node,
   atomicHI_type_node, atomicSI_type_node, atomicDI_type_node and
   atomicTI_type_node.
   * print-tree.c (print_node): Print atomic qualifier.
   * tree-pretty-print.c (dump_generic_node): Print atomic type
   attribute.
   * target.def (atomic_assign_expand_fenv): New hook.
   * doc/tm.texi.in (TARGET_ATOMIC_ASSIGN_EXPAND_FENV): New @hook.
   * doc/tm.texi: Regenerate.
   * targhooks.c (default_atomic_assign_expand_fenv): New function.
   * targhooks.h (default_atomic_assign_expand_fenv): Declare.
   * sync-builtins.def (__atomic_feraiseexcept): New built-in
   function.

 2013-11-05  Joseph Myers  jos...@codesourcery.com
 
   * lib/target-supports.exp
   (check_effective_target_fenv_exceptions): New function.
   * lib/atomic-dg.exp, gcc.dg/atomic/atomic.exp: New files.
   * gcc.dg/atomic/c11-atomic-exec-1.c,
   gcc.dg/atomic/c11-atomic-exec-2.c,
   gcc.dg/atomic/c11-atomic-exec-3.c,
   gcc.dg/atomic/c11-atomic-exec-4.c,
   gcc.dg/atomic/c11-atomic-exec-5.c, gcc.dg/c11-atomic-1.c,
   gcc.dg/c11-atomic-2.c, gcc.dg/c11-atomic-3.c,
   gcc.dg/c90-atomic-1.c, gcc.dg/c99-atomic-1.c: New tests.
 
 libatomic:
 2013-11-05  Joseph Myers  jos...@codesourcery.com
 
   * fenv.c: New file.
   * libatomic.map (LIBATOMIC_1.1): New symbol version.  Include
   __atomic_feraiseexcept.
   * configure.ac (libtool_VERSION): Change to 2:0:1.
   (fenv.h): Test for header.
   * Makefile.am (libatomic_la_SOURCES): Add fenv.c.
   * Makefile.in, auto-config.h.in, configure: Regenerate.

The middle-end, libatomic and testsuite changes are ok.

Jakub

Re: Implement C11 _Atomic

[Uros Bizjak]

On Thu, Nov 7, 2013 at 5:45 PM, Jakub Jelinek ja...@redhat.com wrote:
 On Tue, Nov 05, 2013 at 11:21:56PM +, Joseph S. Myers wrote:
 2013-11-05  Andrew MacLeod  amacl...@redhat.com
   Joseph Myers  jos...@codesourcery.com

   * tree-core.h (enum cv_qualifier): Add TYPE_QUAL_ATOMIC.
   (enum tree_index): Add TI_ATOMICQI_TYPE, TI_ATOMICHI_TYPE,
   TI_ATOMICSI_TYPE, TI_ATOMICDI_TYPE and TI_ATOMICTI_TYPE.
   (struct tree_base): Add atomic_flag field.
   * tree.h (TYPE_ATOMIC): New accessor macro.
   (TYPE_QUALS, TYPE_QUALS_NO_ADDR_SPACE): Add TYPE_QUAL_ATOMIC.
   (TYPE_QUALS_NO_ADDR_SPACE_NO_ATOMIC): New macro.
   (atomicQI_type_node, atomicHI_type_node, atomicSI_type_node)
   (atomicDI_type_node, atomicTI_type_node): New macros for type
   nodes.
   * tree.c (set_type_quals): Set TYPE_ATOMIC.
   (find_atomic_core_type): New function.
   (build_qualified_type): Adjust alignment for qualified types.
   (build_atomic_base): New function
   (build_common_tree_nodes): Build atomicQI_type_node,
   atomicHI_type_node, atomicSI_type_node, atomicDI_type_node and
   atomicTI_type_node.
   * print-tree.c (print_node): Print atomic qualifier.
   * tree-pretty-print.c (dump_generic_node): Print atomic type
   attribute.
   * target.def (atomic_assign_expand_fenv): New hook.
   * doc/tm.texi.in (TARGET_ATOMIC_ASSIGN_EXPAND_FENV): New @hook.
   * doc/tm.texi: Regenerate.
   * targhooks.c (default_atomic_assign_expand_fenv): New function.
   * targhooks.h (default_atomic_assign_expand_fenv): Declare.
   * sync-builtins.def (__atomic_feraiseexcept): New built-in
   function.

 2013-11-05  Joseph Myers  jos...@codesourcery.com

   * lib/target-supports.exp
   (check_effective_target_fenv_exceptions): New function.
   * lib/atomic-dg.exp, gcc.dg/atomic/atomic.exp: New files.
   * gcc.dg/atomic/c11-atomic-exec-1.c,
   gcc.dg/atomic/c11-atomic-exec-2.c,
   gcc.dg/atomic/c11-atomic-exec-3.c,
   gcc.dg/atomic/c11-atomic-exec-4.c,
   gcc.dg/atomic/c11-atomic-exec-5.c, gcc.dg/c11-atomic-1.c,
   gcc.dg/c11-atomic-2.c, gcc.dg/c11-atomic-3.c,
   gcc.dg/c90-atomic-1.c, gcc.dg/c99-atomic-1.c: New tests.

 libatomic:
 2013-11-05  Joseph Myers  jos...@codesourcery.com

   * fenv.c: New file.
   * libatomic.map (LIBATOMIC_1.1): New symbol version.  Include
   __atomic_feraiseexcept.
   * configure.ac (libtool_VERSION): Change to 2:0:1.
   (fenv.h): Test for header.
   * Makefile.am (libatomic_la_SOURCES): Add fenv.c.
   * Makefile.in, auto-config.h.in, configure: Regenerate.

 The middle-end, libatomic and testsuite changes are ok.

Please note that following code form fenv.c won't generate overflow
exception on x87:

  if (excepts  FP_EX_OVERFLOW)
{
  volatile float max = __FLT_MAX__;
  r = max * max;
}

Uros.

Re: Implement C11 _Atomic

[Joseph S. Myers]

On Thu, 7 Nov 2013, Uros Bizjak wrote:

 Please note that following code form fenv.c won't generate overflow
 exception on x87:
 
   if (excepts  FP_EX_OVERFLOW)
 {
   volatile float max = __FLT_MAX__;
   r = max * max;
 }

r being volatile is intended to ensure that the result does get stored 
back to memory, and so in particular that a result computed with excess 
precision gets converted back to float and the exception is raised.

-- 
Joseph S. Myers
jos...@codesourcery.com

Re: Implement C11 _Atomic

[Uros Bizjak]

On Thu, Nov 7, 2013 at 7:26 PM, Joseph S. Myers jos...@codesourcery.com wrote:
 On Thu, 7 Nov 2013, Uros Bizjak wrote:

 Please note that following code form fenv.c won't generate overflow
 exception on x87:

   if (excepts  FP_EX_OVERFLOW)
 {
   volatile float max = __FLT_MAX__;
   r = max * max;
 }

 r being volatile is intended to ensure that the result does get stored
 back to memory, and so in particular that a result computed with excess
 precision gets converted back to float and the exception is raised.

--cut here--
#define _GNU_SOURCE
#include fenv.h

int main(void) {
feenableexcept(FE_INVALID   |
   FE_DIVBYZERO |
   FE_OVERFLOW  |
   FE_UNDERFLOW);

volatile float a = __FLT_MAX__, b = __FLT_MAX__;
volatile float c = a*b;

return 0;
}
--cut here--

[uros@localhost test]$ gcc -lm -g fpex.c
[uros@localhost test]$ ./a.out
Floating point exception (core dumped)
[uros@localhost test]$ gcc -lm -g -m32 fpex.c
[uros@localhost test]$ ./a.out
[uros@localhost test]$

Uros.

Re: Implement C11 _Atomic

[Joseph S. Myers]

On Thu, 7 Nov 2013, Uros Bizjak wrote:

 [uros@localhost test]$ gcc -lm -g fpex.c
 [uros@localhost test]$ ./a.out
 Floating point exception (core dumped)
 [uros@localhost test]$ gcc -lm -g -m32 fpex.c
 [uros@localhost test]$ ./a.out
 [uros@localhost test]$

I see code of the form (testing compilation rather than execution):

flds4(%esp)
flds8(%esp)
fmulp   %st, %st(1)
fstps   12(%esp)

where the fstps should result in the exception, and glibc uses volatile in 
several places, conditional on __FLT_EVAL_METHOD__ != 0, to force a 
conversion to the semantic type (whether for correct results, or to ensure 
exceptions).

-- 
Joseph S. Myers
jos...@codesourcery.com

Re: Implement C11 _Atomic

[Joseph S. Myers]

On Thu, 7 Nov 2013, Uros Bizjak wrote:

  I see code of the form (testing compilation rather than execution):
 
  flds4(%esp)
  flds8(%esp)
  fmulp   %st, %st(1)
  fstps   12(%esp)
 
  where the fstps should result in the exception, and glibc uses volatile in
  several places, conditional on __FLT_EVAL_METHOD__ != 0, to force a
  conversion to the semantic type (whether for correct results, or to ensure
  exceptions).
 
 Yes, this is the exact sequence my example compiles to:
 
  8048405:   d9 44 24 14 flds   0x14(%esp)
  8048409:   d9 44 24 18 flds   0x18(%esp)
  804840d:   de c9   fmulp  %st,%st(1)
  804840f:   d9 5c 24 1c fstps  0x1c(%esp)
 
 unfortunately, it won't generate exception.

Are you sure?  It's documented as generating an exception.  That may mean, 
as usual on x87, setting the exception bit (as can be tested by 
fetestexcept) and only calling a trap handler on the *next* x87 
instruction.  So if fstps is the last floating-point instruction executed 
by the program, a trap handler may not be called - but that's no different 
from an ordinary floating-point compound assignment having the 
exception-raising operation as the last floating-point instruction.

-- 
Joseph S. Myers
jos...@codesourcery.com

Re: Implement C11 _Atomic

[Uros Bizjak]

On Thu, Nov 7, 2013 at 9:33 PM, Joseph S. Myers jos...@codesourcery.com wrote:


  I see code of the form (testing compilation rather than execution):
 
  flds4(%esp)
  flds8(%esp)
  fmulp   %st, %st(1)
  fstps   12(%esp)
 
  where the fstps should result in the exception, and glibc uses volatile in
  several places, conditional on __FLT_EVAL_METHOD__ != 0, to force a
  conversion to the semantic type (whether for correct results, or to ensure
  exceptions).

 Yes, this is the exact sequence my example compiles to:

  8048405:   d9 44 24 14 flds   0x14(%esp)
  8048409:   d9 44 24 18 flds   0x18(%esp)
  804840d:   de c9   fmulp  %st,%st(1)
  804840f:   d9 5c 24 1c fstps  0x1c(%esp)

 unfortunately, it won't generate exception.

 Are you sure?  It's documented as generating an exception.  That may mean,
 as usual on x87, setting the exception bit (as can be tested by
 fetestexcept) and only calling a trap handler on the *next* x87
 instruction.  So if fstps is the last floating-point instruction executed
 by the program, a trap handler may not be called - but that's no different
 from an ordinary floating-point compound assignment having the
 exception-raising operation as the last floating-point instruction.

Aha, here is the problem.

The exception is not generated by fmulp, but by the fstps that follows
fmulp. The fstps will close exception window from fmulp, but fstps
needs another fwait to generate exception. I have added fwait after
fstps manually:

   0x080483fd +29:fstps  0x18(%esp)
   0x08048401 +33:flds   0x18(%esp)
   0x08048405 +37:flds   0x18(%esp)
   0x08048409 +41:fmulp  %st,%st(1)
   0x0804840b +43:fstps  0x1c(%esp)
= 0x0804840f +47:fwait
   0x08048410 +48:leave

And in this case, exception was generated, as marked by = in gdb.

Uros.

Re: Implement C11 _Atomic

[Uros Bizjak]

On Thu, Nov 7, 2013 at 9:55 PM, Uros Bizjak ubiz...@gmail.com wrote:

  I see code of the form (testing compilation rather than execution):
 
  flds4(%esp)
  flds8(%esp)
  fmulp   %st, %st(1)
  fstps   12(%esp)
 
  where the fstps should result in the exception, and glibc uses volatile in
  several places, conditional on __FLT_EVAL_METHOD__ != 0, to force a
  conversion to the semantic type (whether for correct results, or to ensure
  exceptions).

 Yes, this is the exact sequence my example compiles to:

  8048405:   d9 44 24 14 flds   0x14(%esp)
  8048409:   d9 44 24 18 flds   0x18(%esp)
  804840d:   de c9   fmulp  %st,%st(1)
  804840f:   d9 5c 24 1c fstps  0x1c(%esp)

 unfortunately, it won't generate exception.

 Are you sure?  It's documented as generating an exception.  That may mean,
 as usual on x87, setting the exception bit (as can be tested by
 fetestexcept) and only calling a trap handler on the *next* x87
 instruction.  So if fstps is the last floating-point instruction executed
 by the program, a trap handler may not be called - but that's no different
 from an ordinary floating-point compound assignment having the
 exception-raising operation as the last floating-point instruction.

 Aha, here is the problem.

 The exception is not generated by fmulp, but by the fstps that follows
 fmulp. The fstps will close exception window from fmulp, but fstps
 needs another fwait to generate exception. I have added fwait after
 fstps manually:

0x080483fd +29:fstps  0x18(%esp)
0x08048401 +33:flds   0x18(%esp)
0x08048405 +37:flds   0x18(%esp)
0x08048409 +41:fmulp  %st,%st(1)
0x0804840b +43:fstps  0x1c(%esp)
 = 0x0804840f +47:fwait
0x08048410 +48:leave

 And in this case, exception was generated, as marked by = in gdb.

However, this insn also raised FE_INEXACT flag (also on x86_64),
probably not what you wanted. Your code that generates FE_UNDERFLOW
will also raise FE_INEXACT. (and FE_DENORMAL).

Uros.

Re: Implement C11 _Atomic

[Joseph S. Myers]

On Thu, 7 Nov 2013, Uros Bizjak wrote:

 However, this insn also raised FE_INEXACT flag (also on x86_64),
 probably not what you wanted. Your code that generates FE_UNDERFLOW
 will also raise FE_INEXACT. (and FE_DENORMAL).

If the compound assignment raised overflow or underflow, it will also have 
raised inexact, so it's not a problem that this code raises inexact along 
with overflow/underflow (and then raises it again - ISO C expressly does 
not require support for trap handlers that maintain information about the 
order or count of floating-point exceptions).  I raised the case of exact 
underflow in http://www.open-std.org/jtc1/sc22/wg14/13103, but it's a 
matter to be dealt with when standard C gets bindings for non-default 
exception handling (probably in TS 18661-5, not yet written) rather than 
now.

If GCC were to implement such bindings, the issues with delayed x87 
exceptions might also need addressing for them - but given the lack of 
support for exceptions and rounding modes following C99/C11 Annex F, I 
don't really expect support for new bindings for exceptions and rounding 
modes any time soon.

(Separately, C99/C11 also leave it implementation-defined whether 
feraiseexcept raises inexact along with overflow and underflow - but here 
we're dealing with exceptions we know came from floating-point arithmetic, 
rather than an arbitrary user-specified argument to feraiseexcept.)

-- 
Joseph S. Myers
jos...@codesourcery.com

Re: Implement C11 _Atomic

[Andrew MacLeod]

On 11/05/2013 06:21 PM, Joseph S. Myers wrote:

This patch, relative to trunk and based on work done on the C11-atomic
branch, adds support for C11 _Atomic.  It is intended to include all
the required language support.


Thanks for picking this up Joseph... It would have taken me months to do 
the same thing, and been a lot more painful for everyone :-).


It is really excellent work.

Andrew