Re: Optimization bug when byte compiling with gcc 5.3.0 on windows

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: Optimization bug when byte compiling with gcc 5.3.0 on windows

Ray Donnelly
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.

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

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

--

Best regards,

Ray Donnelly,

Continuum Analytics Inc.
______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Reply | Threaded
Open this post in threaded view
|

Re: Optimization bug when byte compiling with gcc 5.3.0 on windows

Duncan Murdoch-2
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.

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.

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

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

Duncan Murdoch


>
> --
>
> Best regards,
>
> Ray Donnelly,
>
> Continuum Analytics Inc.
> ______________________________________________
> [hidden email] mailing list
> https://stat.ethz.ch/mailman/listinfo/r-devel
>

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Reply | Threaded
Open this post in threaded view
|

Re: Optimization bug when byte compiling with gcc 5.3.0 on windows

Jeroen Ooms.
In reply to this post by Ray Donnelly
On Sun, Apr 3, 2016 at 6:44 PM, Ray Donnelly <[hidden email]> wrote:
> I've started to look into building R for Windows using MSYS2 as both the
> build environment and tools + libraries provider (where possible).

Thanks for your reply, and for the patches.

Last time I had a look at this (a few months ago) another problem was
that mingw-w64 v4 was giving different numeric output for some of the
tests in r-base. If I recall correctly, some eigen vectors had their
direction flipped (negative values became positive and vice versa).
Did you notice anything of this kind when running 'make check' and
'make check recommended' ? It is important to us that numeric results
are reproducible between versions of R.

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Reply | Threaded
Open this post in threaded view
|

Re: Optimization bug when byte compiling with gcc 5.3.0 on windows

Ray Donnelly
In reply to this post by Duncan Murdoch-2
On Mon, Apr 4, 2016 at 12:29 PM, Duncan Murdoch <[hidden email]>
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.
>> ______________________________________________
>> [hidden email] mailing list
>> https://stat.ethz.ch/mailman/listinfo/r-devel
>>
>>
>

        [[alternative HTML version deleted]]

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Reply | Threaded
Open this post in threaded view
|

Re: Optimization bug when byte compiling with gcc 5.3.0 on windows

Ray Donnelly
In reply to this post by Jeroen Ooms.
On Mon, Apr 4, 2016 at 7:39 PM, Jeroen Ooms <[hidden email]>
wrote:

> On Sun, Apr 3, 2016 at 6:44 PM, Ray Donnelly <[hidden email]>
> wrote:
> > I've started to look into building R for Windows using MSYS2 as both the
> > build environment and tools + libraries provider (where possible).
>
> Thanks for your reply, and for the patches.
>

No problem.


> Last time I had a look at this (a few months ago) another problem was
> that mingw-w64 v4 was giving different numeric output for some of the
> tests in r-base. If I recall correctly, some eigen vectors had their
> direction flipped (negative values became positive and vice versa).
> Did you notice anything of this kind when running 'make check' and
> 'make check recommended' ? It is important to us that numeric results
> are reproducible between versions of R.
>

I have not seen make check fail with any reports of eigenvector flip.

        [[alternative HTML version deleted]]

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Reply | Threaded
Open this post in threaded view
|

Re: Optimization bug when byte compiling with gcc 5.3.0 on windows

R devel mailing list
In reply to this post by Jeroen Ooms.
>If I recall correctly, some eigen vectors had their
>direction flipped (negative values became positive and vice versa).
>Did you notice anything of this kind when running 'make check' and
>'make check recommended' ? It is important to us that numeric results
>are reproducible between versions of R.

I think that any code that depends on the direction
of an eigenvector should be considered broken.
It is too bad that eigen's output does not have a class
so that an all.equal method that know that the vector
direction is not relevant could be written for it


Bill Dunlap
TIBCO Software
wdunlap tibco.com

On Mon, Apr 4, 2016 at 11:39 AM, Jeroen Ooms <[hidden email]>
wrote:

> On Sun, Apr 3, 2016 at 6:44 PM, Ray Donnelly <[hidden email]>
> wrote:
> > I've started to look into building R for Windows using MSYS2 as both the
> > build environment and tools + libraries provider (where possible).
>
> Thanks for your reply, and for the patches.
>
> Last time I had a look at this (a few months ago) another problem was
> that mingw-w64 v4 was giving different numeric output for some of the
> tests in r-base. If I recall correctly, some eigen vectors had their
> direction flipped (negative values became positive and vice versa).
> Did you notice anything of this kind when running 'make check' and
> 'make check recommended' ? It is important to us that numeric results
> are reproducible between versions of R.
>
> ______________________________________________
> [hidden email] mailing list
> https://stat.ethz.ch/mailman/listinfo/r-devel
>

        [[alternative HTML version deleted]]

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Reply | Threaded
Open this post in threaded view
|

Re: Optimization bug when byte compiling with gcc 5.3.0 on windows

Martin Maechler
>>>>> William Dunlap via R-devel <[hidden email]>
>>>>>     on Mon, 4 Apr 2016 12:26:38 -0700 writes:

    >> If I recall correctly, some eigen vectors had their
    >> direction flipped (negative values became positive and
    >> vice versa).  Did you notice anything of this kind when
    >> running 'make check' and 'make check recommended' ? It is
    >> important to us that numeric results are reproducible
    >> between versions of R.

    > I think that any code that depends on the direction of an
    > eigenvector should be considered broken.  

I agree [even though I may have fallen into that trap in some of
the regression tests I had written in the long past!].

    > It is too bad that eigen's output does not have a class so
    > that an all.equal method that know that the vector
    > direction is not relevant could be written for it

That's an interesting / neat idea.
So we could give it S3 class "eigen"  iff  only.values is not
true, which would also "solve" the slightly differently
structured result of eigen()

Are you (or others) willing to provide a prototype for
all.equal.eigen ?

Martin

    > Bill Dunlap TIBCO Software wdunlap tibco.com

    > On Mon, Apr 4, 2016 at 11:39 AM, Jeroen Ooms
    > <[hidden email]> wrote:

    >> On Sun, Apr 3, 2016 at 6:44 PM, Ray Donnelly
    >> <[hidden email]> wrote: > I've started to look
    >> into building R for Windows using MSYS2 as both the >
    >> build environment and tools + libraries provider (where
    >> possible).
    >>
    >> Thanks for your reply, and for the patches.
    >>
    >> Last time I had a look at this (a few months ago) another
    >> problem was that mingw-w64 v4 was giving different
    >> numeric output for some of the tests in r-base. If I
    >> recall correctly, some eigen vectors had their direction
    >> flipped (negative values became positive and vice versa).
    >> Did you notice anything of this kind when running 'make
    >> check' and 'make check recommended' ? It is important to
    >> us that numeric results are reproducible between versions
    >> of R.
    >>
    >> ______________________________________________
    >> [hidden email] mailing list
    >> https://stat.ethz.ch/mailman/listinfo/r-devel
    >>

    > [[alternative HTML version deleted]]

    > ______________________________________________
    > [hidden email] mailing list
    > https://stat.ethz.ch/mailman/listinfo/r-devel

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Reply | Threaded
Open this post in threaded view
|

Re: Optimization bug when byte compiling with gcc 5.3.0 on windows

Ravi Varadhan-2
all.equal.eigen() should also accommodate complex vectors, right?

Ravi
________________________________________
From: R-devel <[hidden email]> on behalf of Martin Maechler <[hidden email]>
Sent: Monday, April 11, 2016 3:08 AM
To: William Dunlap
Cc: r-devel; Jeroen Ooms
Subject: Re: [Rd] Optimization bug when byte compiling with gcc 5.3.0   on      windows

>>>>> William Dunlap via R-devel <[hidden email]>
>>>>>     on Mon, 4 Apr 2016 12:26:38 -0700 writes:

    >> If I recall correctly, some eigen vectors had their
    >> direction flipped (negative values became positive and
    >> vice versa).  Did you notice anything of this kind when
    >> running 'make check' and 'make check recommended' ? It is
    >> important to us that numeric results are reproducible
    >> between versions of R.

    > I think that any code that depends on the direction of an
    > eigenvector should be considered broken.

I agree [even though I may have fallen into that trap in some of
the regression tests I had written in the long past!].

    > It is too bad that eigen's output does not have a class so
    > that an all.equal method that know that the vector
    > direction is not relevant could be written for it

That's an interesting / neat idea.
So we could give it S3 class "eigen"  iff  only.values is not
true, which would also "solve" the slightly differently
structured result of eigen()

Are you (or others) willing to provide a prototype for
all.equal.eigen ?

Martin

    > Bill Dunlap TIBCO Software wdunlap tibco.com

    > On Mon, Apr 4, 2016 at 11:39 AM, Jeroen Ooms
    > <[hidden email]> wrote:

    >> On Sun, Apr 3, 2016 at 6:44 PM, Ray Donnelly
    >> <[hidden email]> wrote: > I've started to look
    >> into building R for Windows using MSYS2 as both the >
    >> build environment and tools + libraries provider (where
    >> possible).
    >>
    >> Thanks for your reply, and for the patches.
    >>
    >> Last time I had a look at this (a few months ago) another
    >> problem was that mingw-w64 v4 was giving different
    >> numeric output for some of the tests in r-base. If I
    >> recall correctly, some eigen vectors had their direction
    >> flipped (negative values became positive and vice versa).
    >> Did you notice anything of this kind when running 'make
    >> check' and 'make check recommended' ? It is important to
    >> us that numeric results are reproducible between versions
    >> of R.
    >>
    >> ______________________________________________
    >> [hidden email] mailing list
    >> https://stat.ethz.ch/mailman/listinfo/r-devel
    >>

    >   [[alternative HTML version deleted]]

    > ______________________________________________
    > [hidden email] mailing list
    > https://stat.ethz.ch/mailman/listinfo/r-devel

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel


______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Reply | Threaded
Open this post in threaded view
|

Re: Optimization bug when byte compiling with gcc 5.3.0 on windows

Martin Maechler
>>>>> Ravi Varadhan <[hidden email]>
>>>>>     on Mon, 11 Apr 2016 12:42:35 +0000 writes:

    > all.equal.eigen() should also accommodate complex vectors, right?
    > Ravi

Yes, definitely.   The  ?eigen  help page does nicely mention
that in the complex the eigen vectors are even less determined,
i.e. you could multiply them by any unit length complex number
(of which only two are available in the real case:  +/- 1).

Martin

    > ________________________________________
    > From: R-devel <[hidden email]> on behalf of Martin Maechler <[hidden email]>
    > Sent: Monday, April 11, 2016 3:08 AM
    > To: William Dunlap
    > Cc: r-devel; Jeroen Ooms
    > Subject: Re: [Rd] Optimization bug when byte compiling with gcc 5.3.0   on      windows

>>>>> William Dunlap via R-devel <[hidden email]>
>>>>>     on Mon, 4 Apr 2016 12:26:38 -0700 writes:

    >>> If I recall correctly, some eigen vectors had their
    >>> direction flipped (negative values became positive and
    >>> vice versa).  Did you notice anything of this kind when
    >>> running 'make check' and 'make check recommended' ? It is
    >>> important to us that numeric results are reproducible
    >>> between versions of R.

    >> I think that any code that depends on the direction of an
    >> eigenvector should be considered broken.

    > I agree [even though I may have fallen into that trap in some of
    > the regression tests I had written in the long past!].

    >> It is too bad that eigen's output does not have a class so
    >> that an all.equal method that know that the vector
    >> direction is not relevant could be written for it

    > That's an interesting / neat idea.
    > So we could give it S3 class "eigen"  iff  only.values is not
    > true, which would also "solve" the slightly differently
    > structured result of eigen()

    > Are you (or others) willing to provide a prototype for
    > all.equal.eigen ?

    > Martin

    >> Bill Dunlap TIBCO Software wdunlap tibco.com

    >> On Mon, Apr 4, 2016 at 11:39 AM, Jeroen Ooms
    >> <[hidden email]> wrote:

    >>> On Sun, Apr 3, 2016 at 6:44 PM, Ray Donnelly
    >>> <[hidden email]> wrote: > I've started to look
    >>> into building R for Windows using MSYS2 as both the >
    >>> build environment and tools + libraries provider (where
    >>> possible).
    >>>
    >>> Thanks for your reply, and for the patches.
    >>>
    >>> Last time I had a look at this (a few months ago) another
    >>> problem was that mingw-w64 v4 was giving different
    >>> numeric output for some of the tests in r-base. If I
    >>> recall correctly, some eigen vectors had their direction
    >>> flipped (negative values became positive and vice versa).
    >>> Did you notice anything of this kind when running 'make
    >>> check' and 'make check recommended' ? It is important to
    >>> us that numeric results are reproducible between versions
    >>> of R.

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel