Subject: Re: [boost] [config] [atomic] Problem with placement of BOOST_ALIGNMENT - requesting opinions/suggestions
From: Andrey Semashev (andrey.semashev_at_[hidden])
Date: 2017-02-27 16:00:05
On 02/27/17 16:31, Groke, Paul via Boost wrote:
> While preparing a pull request for my Boost.Atomic extensions for the
> z/OS compiler, I noticed that I have a slight problem there. To get the
> code to work on z/OS, I changed the placement of the BOOST_ALIGNMENT
> macro in atomic/detail/storage_type.hpp from...
> BOOST_ALIGNMENT(16) unsigned char data[Size];
> unsigned char data[Size] BOOST_ALIGNMENT(16);
> ...because unfortunately the z compiler doesn't understand the
> __attribute__ ((__aligned__(x))) when it's at the beginning of a
> variable/data member definition. (For the GNU __attribute__
> ((__aligned__(x))) the new location is also the only documented
> location, although GCC doesn't seem to mind.)
This makes the compiler incompatible with the current semantics of
BOOST_ALIGNMENT. You probably need to revise the changes made in the
recent Boost.Config PR regarding BOOST_ALIGNMENT. Please, prepare an
updating PR for Boost.Config.
> While reviewing my changes I now realized that this won't do, because
> BOOST_ALIGNMENT can also be defined as __declspec(align(x)) or
> alignas(x), both of which are legal at the beginning but not at the end
> of a variable/data member definition.
> (I'm not 100% sure about valid positions of alignas (x) - the examples I
> can find use alignas (x) at the beginning, but the compilers we
> currently use - which include MSVC, GCC and Clang, also seem to accept
> it at the end. __declspec(align(x)) definitely is a problem though.)
Note also that BOOST_ALIGNMENT can (and is) also used as a type
attribute. Does your compiler support that?
> And now I'm wondering what to do about this.
> One way to fix this would obviously be to introduce two additional
> macros, e.g. BOOST_ALIGNMENT_DECLSPEC and BOOST_ALIGNMENT_ATTRIBUTE.
> Only one of those would then expand to something non-empty, and both
> would have to be used in variable definitions. The BOOST_ALIGNMENT macro
> would remain as is for type declarations and for compatibility.
> The code in storage_type.hpp would then have to be re-written as
> BOOST_ALIGNMENT_DECLSPEC(16) unsigned char data[Size] BOOST_ALIGNMENT_ATTRIBUTE(16);
> Which of course is very very ugly.
Yeah, I don't really like the duplication.
> Another option would be a
> BOOST_ALIGNED_VARIABLE(alignment, variable_definition)
> macro. This would be nice in definitions like
> BOOST_ALIGNED_VARIABLE(16, unsigned char data[Size]);
> but much less nice in definitions like
> BOOST_ALIGNED_VARIABLE(16, some_template<foo BOOST_PP_COMMA bar BOOST_PP_COMMA baz>);
> And since not all compilers support variadic macros, this is probably
> the best one could do.
I'm not sure BOOST_PP_COMMA would work. I'm not very good at
preprocessor programming, but doesn't BOOST_PP_COMMA get expanded some
time before BOOST_ALIGNED_VARIABLE? That would result in the latter
being invoked with a variable number of arguments.
A typedef could be used as a workaround, although it may be inconvenient
in some cases.
> The third option I can think of would be to leave things as they are,
> which of course would mean that I can't submit my Boost.Atomic
> implementation for z/OS... hm.
> I realize that making things uglier for a single compiler that almost
> nobody uses isn't something many people will cheer for. But then again
> I'd really like to have my Boost.Atomic implementation for z/OS in the
> official Boost releases :)
> So I'm calling out to you for suggestions/opinions/comments. Is there
> another way to combine the above mentioned square peg + round hole? And
> which option would you deem favorable/acceptable?
Frankly, I'm not quite happy with either option, although
BOOST_ALIGNED_VARIABLE seems the lesser evil.
One other idea that I think is worth trying is to see if your compiler
can cope with alignment attribute in a type definition, in a usual
struct BOOST_ALIGNMENT(16) aligned
unsigned char data[Size];
If it does, the code can be changed to avoid applying BOOST_ALIGNMENT to
variables and use it to align types instead.
Although I used BOOST_ALIGNMENT above for shortness, I shouldn't be
saying BOOST_ALIGNMENT because, as currently documented,
BOOST_ALIGNMENT is not supported by your compiler and thus should expand
to nothing (with BOOST_NO_ALIGNMENT defined). If your compiler supports
type alignment in the above syntax, we could introduce a new macro, e.g.
BOOST_TYPE_ALIGNMENT(n), that would be usable in that context, but not
in the context of a variable declaration. It can be accompanied with
BOOST_NO_TYPE_ALIGNMENT, similar to BOOST_NO_ALIGNMENT.
If it doesn't work with types, then you could just sacrifice 128-bit
atomic ops in Boost.Atomic. No changes to Boost.Atomic should be needed
for that because the lesser types should already get the necessary
alignment naturally, and 128-bit storage will be disabled by the current
preprocessor checks. I think, that would be a good solution with minimal
impact on the existing code unless, of course, you require 128-bit atomics.
Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk