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

[Torvald Riegel <triegel at redhat dot com>]

On Sun, 2017-02-26 at 08:55 +0100, Florian Weimer wrote:
> * 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.

I don't read it that way, and I think it is intended to explain the
intention of this paragraph.

> 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.

But it makes a statement about pointers being interchangeable and, at
least as I read it, compatible.  So we can transform this into a
question about unions.

> > 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.

I'm not saying that we should change the definition to a union.  What
2.14.2 in the document cited above states is that the pointers to the
union and the individual parts are interchangeable.  So we can use a
pointer to a part (ie, non-volatile) interchangeably with the pointer to
the union that we use internally.