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
File Changes
Patch Links:
— |
Hi Ben, platforms/minheadless/windows/sqPlatformSpecific-Win32.c still has to cover both x64 and x86 so IMO the right way to write this:
is
#if !defined(_MCW_PC) define _MCW_PC 0#endif define _MCW_IC 0#endif — |
On Sun, 2 Dec 2018 at 03:01, Eliot Miranda <[hidden email]> 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... _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 |
In reply to this post by David T Lewis
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. — |
In reply to this post by David T Lewis
Hi Eliot, I think I captured everything discussed. One extra thing though...
If that is how you understand it, I think that explanation would be a useful code comment. — |
In reply to this post by David T Lewis
Hi Ben,
— |
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 <[hidden email]> a écrit :
|
In reply to this post by David T Lewis
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: — |
In reply to this post by David T Lewis
Okay, its ready to go. Please merge. — |
In reply to this post by David T Lewis
Merged #313 into Cog. — |
In reply to this post by David T Lewis
mpfr is multiple precision (arbitrary precision if you prefer). IMO it's too much for FloatMathPlugin. CRLIBM is also LGPL — |
Free forum by Nabble | Edit this page |