VOGONS

Common searches


First post, by M-HT

User metadata
Rank Newbie
Rank
Newbie

I was looking at a problem with the game Screamer on ARMv8 architecture. It sometimes didn't work, depending on the content of the file CHOICE.DAT.

The problem was with FIST/FISTP instructions.
When storing a value which doesn't fit the target memory, the x87 will store the lowest negative value into the target memory (0x8000 for 16-bit target, 0x80000000 for 32-target and 0x8000000000000000 for 64-bit target).
The current code in Dosbox doesn't do that, at least on ARMv8 architecture - I don't know about other architectures.

I fixed the 16-bit and 32-bit variants by changing these functions in file src/fpu/fpu_instructions.h:

static void FPU_FST_I16(PhysPt addr) {
mem_writew(addr,static_cast<Bit16s>(FROUND(fpu.regs[TOP].d)));
}

static void FPU_FST_I32(PhysPt addr) {
mem_writed(addr,static_cast<Bit32s>(FROUND(fpu.regs[TOP].d)));
}

like this:

static void FPU_FST_I16(PhysPt addr) {
Bit32s value32 = static_cast<Bit32s>(FROUND(fpu.regs[TOP].d));
Bit16s value16 = static_cast<Bit16s>(value32);
mem_writew(addr,(value16==value32)?value32:-32768);
}

static void FPU_FST_I32(PhysPt addr) {
Bit64s value64 = static_cast<Bit64s>(FROUND(fpu.regs[TOP].d));
Bit32s value32 = static_cast<Bit32s>(value64);
mem_writed(addr,(value32==value64)?value32:-2147483648);
}
Last edited by M-HT on 2021-01-02, 11:37. Edited 1 time in total.

Reply 1 of 11, by M-HT

User metadata
Rank Newbie
Rank
Newbie

I thought about it and a more correct but slower solution is to check the boundaries before conversion to integer - like this:

static void FPU_FST_I16(PhysPt addr) {
double value = FROUND(fpu.regs[TOP].d);
mem_writew(addr,(value < 32768.0 && value >= -32768.0)?static_cast<Bit16s>(value):-32768);
}

static void FPU_FST_I32(PhysPt addr) {
double value = FROUND(fpu.regs[TOP].d);
mem_writed(addr,(value < 2147483648.0 && value >= -2147483648.0)?static_cast<Bit32s>(value):-2147483648);
}

static void FPU_FST_I64(PhysPt addr) {
double value = FROUND(fpu.regs[TOP].d);
FPU_Reg blah;
if (value < 9223372036854775808.0 && value >= -9223372036854775808.0) {
blah.ll = static_cast<Bit64s>(value);
} else {
blah.l.lower = 0;
blah.l.upper = -2147483648;
}
mem_writed(addr,blah.l.lower);
mem_writed(addr+4,blah.l.upper);
}
Last edited by M-HT on 2020-12-14, 13:29. Edited 3 times in total.

Reply 3 of 11, by jmarsh

User metadata
Rank Oldbie
Rank
Oldbie

Your value for the I64 comparison looks off by one (it's 2^63-1 instead of 2^63), but it probably wouldn't matter anyway since 63 is too many bits of precision to be accurately represented in a double.
Using frexp() and checking the range of the exponent might be better?

Reply 4 of 11, by M-HT

User metadata
Rank Newbie
Rank
Newbie
jmarsh wrote on 2020-12-13, 21:37:

Your value for the I64 comparison looks off by one (it's 2^63-1 instead of 2^63), but it probably wouldn't matter anyway since 63 is too many bits of precision to be accurately represented in a double.

Yes, you're right. I fixed it.

jmarsh wrote on 2020-12-13, 21:37:

Using frexp() and checking the range of the exponent might be better?

Better how ?

Reply 5 of 11, by Qbix

User metadata
Rank DOSBox Author
Rank
DOSBox Author
blah.ll = static_cast<Bit64s>(value)

Would that work as well for the i64 or is there a real need to round the data twice ?

Water flows down the stream
How to ask questions the smart way!

Reply 6 of 11, by M-HT

User metadata
Rank Newbie
Rank
Newbie
Qbix wrote on 2020-12-14, 08:01:
Would that work as well for the i64 or is there a real need to round the data twice ? […]
Show full quote
blah.ll = static_cast<Bit64s>(value)

Would that work as well for the i64 or is there a real need to round the data twice ?

I missed replacing it. It's now fixed. (The compiler probably optimized it anyway.)

Reply 7 of 11, by Qbix

User metadata
Rank DOSBox Author
Rank
DOSBox Author

Is entering the values into lower and upper endian wise safe ? as I orginally added those to write out 64 bit numbers correctly

would
blah.ll= -9223372036854775808
work ?

(it might be fine, but doesn't hurt to think it over, as endian things can sometimes be tricky)

Water flows down the stream
How to ask questions the smart way!

Reply 8 of 11, by jmarsh

User metadata
Rank Oldbie
Rank
Oldbie
M-HT wrote on 2020-12-14, 05:49:

Better how ?

Just feels a little bit neater/clearer with only one integer comparison against the exponent, since the mantissa value and sign are irrelevant in this case and can be ignored.

static void FPU_FST_I16(PhysPt addr) {
int exp;
double value = FROUND(fpu.regs[TOP].d);
frexp(value,&exp);
mem_writew(addr,(exp < 16) ? static_cast<Bit16s>(value):0x8000);
}

static void FPU_FST_I32(PhysPt addr) {
int exp;
double value = FROUND(fpu.regs[TOP].d);
frexp(value,&exp);
mem_writed(addr,(exp < 32) ? static_cast<Bit32s>(value):0x80000000);
}

static void FPU_FST_I64(PhysPt addr) {
FPU_Reg blah;
int exp;
double value = FROUND(fpu.regs[TOP].d);
frexp(value,&exp);
blah.ll = (exp < 64) ? static_cast<Bit64s>(value):LONGTYPE(0x8000000000000000);
mem_writed(addr,blah.l.lower);
mem_writed(addr+4,blah.l.upper);
}

Reply 9 of 11, by M-HT

User metadata
Rank Newbie
Rank
Newbie
Qbix wrote on 2020-12-14, 13:42:
Is entering the values into lower and upper endian wise safe ? as I orginally added those to write out 64 bit numbers correctly […]
Show full quote

Is entering the values into lower and upper endian wise safe ? as I orginally added those to write out 64 bit numbers correctly

would
blah.ll= -9223372036854775808
work ?

(it might be fine, but doesn't hurt to think it over, as endian things can sometimes be tricky)

Since the values are read from lower and upper, I think that writing into them is safer (endian wise) than writing to blah.ll.

jmarsh wrote on 2020-12-14, 23:19:
M-HT wrote on 2020-12-14, 05:49:

Better how ?

Just feels a little bit neater/clearer with only one integer comparison against the exponent, since the mantissa value and sign are irrelevant in this case and can be ignored.

Maybe this part is neater/cleaner (I guess it depends on your preferences), but you're also exchanging two float comparisons for a library call.

Reply 10 of 11, by Qbix

User metadata
Rank DOSBox Author
Rank
DOSBox Author

frexp is according to gcc mapped to some internal call. I'd guess that it would be just an AND and bitshift in most cases ?

*moving to patches*

Water flows down the stream
How to ask questions the smart way!

Reply 11 of 11, by Qbix

User metadata
Rank DOSBox Author
Rank
DOSBox Author

Went with a variation but doing explicit checks because frexp has some undefined values on NaN and Inf .
Commited in 4436 and 4437

Water flows down the stream
How to ask questions the smart way!