[06/11] target-i386: use floatx80 constants in helper_fld*_ST0()

Instead of using a table which doesn't correspond to anything from
physical in the CPU, use directly the constants in helper_fld*_ST0().

Cc: Andreas Färber <andreas.faerber@web.de>
Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target-i386/op_helper.c |   27 ++++++++-------------------
 1 files changed, 8 insertions(+), 19 deletions(-)

Am 15.05.2011 um 16:13 schrieb Aurelien Jarno:

> Instead of using a table which doesn't correspond to anything from
> physical in the CPU, use directly the constants in helper_fld*_ST0().
>
> Cc: Andreas Färber <andreas.faerber@web.de>
> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
> ---
> target-i386/op_helper.c |   27 ++++++++-------------------
> 1 files changed, 8 insertions(+), 19 deletions(-)
>
> diff --git a/target-i386/op_helper.c b/target-i386/op_helper.c
> index 4d309ab..cec0c76 100644
> --- a/target-i386/op_helper.c
> +++ b/target-i386/op_helper.c
> @@ -99,17 +99,6 @@ static const uint8_t rclb_table[32] = {
> #define floatx80_l2e make_floatx80( 0x3fff, 0xb8aa3b295c17f0bcLL )
> #define floatx80_l2t make_floatx80( 0x4000, 0xd49a784bcd1b8afeLL )
>
> -static const floatx80 f15rk[7] =
> -{
> -    floatx80_zero,
> -    floatx80_one,
> -    floatx80_pi,
> -    floatx80_lg2,
> -    floatx80_ln2,
> -    floatx80_l2e,
> -    floatx80_l2t,
> -};
> -
> /* broken thread support */
>
> static spinlock_t global_cpu_lock = SPIN_LOCK_UNLOCKED;
> @@ -3816,42 +3805,42 @@ void helper_fabs_ST0(void)
>
> void helper_fld1_ST0(void)
> {
> -    ST0 = f15rk[1];
> +    ST0 = floatx80_one;
> }

A backport of this using floatx_... compiles okay.
BeOS R5 boots fine; didn't have any special float tests at hand, so I  
ran an OpenGL teapot demo which I assume is software-rendered.

Andreas

> void helper_fldl2t_ST0(void)
> {
> -    ST0 = f15rk[6];
> +    ST0 = floatx80_l2t;
> }
>
> void helper_fldl2e_ST0(void)
> {
> -    ST0 = f15rk[5];
> +    ST0 = floatx80_l2e;
> }
>
> void helper_fldpi_ST0(void)
> {
> -    ST0 = f15rk[2];
> +    ST0 = floatx80_pi;
> }
>
> void helper_fldlg2_ST0(void)
> {
> -    ST0 = f15rk[3];
> +    ST0 = floatx80_lg2;
> }
>
> void helper_fldln2_ST0(void)
> {
> -    ST0 = f15rk[4];
> +    ST0 = floatx80_ln2;
> }
>
> void helper_fldz_ST0(void)
> {
> -    ST0 = f15rk[0];
> +    ST0 = floatx80_zero;
> }
>
> void helper_fldz_FT0(void)
> {
> -    FT0 = f15rk[0];
> +    FT0 = floatx80_zero;
> }
>
> uint32_t helper_fnstsw(void)
> -- 
> 1.7.2.3
>

On 15 May 2011 15:13, Aurelien Jarno <aurelien@aurel32.net> wrote:
> Instead of using a table which doesn't correspond to anything from
> physical in the CPU, use directly the constants in helper_fld*_ST0().

Actually I rather suspect there is effectively a table in the CPU
indexed by the last 3 bits of the FLD* opcode... It would be
possible to implement this group of insns in QEMU with a single
helper function that took the index into the array, but since the
array seems to be causing weird compilation problems we might
as well stick with the lots-of-helpers approach, at which point
this is a sensible cleanup.

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

Am 20.05.2011 um 12:32 schrieb Peter Maydell:

> On 15 May 2011 15:13, Aurelien Jarno <aurelien@aurel32.net> wrote:
>> Instead of using a table which doesn't correspond to anything from
>> physical in the CPU, use directly the constants in helper_fld*_ST0().
>
> Actually I rather suspect there is effectively a table in the CPU
> indexed by the last 3 bits of the FLD* opcode... It would be
> possible to implement this group of insns in QEMU with a single
> helper function that took the index into the array, but since the
> array seems to be causing weird compilation problems we might
> as well stick with the lots-of-helpers approach, at which point
> this is a sensible cleanup.

In OpenBIOS we once ran into a similar error on ppc64 where a cast  
would've resulted in the truncation of a static pointer ... could this  
be an alignment issue here, that it's being truncated by the floatx80  
cast? I tried using __attribute__((packed)) on the floatx80 type  
without luck.
Or maybe the constant width is being handled weird, with LL being 128  
bits rather than the expected 64 bits? ;)

I wasn't specifically asking for the table to be removed, just for  
some working solution.

Andreas

Am 21.05.11 11:35, schrieb Andreas Färber:
> Am 20.05.2011 um 12:32 schrieb Peter Maydell:
>
>> On 15 May 2011 15:13, Aurelien Jarno <aurelien@aurel32.net> wrote:
>>> Instead of using a table which doesn't correspond to anything from
>>> physical in the CPU, use directly the constants in helper_fld*_ST0().
>>
>> Actually I rather suspect there is effectively a table in the CPU
>> indexed by the last 3 bits of the FLD* opcode... It would be
>> possible to implement this group of insns in QEMU with a single
>> helper function that took the index into the array, but since the
>> array seems to be causing weird compilation problems we might
>> as well stick with the lots-of-helpers approach, at which point
>> this is a sensible cleanup.
>
> In OpenBIOS we once ran into a similar error on ppc64 where a cast 
> would've resulted in the truncation of a static pointer ... could this 
> be an alignment issue here, that it's being truncated by the floatx80 
> cast? I tried using __attribute__((packed)) on the floatx80 type 
> without luck.
> Or maybe the constant width is being handled weird, with LL being 128 
> bits rather than the expected 64 bits? ;)

Some more info:

The issue only pops up with -std=c99 or -std=gnu99, with both gcc 3.4.3 
and 4.3.2. That's reproducible on Darwin gcc 4.0.1 and 4.2 as well.

The following trivial sample program triggers it, there's no magic 
Solaris headers involved:

#include <inttypes.h>
// on OpenSolaris:
// typedef unsigned short uint16_t;
// typedef unsigned long long uint64_t;

typedef struct {
     uint64_t low;
     uint16_t high;
} floatx80;
#define make_floatx80(exp, mant) ((floatx80) { mant, exp })

#define floatx80_l2t make_floatx80( 0x4000, 0xd49a784bcd1b8afeLL )

static const floatx80 mine[1] = {
     floatx80_l2t,
};

int main(void)
{
}

Any thoughts?

Andreas