Optimization bug when byte compiling with gcc 5.3.0 on windows
On Mon, Apr 4, 2016 at 12:29 PM, Duncan Murdoch <murdoch.duncan at gmail.com> wrote:
On 03/04/2016 9:44 PM, Ray Donnelly wrote:
Hi, Apologies for breaking the threading on this, I've only just signed up to the list and the last email was from September 2015. I've started to look into building R for Windows using MSYS2 as both the build environment and tools + libraries provider (where possible). I've managed to get the testsuite to pass on a recent MSYS2 MinGW-w64 x86-64 GCC: gcc.exe (Rev1, Built by MSYS2 project) 5.3.1 20160228 I've attached two patches that I needed, described below. I hope this is the appropriate place and way to suggest patches. Comments for improvements are very welcome.
There were no patches attached, just the link to the mingw-w64 project on Github.
Ah, that's strange, they must have got stripped.
Generally the way to produce patches for R is to use svn diff on a checked out working copy of the trunk. On Windows, Tortoisesvn makes this really simple. Then the patch will include information about the revision it's based on. You then upload it to bugs.r-project.org, along with a description of the problem it solves, and mark it as a bug fix or enhancement request.
Ok, I had used diffutils' diff -urN on the 3.2.4-revised release to generate them.
0005-Win32-Extend-sqrt-NA_real_-hack-to-all-GCC-versions.patch Removes the __GNUC__ <= 4 for Windows ISNAN R_sqrt hack and doesn't replace it with any version check since I don't see any reason to second-guess when it might be fixed. When it is fixed in MinGW-w64 we can just remove the hack and be happy (I would hope to be able to get round to this in the next few months).
I can see increasing the version limit when we commit to gcc 5.x, but I think the point of the test is to remind users of new versions to remind the developers that they have a bug. If we work around it forever, it will never get fixed.
OK. Am I right in thinking that many GNU/Linux distributions already build R with GCC > 4? The bug here lies with MinGW-w64 and not with GCC.
0006-Win32-GCC-5.3-Fix-ISNAN-int-emits-UD2-insn.patch The reason that boxplot.stats() was crashing was because when isnan() is called with an int it emits a UD2 instruction to force a crash, so let us just cast the input value to a double to prevent that. The code for this can be seen here: https://github.com/Alexpux/mingw-w64/blob/master/mingw-w64-headers/crt/math.h#L612-L622
This one I can't guess at without seeing the patch.
Both patches modified the exact same lines in eval.c, so I will need to
regenerate it if we are dropping the first patch (and also generate it with
svn diff), but I may as well inline it since it's so simple:
--- src/main/eval.c 2016-04-03 19:46:51.025442100 +0100
+++ src/main/eval.c.new 2016-04-03 19:46:48.279325900 +0100
@@ -3624,7 +3624,7 @@
toolchain or in our expectations, but these defines attempt to work
around this. */
#if (defined(_WIN32) || defined(_WIN64)) && defined(__GNUC__)
-# define R_sqrt(x) (ISNAN(x) ? x : sqrt(x))
+# define R_sqrt(x) (ISNAN((double)x) ? x : sqrt(x))
#else
# define R_sqrt sqrt
#endif
But, we should fix this in MinGW-w64. It's returning a different +/-NaN
from the input NaN in contrast to all the other platforms that R runs on,
as far as I can gather. In that case, I've proposed a patch to address this
issue and if and when this makes it into a release I will send another
patch to R's Bugzilla to avoid this hack altogether if using that release
or a later version, and otherwise to use a combination of the two patches I
supplied earlier.
Duncan Murdoch
-- Best regards, Ray Donnelly, Continuum Analytics Inc.
______________________________________________ R-devel at r-project.org mailing list https://stat.ethz.ch/mailman/listinfo/r-devel