Message ID | 1305468801-6015-7-git-send-email-aurelien@aurel32.net |
---|---|
State | New |
Headers | show |
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
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; } 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)
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(-)