Subject: [boost] [atomic] Initial comments
From: Grund, Holger (Holger.Grund_at_[hidden])
Date: 2011-10-23 10:27:03
It's been awfully quiet on the list regarding the atomic review. The atomic operations library is an invaluable tool for higher level concurrent data structures and algorithms. While toolchain vendors have already or starting to support <atomic>, many people are stuck on compiler where there is currently no support at all. Building these libraries yourself is tedious and prone to error and errors that are hard to spot.
I am in this situation and therefore very much appreciate Helge's effort and think the lack of review comments so far should not be taken as a lack of interest.
Now, I haven't had the time to look on things in any detail yet. I ignored the docs (~~) and only locked at the atomic header and some of x86 implementation so far. I haven't tried to build anything, but I intend to do a proper review though it's unlikely I'll manage to get to it on the official time line.
I would have put some of the comments of the review tool site, but I can't login right now:
- The fallback atomic_thread|signal_fence in boost/atomic.hpp seem to have external linkage, but no inline. Doesn't that give a ODR violation when included from more than on TU?
- The copy-assignment operators seem to always return atomic&. IIRC, the earlier working drafts said return value_type. Has this changed?
- The primary base_atomic is not aligned at all, AFAICT. I take it because you explicit need to extract bits anyhow and that means a memcpy. Does this have a performance impact?
- Inline asm is the preferred way to build instructions. Did you consider intrinsics where available (e.g. ICC and GCC4+)?
- atomic_flag is not statically initialized: I guess, there is some tradeoff with uniform-initialization syntax and member functions on most (all?) current production compilers. However, it seems to me that static initialization guarantees are atomic_flag's primary if not sole raison d'etre. Did you consider to make it a statically initializable type?
- What's the point of the fences around cmpxchg in x86?
- I'm not a GNU inline assembly expert and it's probably fairly unlikely to really affect codegen, but shouldn't clobbered flags be mentioned? Also the EBX handling for 32-bit Linux code may deserve some additional comment. I take it you can't mention ebx in the clobber list because the register allocator would complain for -fPIC, right? Does the impact unwinding?
- I seem some of the *LOCK_FREE macros are defined involving sizeof. I've always assumed these macros (or the corresponding standard ones) would be suitable for use in #if processing. Wouldn't that be one of the main use cases?
Holger Grund, Vice President
Morgan Stanley | Cross-Technology Services
25 Cabot Square | Canary Wharf | Floor 03
London, E14 4QA
Phone: +44 20 7677-4095
NOTICE: Morgan Stanley is not acting as a municipal advisor and the opinions or views contained herein are not intended to be, and do not constitute, advice within the meaning of Section 975 of the Dodd-Frank Wall Street Reform and Consumer Protection Act. If you have received this communication in error, please destroy all electronic and paper copies and notify the sender immediately. Mistransmission is not intended to waive confidentiality or privilege. Morgan Stanley reserves the right, to the extent permitted under applicable law, to monitor electronic communications. This message is subject to terms available at the following link: http://www.morganstanley.com/disclaimers. If you cannot access these links, please notify us by reply message and we will send the contents to you. By messaging with Morgan Stanley you consent to the foregoing.
Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk