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

Torvald Riegel triegel@redhat.com
Fri Oct 9 20:19:00 GMT 2015


On Fri, 2015-10-09 at 11:52 -0500, Peter Bergner wrote:
> On Fri, 2015-10-09 at 16:41 +0200, Torvald Riegel wrote:
> > On Thu, 2015-09-03 at 16:58 -0500, Peter Bergner wrote:
> >> +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"?
> 
> Ah, good point.  I forgot about the nested example.  In that case, how about
> we just drop the text within the ()'s instead?  I'm not sure we need it
> given your suggested addition below.

Sounds good to me.

> > 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. 
> 
> Well we do mention that we need acquire semantics not just for instructions
> that start transactions, but also for instructions that resume transactions.
> Similar for release and end/suspend, so I guess I'd lean towards what we have
> is sufficient.

OK.

> 
> > Maybe the second "HTM instructions" should be "builtins" too, I don't
> > know.
> 
> Well the builtins expand into multiple hardware instructions and the
> memory barriers are only attached to the HTM instructions and not the
> other instructions that are generated by the builtins, so I think
> HTM instructions is probably more correct here.  But if you think
> builtins is better, I can change it.

No, it's exactly this aspect of the implementation that I didn't know
about, and thus wasn't sure.  Sounds all good.


> Thanks for the review!  I've attached the changes to the documentation below.
> Is this better?

Yes, thanks!



More information about the Gcc-patches mailing list