[atomic] Probable documentation issue: how do we know atomic is safe and/or efficient?

newer
[github] boostorg/boost not listed?

John Maddock

24 Dec 2013 24 Dec '13

1:43 p.m.

I'm looking to use Boost.Atomic for the double-checked-locking-idiom, but I'm having trouble discovering a) Whether the library is correctly implemented. b) How efficient (or otherwise) the library is on various platforms. I'm sure the answer to (a) is that it is correct, but I was surprised that atomic<int> relied on the volatile modifier for thread safe load/stores on MSVC, I had assumed that something more than that would have been required, but I know I'm out of date on this stuff! I can't help thinking that questions like this could be allayed by better documentation - specifically what operations are used to ensure correctness on each supported platform, plus rationale and/or link to reference material. My apologies if this has been covered already in the review, and I realize that documentation is a lot of work, but I can see no way to ensure correctness other than through code inspection, so proper documentation is essential in that case. Regards, John.

Show replies by date

Andrey Semashev

24 Dec 24 Dec

2:07 p.m.

New subject: [atomic] Probable documentation issue: how do we know atomic is safe and/or efficient?

On Tue, Dec 24, 2013 at 5:43 PM, John Maddock wrote:

...

I'm looking to use Boost.Atomic for the double-checked-locking-idiom, but I'm having trouble discovering

a) Whether the library is correctly implemented.

I hope so. :) The tests are there, but they were mostly run on x86 in the test farm.

...

b) How efficient (or otherwise) the library is on various platforms.

In general, if the library reports the particular type as lock-free, you should expect performance comparable to std::atomic. In some cases (notably, older gcc) it's even better.

...

I'm sure the answer to (a) is that it is correct, but I was surprised that atomic<int> relied on the volatile modifier for thread safe load/stores on MSVC, I had assumed that something more than that would have been required, but I know I'm out of date on this stuff!

This is enough in case of x86, which has been the primary, if not the only target for MSVC for a long time. I'm not so experienced with ARM, so Boost.Atomic may need some adjustments for MSVC for this target architecture.

...

I can't help thinking that questions like this could be allayed by better documentation - specifically what operations are used to ensure correctness on each supported platform, plus rationale and/or link to reference material.

Well, if you have knowledge of a particular architecture, you probably know what atomic ops it supports and both Boost.Atomic and std::atomic are most likely based on them. The particular instructions and their code representation (i.e. compiler intrinsics, inline assembly, kernel calls, etc.), I think, is more an implementation detail. It could be generally described for a curious reader though.

...

My apologies if this has been covered already in the review, and I realize that documentation is a lot of work, but I can see no way to ensure correctness other than through code inspection, so proper documentation is essential in that case.

Documentation is hardly a measure of code correctness, but I admit, in case of Boost.Atomic it could be improved.

tim

3:08 p.m.

New subject: [atomic] Probable documentation issue: how do we know atomic is safe and/or efficient?

...

...
a) Whether the library is correctly implemented.

I hope so. :) The tests are there, but they were mostly run on x86 in the test farm.

atomic<>s are hard to test, so it is best to verify the correctness by analyzing the code.

...

...
b) How efficient (or otherwise) the library is on various platforms.

In general, if the library reports the particular type as lock-free, you should expect performance comparable to std::atomic. In some cases (notably, older gcc) it's even better.

for arm/ppc platforms i suppose that std::atomics will perform better, because (a) some operations are emulated with cas (no idea if this has a real-world impact) and (b) boost.atomic does not support all hardware instructions (iirc armv7 provides atomic double-width instructions which boost.simd does not make use of).

...

...
I'm sure the answer to (a) is that it is correct, but I was surprised that atomic<int> relied on the volatile modifier for thread safe load/stores on MSVC, I had assumed that something more than that would have been required, but I know I'm out of date on this stuff!

This is enough in case of x86, which has been the primary, if not the only target for MSVC for a long time. I'm not so experienced with ARM, so Boost.Atomic may need some adjustments for MSVC for this target architecture.

compare http://en.cppreference.com/w/cpp/atomic/memory_order, section "Relationship with volatile"

...

...
My apologies if this has been covered already in the review, and I realize that documentation is a lot of work, but I can see no way to ensure correctness other than through code inspection, so proper documentation is essential in that case.

fyi: the original author did the review and disappeared. i (as in review manager) was actually surprised that he committed the stuff to trunk ... it only ended up in the release, because andrey and me adopted the library, though personally i have limited time to work on stuff other than bug fixes (and no interest in improving the documentation) ... my personal recommendation: * if your compiler supports std::atomic, use it instead * if you need a good API documentation, check the std::atomic documentation * if you want to verify the correctness, use the source ;) cheers, tim

Peter Dimov

2:29 p.m.

New subject: [atomic] Probable documentation issue: how do we knowatomic is safe and/or efficient?

John Maddock wrote:

...

I'm sure the answer to (a) is that it is correct, but I was surprised that atomic<int> relied on the volatile modifier for thread safe load/stores on MSVC, I had assumed that something more than that would have been required, but I know I'm out of date on this stuff!

"volatile" was changed in VC++ 8.0 to guarantee acquire/release semantics, then changed back in 2012 to only do so when /volatile:ms is active, which it is by default, except on ARM. http://msdn.microsoft.com/en-us/library/12a04hfd.aspx

John Maddock

5:17 p.m.

New subject: [atomic] Probable documentation issue: how do weknowatomic is safe and/or efficient?

...

...
I'm sure the answer to (a) is that it is correct, but I was surprised that atomic<int> relied on the volatile modifier for thread safe load/stores on MSVC, I had assumed that something more than that would have been required, but I know I'm out of date on this stuff!

"volatile" was changed in VC++ 8.0 to guarantee acquire/release semantics, then changed back in 2012 to only do so when /volatile:ms is active, which it is by default, except on ARM.

http://msdn.microsoft.com/en-us/library/12a04hfd.aspx

So does this mean that Boost.Atomic is perhaps not safe after all? John.

Andrey Semashev

7:24 p.m.

New subject: [atomic] Probable documentation issue: how do weknowatomic is safe and/or efficient?

On Tuesday 24 December 2013 17:17:37 John Maddock wrote:

...

...
...
I'm sure the answer to (a) is that it is correct, but I was surprised that atomic<int> relied on the volatile modifier for thread safe load/stores on MSVC, I had assumed that something more than that would have been required, but I know I'm out of date on this stuff!

"volatile" was changed in VC++ 8.0 to guarantee acquire/release semantics, then changed back in 2012 to only do so when /volatile:ms is active, which it is by default, except on ARM.

http://msdn.microsoft.com/en-us/library/12a04hfd.aspx

So does this mean that Boost.Atomic is perhaps not safe after all?

No, it's perfectly safe on x86. It is also safe on ARM if /volatile:ms is specified.

Rob Stewart

25 Dec 25 Dec

12:40 p.m.

New subject: [atomic] Probable documentation issue: how do weknowatomic is safe and/or efficient?

On Dec 24, 2013, at 2:24 PM, Andrey Semashev wrote:

...

On Tuesday 24 December 2013 17:17:37 John Maddock wrote:

...
...
...
I'm sure the answer to (a) is that it is correct, but I was surprised that atomic<int> relied on the volatile modifier for thread safe load/stores on MSVC, I had assumed that something more than that would have been required, but I know I'm out of date on this stuff!

"volatile" was changed in VC++ 8.0 to guarantee acquire/release semantics, then changed back in 2012 to only do so when /volatile:ms is active, which it is by default, except on ARM.

http://msdn.microsoft.com/en-us/library/12a04hfd.aspx

So does this mean that Boost.Atomic is perhaps not safe after all?

No, it's perfectly safe on x86. It is also safe on ARM if /volatile:ms is specified.

How do you ensure the use of /volatile:ms? Can users build Boost with /volatile:iso? ___ Rob (Sent from my portable computation engine)

Andrey Semashev

12:47 p.m.

New subject: [atomic] Probable documentation issue: how do weknowatomic is safe and/or efficient?

On Wed, Dec 25, 2013 at 4:40 PM, Rob Stewart wrote:

...

On Dec 24, 2013, at 2:24 PM, Andrey Semashev wrote:

...
On Tuesday 24 December 2013 17:17:37 John Maddock wrote:

...
...
...
I'm sure the answer to (a) is that it is correct, but I was surprised that atomic<int> relied on the volatile modifier for thread safe load/stores on MSVC, I had assumed that something more than that would have been required, but I know I'm out of date on this stuff!

"volatile" was changed in VC++ 8.0 to guarantee acquire/release semantics, then changed back in 2012 to only do so when /volatile:ms is active, which it is by default, except on ARM.

http://msdn.microsoft.com/en-us/library/12a04hfd.aspx

So does this mean that Boost.Atomic is perhaps not safe after all?

No, it's perfectly safe on x86. It is also safe on ARM if /volatile:ms is specified.

How do you ensure the use of /volatile:ms? Can users build Boost with /volatile:iso?

You can't, I was just answering John's question. To be clear, yes, I agree that this needs fixing. It's just no one got around to do it. A ticket would be welcome. Especially, with a patch. :)

Rob Stewart

1:12 p.m.

New subject: [atomic] Probable documentation issue: how do weknowatomic is safe and/or efficient?

On Dec 25, 2013, at 7:47 AM, Andrey Semashev wrote:

...

On Wed, Dec 25, 2013 at 4:40 PM, Rob Stewart wrote:

...
On Dec 24, 2013, at 2:24 PM, Andrey Semashev wrote:

...
...
So does this mean that Boost.Atomic is perhaps not safe after all?

No, it's perfectly safe on x86. It is also safe on ARM if /volatile:ms is specified.

How do you ensure the use of /volatile:ms? Can users build Boost with /volatile:iso?

You can't, I was just answering John's question.

You used the phrase, "perfectly safe," which suggested that /volatile:ms was somehow ensured on x86.

...

To be clear, yes, I agree that this needs fixing.

I wouldn't have described it as you did, but things are clear now. ___ Rob (Sent from my portable computation engine)

Andrey Semashev

1:16 p.m.

New subject: [atomic] Probable documentation issue: how do weknowatomic is safe and/or efficient?

On Wed, Dec 25, 2013 at 5:12 PM, Rob Stewart wrote:

...

On Dec 25, 2013, at 7:47 AM, Andrey Semashev wrote:

...
On Wed, Dec 25, 2013 at 4:40 PM, Rob Stewart wrote:

...
On Dec 24, 2013, at 2:24 PM, Andrey Semashev wrote:

...
...
So does this mean that Boost.Atomic is perhaps not safe after all?

No, it's perfectly safe on x86. It is also safe on ARM if /volatile:ms is specified.

How do you ensure the use of /volatile:ms? Can users build Boost with /volatile:iso?

You can't, I was just answering John's question.

You used the phrase, "perfectly safe," which suggested that /volatile:ms was somehow ensured on x86.

x86 guarantees correct memory view, except for very specific operations. And compiler barriers in the code guarantee that the actual instructions are generated in the right order. So yes, regardless of the /volatile:ms switch, on x86 it works as intended.

Andrey Semashev

26 Jan 26 Jan

5:29 p.m.

New subject: [atomic] Probable documentation issue: how do weknowatomic is safe and/or efficient?

On Wed, Dec 25, 2013 at 4:47 PM, Andrey Semashev wrote:

...

On Wed, Dec 25, 2013 at 4:40 PM, Rob Stewart wrote:

...
On Dec 24, 2013, at 2:24 PM, Andrey Semashev wrote:

...
On Tuesday 24 December 2013 17:17:37 John Maddock wrote:

...
...
...
I'm sure the answer to (a) is that it is correct, but I was surprised that atomic<int> relied on the volatile modifier for thread safe load/stores on MSVC, I had assumed that something more than that would have been required, but I know I'm out of date on this stuff!

"volatile" was changed in VC++ 8.0 to guarantee acquire/release semantics, then changed back in 2012 to only do so when /volatile:ms is active, which it is by default, except on ARM.

http://msdn.microsoft.com/en-us/library/12a04hfd.aspx

So does this mean that Boost.Atomic is perhaps not safe after all?

No, it's perfectly safe on x86. It is also safe on ARM if /volatile:ms is specified.

How do you ensure the use of /volatile:ms? Can users build Boost with /volatile:iso?

You can't, I was just answering John's question.

To be clear, yes, I agree that this needs fixing. It's just no one got around to do it. A ticket would be welcome. Especially, with a patch. :)

I have updated Boost.Atomic (develop branch) so that it should work on Windows ARM, but I don't have hardware to test it. To my knowledge, none of our testers have the hardware as well. So I would appreciate if someone tried it out and reported the results (both positive and negative). Thanks.

3958

Age (days ago)

3991

Last active (days ago)

List overview

Download

10 comments

5 participants

participants (5)

Andrey Semashev
John Maddock
Peter Dimov
Rob Stewart
tim