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

Reply via email to