Hi,
While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at http://testbot.winehq.org/JobDetails.pl?Key=8981
Your paranoid android.
=== WXPPROSP3 (32 bit math) === math.c:2238: Test failed: Got 7fff, expected 7c00 for index 7.
=== WVISTAADM (32 bit math) === math.c:2238: Test failed: Got 7fff, expected 7c00 for index 7.
=== W7PRO (32 bit math) === math.c:2238: Test failed: Got 7fff, expected 7c00 for index 7.
Sorry guys. Looks like there are differences between Windows version. Sigh. Will fix.
Henri, will you get a chance to look at the patch before I re-send so that I can fix any other defects you might see? I don't want to send too many resends to the list.
Thank you so much
Misha
On Sun, Feb 6, 2011 at 9:32 PM, Marvin testbot@testbot.winehq.org wrote:
Hi,
While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at http://testbot.winehq.org/JobDetails.pl?Key=8981
Your paranoid android.
=== WXPPROSP3 (32 bit math) === math.c:2238: Test failed: Got 7fff, expected 7c00 for index 7.
=== WVISTAADM (32 bit math) === math.c:2238: Test failed: Got 7fff, expected 7c00 for index 7.
=== W7PRO (32 bit math) === math.c:2238: Test failed: Got 7fff, expected 7c00 for index 7.
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
Am 07.02.2011 um 03:37 schrieb Misha Koshelev:
- FLOAT full[10] = { 1.0f, -2.0f, 6.55e4, 6.10352e-5, 5.96046e-8, 0.0f, -0.0f, INFINITY, -INFINITY, NAN },
full_exp[10] = { 1.0f, -2.0f, 6.55e4, 6.10352e-5, 5.96046e-8, 0.0f, -0.0f, 65536.0f, -131008.0f, 131008.0f },full_res[10];
I'd rather call them "single" instead of full because 32 bit floats are usually referred to as single precision(doubles are double precision, and 16 bit floats half precision)
math.c:2238: Test failed: Got 7fff, expected 7c00 for index 7.
Looks like Windows doesn't do INF, and returns NaN instead. Your -INF value test already expects NaN as return value, but with the sign bit set. INF or -INF should have all mantissa bits set to 0. NaN looks like +/- INF, just with mantissa != 0(so there are many possible encodings of NaN).
However, it is also possible that Windows doesn't support those special values at all. This is suggested by the fact that +/- NaN is happily converted into a proper single precision value.
On Mon, Feb 7, 2011 at 3:32 AM, Stefan Dösinger stefandoesinger@gmx.at wrote:
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
Am 07.02.2011 um 03:37 schrieb Misha Koshelev:
- FLOAT full[10] = { 1.0f, -2.0f, 6.55e4, 6.10352e-5, 5.96046e-8, 0.0f, -0.0f, INFINITY, -INFINITY, NAN },
- full_exp[10] = { 1.0f, -2.0f, 6.55e4, 6.10352e-5, 5.96046e-8, 0.0f, -0.0f, 65536.0f, -131008.0f, 131008.0f },
- full_res[10];
I'd rather call them "single" instead of full because 32 bit floats are usually referred to as single precision(doubles are double precision, and 16 bit floats half precision)
Done.
math.c:2238: Test failed: Got 7fff, expected 7c00 for index 7.
Looks like Windows doesn't do INF, and returns NaN instead. Your -INF value test already expects NaN as return value, but with the sign bit set. INF or -INF should have all mantissa bits set to 0. NaN looks like +/- INF, just with mantissa != 0(so there are many possible encodings of NaN).
However, it is also possible that Windows doesn't support those special values at all. This is suggested by the fact that +/- NaN is happily converted into a proper single precision value.
Stefan: I believe this is actually a _difference_ between Windows versions.
As you will note on the full results from winetestbot as well as my own testing on XP (what I _thought_ was SP3) is that only 3/9 platforms have this error. The rest pass fine. Thus, it seems a variance in the tests.
Please see http://testbot.winehq.org/JobDetails.pl?Key=8981.
I have attached a patch that passes all tests: https://testbot.winehq.org/JobDetails.pl?Key=8985
What do you think?
If I am missing something please do let me know.
Thank you Misha
p.s. I heard down the grapevine that Henri might be on vacation for a week. If so, please wish him the best for me and I hope he enjoys his time. If I am mistaken, would be great to know. Thank you.
-----BEGIN PGP SIGNATURE----- Version: GnuPG/MacGPG2 v2.0.17 (Darwin)
iQIcBAEBAgAGBQJNT64vAAoJEN0/YqbEcdMwHskP/2UvtNtsTuC6wL9skKbAhMxf 9AyjSx+k8BOb2fQpkLrrR8gPC3feERax+MCrprPDgjSWftWKxWezfRDSAL9eR/+E BQNRq0v+NfEITVCHVbMee3CNV+acWv2rXViz2HT03JCbuTe3htBytf8XPGPcrfUj ls1KjNPVoYxH5cQJ4sVwAxbMR7CVI9F++B9LiLV8UzJkeYhztilKC3Qu+PLj1QFh jXhfbwHMgCo4cunLIPTxZFndFcLvKwHVUSpZcFmY/OQBxWDAKqlo4fQKVw8Gonlb Q0AbwKMbWQeOPq8Wlyb0XFV35qdUdDkAkJ0139SiyMCakgzmnebAmpx2Ur5/LNQl t82jG+30Ajvnq43oXyczrmoueLtCdOqrXUiEl8wSpcQqFdC13N67NzzGNRvhBfNK wJw25P9VZZCvpw5Wx3iqVXpk1Honrjmrn/hgLLHCKOA1Iv4mByp1svu263n4mfIq Ga3gUdKzcv++1qr9eOfFfG7waGxrNZ8+X2q5Pp6f0SmeoX2nlhYjzOlCOdFDbFA1 yEjHXFYkeZw9PD83c7fOkpLkrEcyDCEol9reDzzQXqmFKYoYP2JLLvRjjnPWoxk0 LviYxmFQ1kZMBhR6WojGx1WjEPVEa+uQxmrK4n6H8fymsXVGu5TfbFgJvmrmg2hG hRDKzYpNRuUYojjHKP6N =EhgD -----END PGP SIGNATURE-----
Am Montag 07 Februar 2011, 19:37:09 schrieb Misha Koshelev:
Stefan: I believe this is actually a _difference_ between Windows versions.
As you will note on the full results from winetestbot as well as my own testing on XP (what I _thought_ was SP3) is that only 3/9 platforms have this error. The rest pass fine. Thus, it seems a variance in the tests.
The results still don't make sense.
Basically it seems to me that native d3dx9 lacks support for NaN and INF. Those magic numbers are created by making the highest possible exponent a special marker(thus reduce exp_max by 1 and make exp_max + 1 special). This is why halfs end up with 65536.0 as just not reachable number. The highest exponent is 15, the highest mantissa value is 1023. This gives value = 2^15 * (1 + 1023/1024) = 65504.
However, it looks like Windows(all versions) treat e = 16 like regular numbers, allowing max numbers from -131008.0 to +131008.0. This is shown by the conversion of 0x7fff to 131008.0 and 0xffff to -131008.0, and the conversion of -INF to 0xffff (sign bit, exp=max+1=16, mantissa=max=1023). I suspect someone tested INF and saw that it was converted into something that looked like NaN and added a line like this:
if(isinf(input)) return 0x7c00; or if(input > 65504) return 0x7c00;
but didn't properly implement NaN and INF. Then someone realized such a check is just stupid given the lack of infrastructure and removed it. And since the code is maybe written in assembler the check got only removed from the 32 bit version, not the 64 bit one. (Notice how old windows versions and 64 bit ones return 0x7c00 and new 32 bit ones return 0x7fff).
I'd suggest a few more tests: See what a number like 80000.0 is converted to. Look what happens to 65503, 65504, 65505, 65534, 65535, 65536. And see what values like 0x7c01,7c02, 7ffe, 7ffd are converted to.
On Mon, Feb 7, 2011 at 2:14 PM, Stefan Dösinger stefandoesinger@gmx.at wrote:
Am Montag 07 Februar 2011, 19:37:09 schrieb Misha Koshelev:
Stefan: I believe this is actually a _difference_ between Windows versions.
As you will note on the full results from winetestbot as well as my own testing on XP (what I _thought_ was SP3) is that only 3/9 platforms have this error. The rest pass fine. Thus, it seems a variance in the tests.
The results still don't make sense.
Basically it seems to me that native d3dx9 lacks support for NaN and INF. Those magic numbers are created by making the highest possible exponent a special marker(thus reduce exp_max by 1 and make exp_max + 1 special). This is why halfs end up with 65536.0 as just not reachable number. The highest exponent is 15, the highest mantissa value is 1023. This gives value = 2^15 * (1 + 1023/1024) = 65504.
However, it looks like Windows(all versions) treat e = 16 like regular numbers, allowing max numbers from -131008.0 to +131008.0. This is shown by the conversion of 0x7fff to 131008.0 and 0xffff to -131008.0, and the conversion of -INF to 0xffff (sign bit, exp=max+1=16, mantissa=max=1023). I suspect someone tested INF and saw that it was converted into something that looked like NaN and added a line like this:
if(isinf(input)) return 0x7c00; or if(input > 65504) return 0x7c00;
but didn't properly implement NaN and INF. Then someone realized such a check is just stupid given the lack of infrastructure and removed it. And since the code is maybe written in assembler the check got only removed from the 32 bit version, not the 64 bit one. (Notice how old windows versions and 64 bit ones return 0x7c00 and new 32 bit ones return 0x7fff).
I'd suggest a few more tests: See what a number like 80000.0 is converted to. Look what happens to 65503, 65504, 65505, 65534, 65535, 65536. And see what values like 0x7c01,7c02, 7ffe, 7ffd are converted to.
Thanks Stefan.
I believe I've found that the threshold value is actually 65520.0 for conversions from single to half precision (tests are still running but I checked on my machine at least so the half_ver1 values are correct; additionally, I tested half_ver2 without the negative numbers previously on winetestbot http://testbot.winehq.org/JobDetails.pl?Key=8995). The full test results with the attached patch will be here: https://testbot.winehq.org/JobDetails.pl?Key=9000
In any case, I have gotten the half to single precision conversion to work in Wine by extending to 16 bits for the exponent (changing < 31 to < 32).
However, I am still having trouble with the single to half precision and believe I am spinning my wheels a bit at this point.
Here are the errors I'm getting:
math.c:2249: Test failed: Got 7800, expected 7bff or 7bff for index 3. math.c:2249: Test failed: Got ffff, expected fc00 or fce2 for index 8. math.c:2249: Test failed: Got f800, expected fbff or fbff for index 11. math.c:2249: Test failed: Got ffff, expected fc00 or fc00 for index 12. math.c:2249: Test failed: Got ffff, expected fc00 or fc00 for index 13. math.c:2249: Test failed: Got ffff, expected fc00 or fc00 for index 14. math.c:2249: Test failed: Got ffff, expected fc00 or fc00 for index 15. math.c:2249: Test failed: Got 6400, expected 7fff or 7fff for index 18. math.c:2249: Test failed: Got 6400, expected ffff or ffff for index 19.
Any ideas?
Thanks Misha
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
Am 07.02.2011 um 23:49 schrieb Misha Koshelev: I have no full explanation yet, just bits and pieces. By the way, if you haven't done so already I recommend a thorough study of http://en.wikipedia.org/wiki/Floating_point for background knowledge. http://en.wikipedia.org/wiki/IEEE_754-2008 may also be interesting, but I haven't read that wikipedia page so I don't really know what is in it.
I believe I've found that the threshold value is actually 65520.0 for conversions from single to half precision
That makes sense, it is halfway between 65536 and 65604. At this threshold values are rounded up or down
In any case, I have gotten the half to single precision conversion to work in Wine by extending to 16 bits for the exponent (changing < 31 to < 32).
You may as well replace it with TRUE because the exponent won't be >= 32. Too few bits to express anything bigger than 31.
However, I am still having trouble with the single to half precision and believe I am spinning my wheels a bit at this point.
I'm still trying to make sense of the Windows values :-/
math.c:2249: Test failed: Got 7800, expected 7bff or 7bff for index 3.
Looks like the exponent is 1 too high and the mantissa zero instead of 1023. Probably a rounding / off by one bug in the code from wined3d. Different behavior wrt what happens when you exactly hit the rounding threshold. (Ie, do you round an exact 5 up or down(to 0 or 10)?)
math.c:2249: Test failed: Got ffff, expected fc00 or fce2 for index 8. math.c:2249: Test failed: Got ffff, expected fc00 or fc00 for index 12. math.c:2249: Test failed: Got ffff, expected fc00 or fc00 for index 13. math.c:2249: Test failed: Got ffff, expected fc00 or fc00 for index 14. math.c:2249: Test failed: Got ffff, expected fc00 or fc00 for index 15.
This is this statement: + if (*in < -65520) return 0xFFFF; You probably wanted to return fc00 here, if you are trying to emulate case 1. Otherwise remove both CONFUSION HERE statements, change the if(exp > 30) case to exp > 31 and set ret = 0x7fff there. This will give you case 2.
math.c:2249: Test failed: Got 6400, expected 7fff or 7fff for index 18. math.c:2249: Test failed: Got 6400, expected ffff or ffff for index 19.
Hmm, I am afraid you can't remove the check for NaN entirely. If you try to any float operation on NaN undefined things are going to happen(Well, you get NaN, and later try to pick its bits apart)
On Mon, Feb 7, 2011 at 6:20 PM, Stefan Dösinger stefandoesinger@gmx.at wrote:
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
Am 07.02.2011 um 23:49 schrieb Misha Koshelev: I have no full explanation yet, just bits and pieces. By the way, if you haven't done so already I recommend a thorough study of http://en.wikipedia.org/wiki/Floating_point for background knowledge. http://en.wikipedia.org/wiki/IEEE_754-2008 may also be interesting, but I haven't read that wikipedia page so I don't really know what is in it.
I believe I've found that the threshold value is actually 65520.0 for conversions from single to half precision
That makes sense, it is halfway between 65536 and 65604. At this threshold values are rounded up or down
In any case, I have gotten the half to single precision conversion to work in Wine by extending to 16 bits for the exponent (changing < 31 to < 32).
You may as well replace it with TRUE because the exponent won't be >= 32. Too few bits to express anything bigger than 31.
However, I am still having trouble with the single to half precision and believe I am spinning my wheels a bit at this point.
I'm still trying to make sense of the Windows values :-/
math.c:2249: Test failed: Got 7800, expected 7bff or 7bff for index 3.
Looks like the exponent is 1 too high and the mantissa zero instead of 1023. Probably a rounding / off by one bug in the code from wined3d. Different behavior wrt what happens when you exactly hit the rounding threshold. (Ie, do you round an exact 5 up or down(to 0 or 10)?)
math.c:2249: Test failed: Got ffff, expected fc00 or fce2 for index 8. math.c:2249: Test failed: Got ffff, expected fc00 or fc00 for index 12. math.c:2249: Test failed: Got ffff, expected fc00 or fc00 for index 13. math.c:2249: Test failed: Got ffff, expected fc00 or fc00 for index 14. math.c:2249: Test failed: Got ffff, expected fc00 or fc00 for index 15.
This is this statement:
- if (*in < -65520) return 0xFFFF;
You probably wanted to return fc00 here, if you are trying to emulate case 1. Otherwise remove both CONFUSION HERE statements, change the if(exp > 30) case to exp > 31 and set ret = 0x7fff there. This will give you case 2.
math.c:2249: Test failed: Got 6400, expected 7fff or 7fff for index 18. math.c:2249: Test failed: Got 6400, expected ffff or ffff for index 19.
Hmm, I am afraid you can't remove the check for NaN entirely. If you try to any float operation on NaN undefined things are going to happen(Well, you get NaN, and later try to pick its bits apart)
-----BEGIN PGP SIGNATURE----- Version: GnuPG/MacGPG2 v2.0.17 (Darwin)
iQIcBAEBAgAGBQJNUH4mAAoJEN0/YqbEcdMwiNcP/R4sa+Z6L39FtN8qE1nX+Pwj KER3a4MWQ8DWDQgjXR09l3kYua9tM8kH5XmZNNpthH5Uck/CCdFOfCx+j3+CLuJq nbFkT5kwVA4qzv67o9qJxJhCaL2+9rXtB6ADOSU3QfLYn7L/UXzxwENzxgQlvFG5 +0GzkdlimMGB8+Rj8mYte/ORFEQQnLoCFe3d1UT9RQQhOCqQvlDuaptBWQCiFiAc GgSQuBzC7YbZxLz8i0EDjfn50X5lW0iVo0p682RJzmod+oCeFztjY/G33wGyA1MA 45Ui6onTYpN1OES1esw/WtePkM8dKrSDlw+gu5rmU9c8JwhPFnNvs/1FUvHD1bGG WjHkv3hLCf3XVjMk1aT70ItFQ8ph2b6IlU3GOSvQvrYbDB8jfb7lbJnuqzd4hrd4 ycTzAf5YULwuQ5m7Ie5QtWO+hG0iB/I0aqGIAnNj6tSO3b/bfmBlQKAauVnoUnDq sYgIEmW0GO15Ndsd/ipfbh414qQsNPyGQNdnhivkJK2PrVaiyXb/ztJf/vLDKjEw cRPoaAfQyIsfPD+cqKq4pnw2rwvr2tkOby11UQ8jfDeaTldp3redf0py0o37CGq1 520QC0CqC/A4MHawwbgcbFTi6XX/DCkHW4IOEirJvnweV8mw8V9IP4XyY/xmYepV ukB4hwY8XvIe4/Zxj8IP =bs4K -----END PGP SIGNATURE-----
Wow, thanks so much. That really helped.
The attached patch now passes all Wine tests and Windows tests as well: https://testbot.winehq.org/JobDetails.pl?Key=9020
I had to use the signbit function as the > 0.0f check does not work for NaNs.
Any other suggestions for the patch before re-submission to wine-patches? And sorry if I missed is Henri on vacation?
Thanks
Misha
Misha Koshelev misha680@gmail.com wrote:
+static inline unsigned short float_32_to_16(const float *in)
+static inline float float_16_to_32(const unsigned short *in) {
Is there a reason that you pass a pointer to the helper instead of a value? Also placing a brace on its own line would follow the style of other functions.
On Tue, Feb 8, 2011 at 12:02 AM, Dmitry Timoshkov dmitry@codeweavers.com wrote:
Misha Koshelev misha680@gmail.com wrote:
+static inline unsigned short float_32_to_16(const float *in)
+static inline float float_16_to_32(const unsigned short *in) {
Is there a reason that you pass a pointer to the helper instead of a value? Also placing a brace on its own line would follow the style of other functions.
-- Dmitry.
Fair enough. The latter is a Java-ism.
How about this one?
Thanks Misha
Am Dienstag 08 Februar 2011, 17:52:06 schrieb Misha Koshelev:
switch (fpclassify(in))
Is this portable? Does it work on msvc too?
I don't have msvc. Please suggest how I can check this. Thank you. Yours Misha
On Feb 8, 2011 4:01 PM, "Stefan Dösinger" stefandoesinger@gmx.at wrote:
Am Dienstag 08 Februar 2011, 17:52:06 schrieb Misha Koshelev:
switch (fpclassify(in))
Is this portable? Does it work on msvc too?
On 2/8/11 2:45 PM, Misha Koshelev wrote:
I don't have msvc. Please suggest how I can check this.
By looking on MSDN. All the library functions that Visual C++ supports are documented there.
Chip
Am Dienstag 08 Februar 2011, 22:45:52 schrieb Misha Koshelev:
I don't have msvc. Please suggest how I can check this. Thank you. Yours Misha
According to http://www.johndcook.com/math_h.html it exists, but has a different name. So we'd need some wrapper around it in libwine. Maybe one exists already, I can't check right now. If not I recommend to stick to isnan() and isinf()
On Wed, Feb 9, 2011 at 4:40 AM, Stefan Dösinger stefandoesinger@gmx.at wrote:
Am Dienstag 08 Februar 2011, 22:45:52 schrieb Misha Koshelev:
I don't have msvc. Please suggest how I can check this. Thank you. Yours Misha
According to http://www.johndcook.com/math_h.html it exists, but has a different name. So we'd need some wrapper around it in libwine. Maybe one exists already, I can't check right now. If not I recommend to stick to isnan() and isinf()
Thanks. Do you have a good way to compare to 0.0f and -0.0f? For some reason, when I took out references and instead used actually float parameters, my comparisons of ((unsigned float) in) == 0x00000000 became true for Inf and NaN as well?
Thank you Misha
On Wed, Feb 9, 2011 at 4:40 AM, Stefan Dösinger stefandoesinger@gmx.at wrote:
Am Dienstag 08 Februar 2011, 22:45:52 schrieb Misha Koshelev:
I don't have msvc. Please suggest how I can check this. Thank you. Yours Misha
According to http://www.johndcook.com/math_h.html it exists, but has a different name. So we'd need some wrapper around it in libwine. Maybe one exists already, I can't check right now. If not I recommend to stick to isnan() and isinf()
Stefan et al:
Thank you. How about this guy?
I'm using isnan(), isinf(), and a hex comparison for +0.0f and -0.0f.
Misha
Am Freitag 11 Februar 2011, 20:48:58 schrieb Misha Koshelev:
Stefan et al:
Thank you. How about this guy?
- if (exp > 31)
- {
/* too big */ret = 0x7fff; /* INF */- }
That's not INF, that is NaN. Or, if you don't read it as a special value the highest possible positive number in a float16 format that doesn't understand NaNs or INFs
Otherwise it looks OK
On Fri, Feb 11, 2011 at 3:58 PM, Stefan Dösinger stefandoesinger@gmx.at wrote:
Am Freitag 11 Februar 2011, 20:48:58 schrieb Misha Koshelev:
Stefan et al:
Thank you. How about this guy?
- if (exp > 31)
- {
- /* too big */
- ret = 0x7fff; /* INF */
- }
That's not INF, that is NaN. Or, if you don't read it as a special value the highest possible positive number in a float16 format that doesn't understand NaNs or INFs
Otherwise it looks OK
Thank you.
Misha
Am Freitag 11 Februar 2011, 20:48:58 schrieb Misha Koshelev:
- if (*((unsigned int *)&in) == 0x00000000) return 0x0000;
- if (*((unsigned int *)&in) == 0x80000000) return 0x8000;
Thinking about it, there's something about this line that is not so nice: It relies on the actual encoding of the float, which may technically be platform specific if there's a CPU that implements non-IEEE- 754 floats.
You could try something like this(pseudo code, I removed the *s)
if(in == 0.0) { if(sign(1.0 / in) == positive) return 0x0000; else return 0x8000; }
The idea is that 1.0 / 0.0 returns Inf and 1.0 / -0.0 -Inf. I may be mistaken about that though.
On Fri, Feb 11, 2011 at 10:20:10PM +0100, Stefan D?singer wrote:
Am Freitag 11 Februar 2011, 20:48:58 schrieb Misha Koshelev:
- if (*((unsigned int *)&in) == 0x00000000) return 0x0000;
- if (*((unsigned int *)&in) == 0x80000000) return 0x8000;
Thinking about it, there's something about this line that is not so nice: It relies on the actual encoding of the float, which may technically be platform specific if there's a CPU that implements non-IEEE- 754 floats.
The 'usual' non-IEEE fp systems are vax (you probably can't put enough memory in a vax to run wine!) and some arm systems where the two 32bit words of a double aren't stored in the expected order!
Some embedded systems might not bother with denormalised encodings for values near zero. I don't know if anything uses an abolute offset to avoid that gap (the main difference between aLaw anf uLaw audio).
It also requires that the compiler use stricter data aliasing rules than are required by the C standard. Using a union is ok (but you must write and read a union variable, not just cast to a union type.
David
Am Freitag 11 Februar 2011, 22:39:05 schrieb David Laight:
The 'usual' non-IEEE fp systems are vax (you probably can't put enough memory in a vax to run wine!) and some arm systems where the two 32bit words of a double aren't stored in the expected order!
I don't expect to find any non-IEEE system where wine runs in the near future, or maybe in our lifetime. Its just that this function goes to great lengths to make sure it doesn't rely on the actual encoding that it is annoying that we have to give it up for detecting the sign of zeroes.
If anything I'd say lets just make the actual sign depend on the bits:
if(in == 0.0) { if( ((unsigned int) in) == 0x00000000) return 0x0000; else return 0x8000; } That way if a crazy system crosses our way that happens to encode 42.0 as 0x80000000 we'll get it right and just get the sign of zeroes wrong. However, I'd be glad if we had a really foolproof function.
-----------
For IEEE systems we could optimize the entire routine, protected by ifdefs: 1) Pick apart the 32 bit float like we do in the 16->32 case 2) Shift mantissa and exponent a bit to the right, taking care of rounding 3) Put them together as 16 bit 4) profit!
No exp() and other stuff needed.
On Fri, Feb 11, 2011 at 11:16:55PM +0100, Stefan D?singer wrote:
.... Its just that this function goes to great lengths to make sure it doesn't rely on the actual encoding that it is annoying that we have to give it up for detecting the sign of zeroes.
In that case why not?
int is_neq_z(double x) { union { char i[sizeof (double)]; double d; } ux, u0; int i;
if (x != 0.0) return 0; u0.d = 0.0; ux.d = x; for (i = 0; i < sizeof (double); i++) { if (ux.i[i] != u0.i[i]) return 1; } return 0; }
Which works provided that there are only 2 encoding patterns for zero.
David
On Fri, Feb 11, 2011 at 5:16 PM, Stefan Dösinger stefandoesinger@gmx.at wrote:
Am Freitag 11 Februar 2011, 22:39:05 schrieb David Laight:
The 'usual' non-IEEE fp systems are vax (you probably can't put enough memory in a vax to run wine!) and some arm systems where the two 32bit words of a double aren't stored in the expected order!
I don't expect to find any non-IEEE system where wine runs in the near future, or maybe in our lifetime. Its just that this function goes to great lengths to make sure it doesn't rely on the actual encoding that it is annoying that we have to give it up for detecting the sign of zeroes.
If anything I'd say lets just make the actual sign depend on the bits:
if(in == 0.0) { if( ((unsigned int) in) == 0x00000000) return 0x0000; else return 0x8000; } That way if a crazy system crosses our way that happens to encode 42.0 as 0x80000000 we'll get it right and just get the sign of zeroes wrong. However, I'd be glad if we had a really foolproof function.
For IEEE systems we could optimize the entire routine, protected by ifdefs:
- Pick apart the 32 bit float like we do in the 16->32 case
- Shift mantissa and exponent a bit to the right, taking care of rounding
- Put them together as 16 bit
- profit!
No exp() and other stuff needed.
Ok guys, I get it.
We are trying to have the function work on non-IEEE systems.
How about the attached. It's basically
if (in == 0.0f) return (sign ? 0x8000 : 0x0000);
Seems to work:
https://testbot.winehq.org/JobDetails.pl?Key=9224
What do you think?
Misha
On Tue, Feb 08, 2011 at 01:02:36PM +0800, Dmitry Timoshkov wrote:
+static inline float float_16_to_32(const unsigned short *in) {
... Also placing a brace on its own line would follow the style of other functions.
The brace goes in column 1 so that [[ and ]] work in vi. Putting the function name in column 1 lets you search for the definition using the anchored regexp ^function_name>
Putting { and } on the same lines as for, while and else avoids problems like determining whether this is intended to be the bottom or top of a loop: bar = 1; } while (very long conditional expression that is truncated by the ... { int foo;
and whether there is an else on the line you can't see below the bottom of the window when you can see: if (...) { ... } The 'else' problem is much worse if you are looking at a printout on fanfold paper,
David
The brace goes in column 1 so that [[ and ]] work in vi. Putting the function name in column 1 lets you search for the definition using the anchored regexp ^function_name>
Code style in Wine doesn't have anything to do with editors. The basic rule is that when you change existing code or add code to an existing module you should use the style used by the existing code. If you add a new module you may pick any style you like as long as it is somewhat sane.
On Thu, Feb 10, 2011 at 6:13 AM, Stefan Dösinger stefandoesinger@gmx.at wrote:
The brace goes in column 1 so that [[ and ]] work in vi. Putting the function name in column 1 lets you search for the definition using the anchored regexp ^function_name>
Code style in Wine doesn't have anything to do with editors. The basic rule is that when you change existing code or add code to an existing module you should use the style used by the existing code. If you add a new module you may pick any style you like as long as it is somewhat sane.
Just fyi, I do not mean to speak for Henri, but I believe he had some code preferences with regards to contributing new code to d3dx9 that are stylistically somewhat different than the existing code that was there (at least in mesh.c).
Thank you
Yours Misha
-
Charles Davis -
David Laight -
Dmitry Timoshkov -
Marvin -
Misha Koshelev -
Stefan Dösinger