[Rd] Optimization bug when byte compiling with gcc 5.3.0 on windows

[Ray Donnelly]

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
>>
>>
>

	[[alternative HTML version deleted]]