Following on from @ronsaldo's PR #311, these are some additional fixes for 64-bit MSVC build from my minheadless trial (http://forum.world.st/Minheadless-trial-td5082569.html). We both made some changes around 32/64 bit detection in CMakeLists.txt so that still needs some work. All my other fixes were already covered in PR #311. You can view, comment on, or merge this pull request online at:
https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/313
-- Commit Summary --
* Fixes for 64-bit MSVC minheadless build
-- File Changes --
M platforms/Cross/plugins/IA32ABI/x64win64abicc.c (3) M platforms/minheadless/windows/sqPlatformSpecific-Win32.c (6) M platforms/minheadless/windows/sqPlatformSpecific-Win32.h (4)
-- Patch Links --
https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/313.patch https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/313.diff
Hi Ben,
platforms/minheadless/windows/sqPlatformSpecific-Win32.c still has to cover both x64 and x86 so IMO the right way to write this:
/* Setup the FPU */ // x64 does not support _MCW_PC or _MCW_IC (https://msdn.microsoft.com/en-us/library/e9b52ceh.aspx) _controlfp(FPU_DEFAULT, _MCW_EM | _MCW_RC );
is
/* Setup the FPU */ // x64 does not support _MCW_PC or _MCW_IC (https://msdn.microsoft.com/en-us/library/e9b52ceh.aspx) #if !defined(_MCW_PC) # define _MCW_PC 0 #endif #if !defined(_MCW_IC) # define _MCW_IC 0 #endif _controlfp(FPU_DEFAULT, _MCW_EM | _MCW_RC | _MCW_PC | _MCW_IC);
On Sun, 2 Dec 2018 at 03:01, Eliot Miranda notifications@github.com wrote:
Hi Ben,
platforms/minheadless/windows/sqPlatformSpecific-Win32.c still has to
cover both x64 and x86 so IMO the right way to write this:
/* Setup the FPU */ // x64 does not support _MCW_PC or _MCW_IC (
https://msdn.microsoft.com/en-us/library/e9b52ceh.aspx)
_controlfp(FPU_DEFAULT, _MCW_EM | _MCW_RC );
is
/* Setup the FPU */ // x64 does not support _MCW_PC or _MCW_IC (
https://msdn.microsoft.com/en-us/library/e9b52ceh.aspx)
#if !defined(_MCW_PC) #define _MCW_PC 0 #endif
#if !defined(_MCW_IC) #define _MCW_IC 0 #endif
_controlfp(FPU_DEFAULT, _MCW_EM | _MCW_RC | _MCW_PC | _MCW_IC);
Good point. I didn't consider that. But given that _controlfp is called from two locations, I'm not sure where to put the defines. * up the top of the c-file * close to the first call for definition/use locality * factor both calls out to a function
Also is there any concern about that the possibility of different behaviour between 32-bit and 64-bit? I would guess not, but just asking.
Then I got curious what those defs actually were... [1] https://docs.microsoft.com/en-us/previous-versions/c9676k6h(v=vs.140) [2] https://www.intel.com/content/dam/support/us/en/documents/processors/pentium...
_MCW_IC Infinity control [1] "8.1.6 Infinity Control Flag [2] The infinity control flag (bit 12 of the x87 FPU control word) is provided for compatibility with the Intel 287 Math Coprocessor; it is not meaningful for later version x87 FPU coprocessors or IA-32 processors."
So it seems redundant, or maybe I will learn something new today. Do we expect any modern supported machine to be using a 287 Coprocessor?
_MCW_PC Precision control [1] "8.1.5.2 Precision Control Field [2] The precision-control (PC) field (bits 8 and 9 of the x87 FPU control word) determines the precision (64, 53, or 24 bits) of floating-point calculations made by the x87 FPU (see Table 8-2). The ##>>default precision is double extended precision<<##, which uses the full 64-bit significand available with the double extended-precision floating-point format of the x87 FPU data registers. This setting is best suited for most applications, because it allows applications to take full advantage of the maximum precision available with the x87 FPU data registers."
So do we ever change the FPU away from the default to 53bits or 24bits? If not, then it seems redundant. The code example of [1] shows how _MCW_PC and _PC_24 are used to change the FPU precision and back.
cheers -ben
Hi Ben,
put the definitions at the top of the file or at the first occurrence of _controlfp as you see fit. It does look as if we can ignore _MCW_IC. _MCW_PC on the other hand is vital. If the VM called foreign code though the FFI and that foreign code changed the _MCW_PC mode before calling back, unless the callback entry code resets the fp mode back to the default, the VM's floating point results would be incorrect. Essentially callbacks must reinitialize the processor so that it functions as expected on each callback in case foreign code puts the processor into some other mode. With simple processors this isn't an issue but with the x86 it is.
An alternative may be to jettison the entire use of _controlfp if we can be convinced that the VM only makes use of SSE instructions to implement floating point. That's the case with the JIT floating point code. And we use -SSE3 flags on all compilers I'm aware of. If we go this route I would still include the _controlfp code, surrounded by #if 0 ... #endif, simply to document the issue.
Hi Eliot, I think I captured everything discussed.
One extra thing though... I was wondering that if x64 doesn't support _MCW_RC, then the _PC_53 is irrelevant, then our 64-bit VM was using the CPU default 64-bit extended floating point, so was different from our 32-bit VM, but then I found this insight...
The CPU hardware default is 64-bit precision mode (80-bit long double); Microsoft expects software to set 53-bit mode before any user mode x87 instructions are reached. Microsoft made a change in responsibility for initializing precision mode in the X64 OS. The X64 OS sets 53-bit mode prior to starting your .exe, where the 32-bit OS expected the program to make that initialization.
If that is how you understand it, I think that explanation would be a useful code comment.
Hi Ben,
ideally we'd have bit-identical FP on all platforms. Currently this is only achievable using Andreas Raab's FloatMath plugin, used by Croquet and Terf, and not yet tested in 64-bits. What that has to say about whether we support the default or 53-bit on x64 I don't know. i guess we should for now simply support the default m ode. In any case it's unlikely to make much difference because the computational model yields results being truncated to 53 bits after each operation (i.e. other than in the graphics subsystem, floating point operations are performed on double-precision floats so there's no scope for extended calculations to use 64-bit precision internally. So feel free to include the above as commentary but I think we want to apply your pull request asap and so we should leave the precision argument for a later day when we have more data (such as a 64-bit Terf to test).
FloatMathPlugin is based on Sun microsystems fdlibm. This library does not compile correctly on modern C compiler because it uses pointer aliasing to access bits of a double as two int32. Nowadays, pointer aliasing is undefined behavior. The same library was used in java and compiled with special flags, but this is fragile. It is easy to replace this UB, but that mean forking this now unmaintained code, unless we find another already existing fork. No time to search now.
Le lun. 3 déc. 2018 à 22:31, Eliot Miranda notifications@github.com a écrit :
Hi Ben,
ideally we'd have bit-identical FP on all platforms. Currently this is only achievable using Andreas Raab's FloatMath plugin, used by Croquet and Terf, and not yet tested in 64-bits. What that has to say about whether we support the default or 53-bit on x64 I don't know. i guess we should for now simply support the default m ode. In any case it's unlikely to make much difference because the computational model yields results being truncated to 53 bits after each operation (i.e. other than in the graphics subsystem, floating point operations are performed on double-precision floats so there's no scope for extended calculations to use 64-bit precision internally. So feel free to include the above as commentary but I think we want to apply your pull request asap and so we should leave the precision argument for a later day when we have more data (such as a 64-bit Terf to test).
— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/313#issuecomment-443877985, or mute the thread https://github.com/notifications/unsubscribe-auth/AhLyW19NRT5NIHT-ZwfbaMs_Mn160JzRks5u1ZhKgaJpZM4Y84NR .
Hi Nicolas, thanks for the reminder. The problem is indeed that that plugin is written w.r.t. fdlibm which is effectively obsolete. However there is an alternative. See first this for a good overview of the issue: http://christian-seiler.de/projekte/fpmath/ and then this for a widely used solution: https://www.mpfr.org This still presents problems; we'd hav e to modify the JIT to generate analogous code to Gnu's MPFR. But at least we have a reference implementation against which to test. We should perhaps take this discussion to a new issue.
Okay, its ready to go. Please merge.
Merged #313 into Cog.
mpfr is multiple precision (arbitrary precision if you prefer). IMO it's too much for FloatMathPlugin. CRLIBM (correctly rounded mathematical libray) could be a good modern alternative to fdlibm (from INRIA too).
CRLIBM is also LGPL https://hal-ens-lyon.archives-ouvertes.fr/ensl-01529804/file/crlibm.pdf
vm-dev@lists.squeakfoundation.org
-
Ben Coman -
Ben Coman -
Eliot Miranda
-
Nicolas Cellier -
Nicolas Cellier