On Thu, Sep 3, 2015 at 5:58 PM, Peter Bergner <berg...@vnet.ibm.com> wrote: > While debugging a transaction lock elision issue, we noticed that the > compiler was moving some loads/stores outside of the transaction body, > because the HTM instructions were not marked as memory barriers, which > is bad. Looking deeper, I also noticed that neither Intel and S390 > have their HTM instructions marked as memory barriers either, although > Andi did submit a patch last year: > > https://gcc.gnu.org/ml/gcc-patches/2014-10/msg02999.html > > Richi and r~ both said the memory barrier should be part of the patterns: > > https://gcc.gnu.org/ml/gcc-patches/2014-11/msg02235.html > > The following patch uses that suggestion by adding memory barriers to > all of our HTM instructions, which fixes the issue. I have also added > a __TM_FENCE__ macro users can test to see whether the compiler treats > the HTM instructions as memory barriers or not, in case they want to > explicitly add memory barriers to their code when using older compilers. > > 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? > > This has passed bootstrap/regtesting on powerpc64le-linux. Is this ok > for mainline? > > Since this is a correctness issue, I'd like to eventually backport this to > the release branches. Is that ok once I've verified bootstrap/regtesting > on them? > > Once this is committed, I can take a stab at fixing Intel and S390 similarly, > unless someone beats me to it (hint hint :). I'd need help testing it though, > since I don't have access to Intel or S390 hardware that support HTM. > > Peter > > * config/rs6000/htm.md (UNSPEC_HTM_FENCE): New. > (tabort, tabort<wd>c, tabort<wd>ci, tbegin, tcheck, tend, > trechkpt, treclaim, tsr, ttest): Rename define_insns from this... > (*tabort, *tabort<wd>c, *tabort<wd>ci, *tbegin, *tcheck, *tend, > *trechkpt, *treclaim, *tsr, *ttest): ...to this. Add memory barrier. > (tabort, tabort<wd>c, tabort<wd>ci, tbegin, tcheck, tend, > trechkpt, treclaim, tsr, ttest): New define_expands. > * config/rs6000/rs6000-c.c (rs6000_target_modify_macros): Define > __TM_FENCE__ for htm. > * doc/extend.texi: Update documentation for htm builtins.
This is okay. Torvald should comment on the descriptive text. Thanks, David