Patchwork libitm: Add custom HTM fast path for RTM on x86_64.

login
register
mail settings
Submitter Torvald Riegel
Date Aug. 30, 2013, 5:17 p.m.
Message ID <1377883078.3196.11778.camel@triegel.csb>
Download mbox | patch
Permalink /patch/271382/
State New
Headers show

Comments

Torvald Riegel - Aug. 30, 2013, 5:17 p.m.
On Fri, 2013-08-30 at 16:49 +0200, Rainer Orth wrote:
> Torvald Riegel <triegel@redhat.com> writes:
> 
> > On Mon, 2013-08-26 at 09:49 -0700, Richard Henderson wrote:
> >> On 08/22/2013 02:57 PM, Torvald Riegel wrote:
> >> > On Thu, 2013-08-22 at 12:05 -0700, Richard Henderson wrote:
> >> >> On 08/22/2013 11:39 AM, Torvald Riegel wrote:
> >> >>> +	/* Store edi for future HTM fast path retries.  We use a stack slot
> >> >>> +	   lower than the jmpbuf so that the jmpbuf's rip field will overlap
> >> >>> +	   with the proper return address on the stack.  */
> >> >>> +	movl	%edi, -64(%rsp)
> >> >>
> >> >> You havn't allocated the stack frame here, and you're storing
> >> >> outside the redzone.  This is invalid.
> >> >>
> >> >> Two possibilities:
> >> >>
> >> >>  (1) always allocate the stack frame on entry to
> >> >>      the function (adds two register additions to
> >> >>      the htm fast path -- in the noise i'd think)
> >> >>
> >> >>  (2) store the edi value in the non-htm path, with
> >> >>      the pr_HTMRetryableAbort bit or'd in.  (adds an
> >> >>      extra store to the non-htm path; probably noise).
> >> >>      You'd want to mask out that bit when you reload it.
> >> > 
> >> > Oops.  Picked fix (2).  Better now?
> >> > 
> >> 
> >> Move the andl of edi down into the HAVE_AS_RTM block, above the orl of
> >> HTMRetriedAfterAbort.  Ok with that change.
> >
> > Committed as r202101.
> >
> 
> The patch has broken Solaris/x86 bootstrap:

Sorry about that.  I'll try to remember to test on non-Linux / non-futex
systems too next time.  I committed the attached patch as r202116.
(Please note that I'll be offline for two weeks starting in about an
hour (yes, bad timing), so I hope I didn't miss anything else...)

Torvald
commit 29006efcecde6ec0366f6c88a662d30b9efb931f
Author: torvald <torvald@138bc75d-0d04-0410-961f-82ee72b054a4>
Date:   Fri Aug 30 17:13:05 2013 +0000

    libitm: Fix wrong initialization order introduced with r202101.
    
    	* config/posix/rwlock.cc: Fix initialization order.
    
    git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@202116 138bc75d-0d04-0410-961f-82ee72b054a4

Patch

diff --git a/libitm/config/posix/rwlock.cc b/libitm/config/posix/rwlock.cc
index 488e9c2..61b6ad9 100644
--- a/libitm/config/posix/rwlock.cc
+++ b/libitm/config/posix/rwlock.cc
@@ -30,11 +30,11 @@  namespace GTM HIDDEN {
 // ??? Move this back to the header file when constexpr is implemented.
 
 gtm_rwlock::gtm_rwlock()
-  : mutex (PTHREAD_MUTEX_INITIALIZER),
+  : summary (0),
+    mutex (PTHREAD_MUTEX_INITIALIZER),
     c_readers (PTHREAD_COND_INITIALIZER),
     c_writers (PTHREAD_COND_INITIALIZER),
     c_confirmed_writers (PTHREAD_COND_INITIALIZER),
-    summary (0),
     a_readers (0),
     w_readers (0),
     w_writers (0)