[PATCH, rs6000] Add memory barriers to tbegin, tend, etc.

[Torvald Riegel]

Sorry for the much delayed response.  I've been sick and am slowly
catching up.

On Thu, 2015-09-03 at 16:58 -0500, Peter Bergner wrote:
> On a glibc thread discussing this issue, Torvald also asked that I add
> documention describing the memory consistency semantics the HTM instructions
> should have, so I added a blurb about that.  Torvald, is the text below
> what you were looking for?

> Index: gcc/doc/extend.texi
> ===================================================================
> --- gcc/doc/extend.texi	(revision 227308)
> +++ gcc/doc/extend.texi	(working copy)
> @@ -16030,6 +16030,23 @@
>  unsigned int __builtin_tsuspend (void)
>  @end smallexample
>  
> +Note that the semantics of the above HTM builtins are required to mimic the
> +locking semantics used for critical sections.  Builtins that are used to
> +create a new transaction or restart a suspended transaction (specifically any
> +builtin that changes the state from non-transactional to transactional)

Those builtins could be nested in transactions yet would need to provide
the same semantics at least as far as the compiler is concerned, so
maybe change "specifically" to "for example"?

> must
> +have lock acquisition like semantics while those builtins that end or suspend
> +a transaction (ie, changes the state from transactional to non-transactional)
> +must have lock release like semantics.

I would be a bit more specific just to avoid confusion about how the
locks are implemented or what specific semantics they have.  What about
adding this or something similar:

"Specifically, this must mimic lock semantics as specified by C++11, for
example: Lock acquisition is as-if an execution of
__atomic_exchange_n(&globallock,1,__ATOMIC_ACQUIRE) that returns 0, and
lock release is as-if an execution of
__atomic_store(&globallock,0,__ATOMIC_RELEASE), with globallock being an
implicit implementation-defined lock used for all transactions."

> The HTM instructions associated with
> +with the builtins inherently provide the correct acquisition and release
> +semantics required.

I would say "hardware barriers" or "hardware fences" instead of
"semantics" to clarify that this is just one part.

> However, the compiler must be prohibited from moving
> +loads and stores across the HTM instructions.  This has been accomplished
> +by adding memory barriers to the associated HTM instructions.

I think it would be good to make the requirement tighter, so that we do
not promise too much.  What about changing this to:

"However, the compiler must also be prohibited from moving loads and
stores across the builtins in a way that would violate their semantics.
This has been accomplished by adding memory barriers to the associated
HTM instructions (which is a conservative approach to provide acquire
and release semantics)."

I haven't thought enough about txn suspend/resume/abort to make a
proposal how their required semantics would be specified best.  Maybe we
should call this out explicitly, or maybe the sentences above are
sufficient because they refer to HTM instructions generally. 

Maybe the second "HTM instructions" should be "builtins" too, I don't
know.

> Earlier versions
> +of the compiler did not treat the HTM instructions as memory barriers.
> +A @code{__TM_FENCE__} macro has been added, which can be used to determine
> +whether the current compiler treats HTM instructions as memory barriers or not.

"builtins" instead of "HTM instructions"?

> +This allows the user to explicitly add memory barriers to their code when
> +using an older version of the compiler.
> +
>  The following set of built-in functions are available to gain access
>  to the HTM specific special purpose registers.

Thanks for documenting the semantics!