Re: [PATCH] libitm: Add custom HTM fast path for RTM on x86_64.

[Torvald Riegel <triegel at redhat dot com>]

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

Attachment: patch1
Description: Text document