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

[Rainer Orth <ro at CeBiTec dot Uni-Bielefeld dot DE>]

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:

ro@arenal 290 > /bin/ksh ./libtool --tag=CXX   --mode=compile /var/gcc/regression/trunk/11-gcc/build/./gcc/xg++ -B/var/gcc/regression/trunk/11-gcc/build/./gcc/ -nostdinc++ -nostdinc++ -I/var/gcc/regression/trunk/11-gcc/build/i386-pc-solaris2.11/libstdc++-v3/include/i386-pc-solaris2.11 -I/var/gcc/regression/trunk/11-gcc/build/i386-pc-solaris2.11/libstdc++-v3/include -I/vol/gcc/src/hg/trunk/local/libstdc++-v3/libsupc++ -I/vol/gcc/src/hg/trunk/local/libstdc++-v3/include/backward -I/vol/gcc/src/hg/trunk/local/libstdc++-v3/testsuite/util -L/var/gcc/regression/trunk/11-gcc/build/i386-pc-solaris2.11/libstdc++-v3/src -L/var/gcc/regression/trunk/11-gcc/build/i386-pc-solaris2.11/libstdc++-v3/src/.libs -L/var/gcc/regression/trunk/11-gcc/build/i386-pc-solaris2.11/libstdc++-v3/libsupc++/.libs -B/vol/gcc/i386-pc-solaris2.11/bin/ -B/vol/gcc/i386-pc-solaris2.11/lib/ -isystem /vol/gcc/i386-pc-solaris2.11/include -isystem /vol/gcc/i386-pc-solaris2.11/sys-include    -DHAVE_CONFIG_H -I. -I/vol/gcc/src/hg/trunk/local/libitm  -I/vol/gcc/src/hg/trunk/local/libitm/config/x86 -I/vol/gcc/src/hg/trunk/local/libitm/config/posix -I/vol/gcc/src/hg/trunk/local/libitm/config/generic -I/vol/gcc/src/hg/trunk/local/libitm  -march=i486 -mtune=i386 -fomit-frame-pointer -mrtm -Wall -Werror  -Wc,-pthread -std=gnu++0x -funwind-tables -fno-exceptions -fno-rtti -fabi-version=4 -g -O2 -MT rwlock.lo -MD -MP -MF .deps/rwlock.Tpo -c -o rwlock.lo /vol/gcc/src/hg/trunk/local/libitm/config/posix/rwlock.cc
libtool: compile:  /var/gcc/regression/trunk/11-gcc/build/./gcc/xg++ -B/var/gcc/regression/trunk/11-gcc/build/./gcc/ -nostdinc++ -nostdinc++ -I/var/gcc/regression/trunk/11-gcc/build/i386-pc-solaris2.11/libstdc++-v3/include/i386-pc-solaris2.11 -I/var/gcc/regression/trunk/11-gcc/build/i386-pc-solaris2.11/libstdc++-v3/include -I/vol/gcc/src/hg/trunk/local/libstdc++-v3/libsupc++ -I/vol/gcc/src/hg/trunk/local/libstdc++-v3/include/backward -I/vol/gcc/src/hg/trunk/local/libstdc++-v3/testsuite/util -L/var/gcc/regression/trunk/11-gcc/build/i386-pc-solaris2.11/libstdc++-v3/src -L/var/gcc/regression/trunk/11-gcc/build/i386-pc-solaris2.11/libstdc++-v3/src/.libs -L/var/gcc/regression/trunk/11-gcc/build/i386-pc-solaris2.11/libstdc++-v3/libsupc++/.libs -B/vol/gcc/i386-pc-solaris2.11/bin/ -B/vol/gcc/i386-pc-solaris2.11/lib/ -isystem /vol/gcc/i386-pc-solaris2.11/include -isystem /vol/gcc/i386-pc-solaris2.11/sys-include -DHAVE_CONFIG_H -I. -I/vol/gcc/src/hg/trunk/local/libitm -I/vol/gcc/src/hg/trunk/local/libitm/config/x86 -I/vol/gcc/src/hg/trunk/local/libitm/config/posix -I/vol/gcc/src/hg/trunk/local/libitm/config/generic -I/vol/gcc/src/hg/trunk/local/libitm -march=i486 -mtune=i386 -fomit-frame-pointer -mrtm -Wall -pthread -Werror -std=gnu++0x -funwind-tables -fno-exceptions -fno-rtti -fabi-version=4 -g -O2 -MT rwlock.lo -MD -MP -MF .deps/rwlock.Tpo -c /vol/gcc/src/hg/trunk/local/libitm/config/posix/rwlock.cc  -fPIC -DPIC -o .libs/rwlock.o
In file included from /vol/gcc/src/hg/trunk/local/libitm/libitm_i.h:83:0,
                 from /vol/gcc/src/hg/trunk/local/libitm/config/posix/rwlock.cc:25:
/vol/gcc/src/hg/trunk/local/libitm/config/posix/rwlock.h: In constructor 'GTM::gtm_rwlock::gtm_rwlock()':
/vol/gcc/src/hg/trunk/local/libitm/config/posix/rwlock.h:64:18: error: 'GTM::gtm_rwlock::c_confirmed_writers' will be initialized after [-Werror=reorder]
   pthread_cond_t c_confirmed_writers; // Writers wait here for readers
                  ^
/vol/gcc/src/hg/trunk/local/libitm/config/posix/rwlock.h:59:29: error:   'std::atomic<unsigned int> GTM::gtm_rwlock::summary' [-Werror=reorder]
   std::atomic<unsigned int> summary; // Bitmask of the above.
                             ^
/vol/gcc/src/hg/trunk/local/libitm/config/posix/rwlock.cc:32:1: error:   when initialized here [-Werror=reorder]
 gtm_rwlock::gtm_rwlock()
 ^
cc1plus: all warnings being treated as errors

Moving the summary initialization first fixes this.

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University