Re: [PATCH] Fix memory orders description in atomic ops built-ins docs.

[Matthew Wahab <matthew dot wahab at arm dot com>]

On 21/05/15 19:26, Torvald Riegel wrote:
> On Thu, 2015-05-21 at 16:45 +0100, Matthew Wahab wrote:
> > On 19/05/15 20:20, Torvald Riegel wrote:
> > > On Mon, 2015-05-18 at 17:36 +0100, Matthew Wahab wrote:
> > > > Hello,
> > > >
> > > > On 15/05/15 17:22, Torvald Riegel wrote:
> > > > > This patch improves the documentation of the built-ins for atomic
> > > > > operations.

I think we're talking at cross-purposes and not really getting anywhere. I've replied 
to some of your comments below, but it's mostly a restatement of points already made.

I'll repeat that, although I have concerns about the patch, I don't object to it 
going in. Maybe wait a few days to see if anybody else wants to comment but, at this 
point and, since it's a documentation patch and won't break anything, it's better to 
just commit and deal with any problems come up.

> > We seem to have different views about the purpose of the manual page. I'm treating it
> > as a description of the built-in functions provided by gcc to generate the code
> > needed to implement the C++11 model. That is, the built-ins are distinct from C++11
> > and their descriptions should be, as far as possible, independent of the methods used
> > in the C++11 specification to describe the C++11 memory model.
>
> OK.  But we'd need a *precise* specification of what they do if we'd
> want to make them separate from the C++11 memory model.  And we don't
> have that, would you agree?

There is a difference between the sort of description that is needed for a formal 
specification and the sort that would be needed for a programmers manual. The best 
example of this that I can think of is the Standard ML definition 
(http://sml-family.org). That is a mathematical (so precise) definition that is 
invaluable if you want an unambiguous specification of the language. But its useless 
for anybody who just wants to use Standard ML to write programs. For that, you need 
go to the imprecise descriptions that are given in books about SML and in the 
documentation for SML compilers and libraries.

The problem with using the formal SML definition is the same as with using the formal 
C++11 definition: most of it is detail needed to make things in the formal 
specification come out the right way. That detail, about things that are internal to 
the definition of the specification, makes it difficult to understand what is 
intended to be available for the user.

The GCC manual seems to me to be aimed more at the people who want to use GCC to 
write code and I don't think that the patch makes much allowance for them. I do think 
that more precise statements about the relationship to C++11 are useful to have. Its 
the sort of constraint that ought to be documented somewhere. But it seems to be more 
of interest to compiler writers or, at least, to users who are as knowledgeable as 
compiler writers. A document targeting that group, such as the GCC internals or a GCC 
wiki-page, would seem to be a better place for the information.

(Another example of the distinction may be the Intel Itanium ABI documentation which 
has a programmers description of the synchronization primitives and a separate, 
formal description of their behaviour.)

For what it's worth, my view of how C++11, the __atomics and the machine code line up 
is that each is a distinct layer. Each layer implements the requirements of the 
higher (more abstract) layer but is otherwise entirely independent. That's why I 
think that a description of the __atomic built-in, aimed at compiler users rather 
than writers and that doesn't expect knowledge of C++11 is desirable and possible.

> > I'm also concerned that the patch, by describing things in terms of formal C++11
> > concepts, makes it more difficult for people to know what the built-ins can be
> > expected to do and so make the built-in more difficult to use[..]
>
> I hadn't thought about that possible danger, but that would be right.
> The way I would prefer to counter that is that we add a big fat warning
> to the __sync built-ins that we don't have a precise specification for
> them and that there are several corners of hand-waving and potentially
> further issues, and that this is another reason to prefer the __atomic
> built-ins.  PR 65697 etc. are enough indication for me that we indeed
> lack a proper specification.

Increasing uncertainty about the __sync built-ins wouldn't make people move to 
equally uncertain __atomic built-ins. There's enough knowledge and use of the __sync 
builtins to make them a more comfortable choice then the C++11 atomics and in the 
worst case it would push people to roll their own synchronization functions with 
assembler or system calls.

> Well, "just wants to add a memory barrier" is a the start of the
> problem.  The same way one needs to understand a hardware memory model
> to pick the right HW instruction(s), the same one needs to understand a
> programming language memory model to pick a fence and understand its
> semantics.

Sometimes you just want the hardware instructions and don't care about the 
programming language semantics.

I suspect that for most people using GCC, the only time that the programming language 
specification matters is when their bug-report gets rejected as invalid code. Which 
is to say that you may be expecting more knowledge of the C++11 specification than 
most people actually have.

> > Note that none of this requires a tutorial of any kind. I'm just suggesting that the
> > manual should describe what behaviour should be expected of the code generated for
> > the functions. For the memory orders, that would mean describing what constraints
> > need to be met by the generated code.
>
> I'd bet that if one describes these constraints correctly, you'll get a
> large document -- even if one removes any introductory or explanatory
> parts that could make it a tutorial.

Agreed. It's possible to get an arbitrarily large, precise definition for even the 
more trivial programming language. It depends on how far you want to go with "precise".

That said, my point was that, for user documentation, it's usual to abstract 
sufficiently to be able to give (a good approximation to) a correct, useful and 
succinct description. Completeness isn't necessary, that belongs in the compiler 
writer's docs.

> If you haven't, please just look at
> http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2015/n4455.html
> and try to specify the constraints just for the valid optimizations
> described there.  This should be a good indication of why I think
> specifying the reordering / behavior constraints is nontrivial.

I don't remember seeing this, it's an interesting paper. It deals with things that 
matter to compiler writers though so I don't think it's relevant to point I was 
trying to make (that there's a distinction between documentation for compiler users 
and compiler writers).

> > The requirement that the atomics should support
> > C++11 could be met by making sure that the description of the expected behaviour is
> > sufficient for C++11.
>
> We don't just want the semantics of __atomic* to be sufficient for C++11
> but also want them to be *as weak as possible* to be still sufficient
> for C++11 -- otherwise, we'll make C++11 code less efficient than it can
> be.

Yes, that's implied by 'sufficient'.

> > My understanding is that happens-before is a relation used in the C++11 specification
> > for a specific meaning.[..]
> > so saying that it applies to a gcc built-in would be wrong.
>
> Simplified, we can map 1:1 between an __atomic built-in and an
> equivalent atomic operation.  The exception is basically the data
> definitions for an atomic type (e.g., atomic<T>): While C++11 hides the
> data and data type required for an atomically accessible variable, the
> built-ins assume that the caller will target a suitable memory location.
>
> > Using the
> > gcc built-in rather than the equivalent C++11 library function would result in
> > program that C++11 regards as invalid. (Again, as I understand it.)
>
> It wouldn't be invalid but simply not defined by C++11.  But that's fine
> because the built-ins are a GCC-specific extension (which is compatible
> with C++11 atomics, of course).

But the distinction between the GCC built-ins and the C++11 library functions is 
still there. As far as the C++11 specification is concerned, the GCC __atomic would 
not establish the same relationships, such as inter-thread happens-before, as the 
equivalent C+++11 atomic function. As far as the C++11 specification is concerned, 
the GCC built-in is just another function.

> [...]

Apologies if I missed anything important. As I said, I doubt we'll come to agreement 
on this so I think that the best way to make progress is to see if anybody else has 
objections and commit if not.

Matthew