Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 18 Jul 2012 14:36:16 +1000 (EST)
From:      Bruce Evans <brde@optusnet.com.au>
To:        Konstantin Belousov <kostikbel@gmail.com>
Cc:        Mark Linimon <linimon@lonesome.com>, freebsd-amd64@freebsd.org
Subject:   Re: amd64/169927: siginfo, si_code for fpe errors when error occurs using the SSE math processor
Message-ID:  <20120718140523.X1862@besplex.bde.org>
In-Reply-To: <20120718035220.GO2676@deviant.kiev.zoral.com.ua>
References:  <201207172150.q6HLoFvB096742@freefall.freebsd.org> <20120718103210.E834@besplex.bde.org> <20120718035220.GO2676@deviant.kiev.zoral.com.ua>

next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, 18 Jul 2012, Konstantin Belousov wrote:

> On Wed, Jul 18, 2012 at 10:32:23AM +1000, Bruce Evans wrote:
>> OK.  Also, you can tell instruction that faulted, provided there are
>> no spurious faults.  I think spurious faults can only occur for IRQ13
>> exception handling which was never supported for amd64 and is no longer
>> supported for i386.  My old test program that is broken by not clobbering
>> the status was mainly for finding and fixing such spurious faults.
>> I never owned an i387 or i486SX and had to break i486 exception handling
>> to test IRQ13.
> 486SX+487 never used irq13 as well, since 487 was the 'full' DX package,
> which turned off SX socket.

The 487 is still a separate package.  I thought it had to use IRQ13 to
communicate with the 486SX since the NE# (?) and ERR# (?) pins only
work for a single package.  Do they actually work for the separate
package?

>> Just noticed another style/efficiency bug: this should use curpcb,
>> not curthread->td_pcb.  I don't like the curpcb global, but as
>> long as it exists, it should be used.  Every time I looked at
>> removing this global, it seemed better to keep it.  C code can
>> easily use curthread->td_pcb instead of curpcb, and this is faster
>> if curthread is cached.  But CURPCB is easier to use in asm.
> I changed to use curpcb (its use is indeed mainly for asm context switch
> code). I do want to use pcb_save though, see below.
>> ...
>> I now quote your newer patch for the fputrap_x87() cleanup:
>>
>>> @@ -531,8 +534,9 @@ static char fpetable[128] = {
>>>   * solution for signals other than SIGFPE.
>>>   */
>>>  int
>>> -fputrap()
>>> +fputrap_x87(void)
>>>  {
>>> +	struct savefpu *pcb_save;
>>
>> No need for this before or after.  Let the compiler cache it.  Just use
>> curpcb now.
> The curpcb access shall be spelled as PCPU_GET(curpcb). Please note that
> compiler is not allowed to cache the accesses, since there is __asm
> __volatile expression to indirect through %gs-prefixed read.

Fix curpcb then.  curthread was de-pessimized to use __pure2 and to not
use __volatile.  I could never this to work to cache curthread when I 
ried it, but it apparently works in -current.

curpcb is no more fragile than curthread without the __volatile.
Especially in fputrap*() where they are accessed under critical_enter().
They are switched at the same time.  Any locking in pcpu access functions
for them would be useless, since it goes away when the function returns.
Apparently they are sufficiently locked by the logic of the code
(unlike fpucurthread, which requires the critical_enter()).

curthread is the only pcpu variable important enough to have a special
access function in amd64 pcpu.h.  curpcb is not very important, but
spelling it curpcb = __curpcb() instead of PCPU_GET(curpcb) is
simpler for clients as well as faster.

> diff --git a/sys/amd64/amd64/fpu.c b/sys/amd64/amd64/fpu.c
> ...

OK except for the curpcb caching and the fnclex removal, which should
be done separately.

Speaking of ABI changes, the default i386 FP rounding precision (and
collateral gcc bugs) should have been changed from 53 to 64 bits a
few years ago, after libc (printf) and libm started working around
the old gcc precision bugs well enough.  That will be really painful
for compat code.

Bruce



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20120718140523.X1862>