Re: [PATCH 1/2] Optimize generic spinlock code and use C11 like atomic macros.

[Florian Weimer <fw@deneb.enyo.de>]

* Torvald Riegel:

> On Sun, 2017-02-19 at 10:20 +0100, Florian Weimer wrote:
>> * Torvald Riegel:
>> 
>> >> On 12/16/2016 09:12 PM, Florian Weimer wrote:
>> >> > That's undefined:
>> >> >
>> >> > “If an attempt is made to refer to an object defined with a
>> >> > volatile-qualified type through use of an lvalue with
>> >> > non-volatile-qualified type, the behavior is undefined.”
>> >> >
>> >> > But we cannot drop the volatile qualifier from the definition of
>> >> > pthread_spinlock_t because it would change the C++ mangling of
>> >> > pthread_spinlock_t * and similar types.
>> >
>> > Generally, I wouldn't agree with Florian's comment.  However, all we are
>> > interested in is the storage behind the volatile-qualified type.  Users
>> > aren't allowed the object through other means than the pthread_spin*
>> > functions, so if we cast everywhere, we'll never have any
>> > volatile-qualified accesses.
>> 
>> The spinlock is defined with the volatile qualifier.  This means that
>> accesses without the qualifier are undefined.
>
> The footnote for 6.7.3p6 (footnote 133, N1570) reads:
> This applies to those objects that behave as if they were defined with
> qualified types, even if they are
> never actually defined as objects in the program (such as an object at a
> memory-mapped input/output
> address).
>
> I don't remember whether footnotes are normative,

They are not.

> but this doesn't apply in our case.

I think it's a clarification intended to *extend* the scope of the
paragraph to objects not defined using C language elments.  It's not
intended to restrict this paragraph to such objects only.  The
footnote is not particularly meaningful anyway because it discusses
certain vendor extensions whose behavior is not described by the
standard.

(And on GNU, objects created by mmap or dlopen are not implicitly
volatile.)

>> > I believe none of the architectures we
>> > support makes weaker requirements on alignment for volatile-qualified
>> > than for non-volatile-qualified types, so I can't see any problem in
>> > practice with the cast.
>> 
>> We also need separate compilation or some other optimization barrier.
>
> Also consider 2.14.2 in
> https://www.cl.cam.ac.uk/~pes20/cerberus/notes30-full.pdf

Not sure how this applies here.  There's no volatile qualifier in
there.

> This states that one can cast between a union and the pointers to
> individual parts of a union.  If the spinlock's underlying type and the
> volatile-qualified type both have the same size+alignment, then the
> union will have the same size and alignment too.
> In our virtual union, we only ever use the non-volatile member (users
> are not allowed to acccess spinlocks other than through pthread_spin_*
> functions, which don't use volatile-qualified accesses).  But we pass
> the pointer to the volatile member around as handle to the spinlock.

We also need to cover the case when a spinlock is defined like this:

static pthread_spinlock_t lock;

And I don't see a way to get rid of the volatile there without a
compiler extension.

We cannot introduce an union here because …

> Maybe its best to just change pthread_spinlock_t from volatile int to
> int.

… like this, it would change the C++ name mangling of anything related
to pthread_spinlock_t.  It is defined as a typedef, so the mangling
uses the definition to refer to the type, not the name, according to
the language rules, where typedef does not create a new type, and
typedefs with the same definition can be used interchangeably.