On Tue, Mar 14, 2017 at 6:32 AM, NightStrike <nightstr...@gmail.com> wrote: > On Mon, Mar 13, 2017 at 5:01 AM, Janne Blomqvist > <blomqvist.ja...@gmail.com> wrote: >> On Sun, Mar 12, 2017 at 10:46 PM, Janne Blomqvist >> <blomqvist.ja...@gmail.com> wrote: >>> On Sun, Mar 12, 2017 at 7:26 PM, NightStrike <nightstr...@gmail.com> wrote: >>>> On Mon, Feb 27, 2017 at 6:15 AM, Janne Blomqvist >>>> <blomqvist.ja...@gmail.com> wrote: >>>>> Don't try to use rand_s on CYGWIN >>>>> >>>>> CYGWIN seems to include _mingw.h and thus __MINGW64_VERSION_MAJOR is >>>>> defined even though rand_s is not available. Thus add an extra check >>>>> for __CYGWIN__. >>>>> >>>>> Thanks to Tim Prince and Nightstrike for bringing this issue to my >>>>> attention. >>>>> >>>>> Committed as r245755. >>>>> >>>>> 2017-02-27 Janne Blomqvist <j...@gcc.gnu.org> >>>>> >>>>> * intrinsics/random.c (getosrandom): Don't try to use rand_s on >>>>> CYGWIN. >>>> >>>> 1) There was no patch attached to the email. >>> >>> Oh, sorry. Well, you can see it at >>> https://gcc.gnu.org/viewcvs/gcc?view=revision&revision=245755 > > Thanks. You know, this is actually better than attaching a patch (as > a general thing for emails sent after things are already committed), > as there can be no difference between what was emailed and what was > committed. > >>>> 2) As mentioned on IRC, I don't think this is the right fix. The real >>>> problem is that time_1.h includes windows.h on CYGWIN, which it >>>> shouldn't be doing. This then pollutes the translation unit with all >>>> sorts of MINGW-isms that aren't exactly appropriate for cygwin. >>>> Removing the include in time_1.h and then adjusting a few ifdefs in >>>> system_clock.c that also had the same bug fixes the problem. The >>>> testsuite is running right now on this. >>> >>> It used to be something like that, but IIRC there were some complaints >>> about SYSTEM_CLOCK on cygwin that were due to the cygwin >>> clock_gettime() or something like that, and after some discussion with >>> someone who knows something about cygwin/mingw (since you apparently >>> have no memory of it, I guess it was Kai), it was decided to use >>> GetTickCount & QPC also on cygwin. >> >> I searched a bit, and it seems the culprit is the thread starting at >> >> https://gcc.gnu.org/ml/fortran/2013-04/msg00033.html >> >> So it turned out that clock_gettime(CLOCK_MONOTONIC, ...) always >> returned 0 on cygwin, hence the code was changed to use the windows >> API functions GetTickCount and QPC also on cygwin at >> >> https://gcc.gnu.org/ml/fortran/2013-04/msg00124.html >> >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56919 > > That all led me to this thread: > > https://cygwin.com/ml/cygwin/2013-04/msg00184.html > > which led me to: > > https://cygwin.com/ml/cygwin/2013-04/msg00191.html > > where Corinna fixed Angelo's original issue that sparked the whole > thing. So, from my perspective, Cygwin hasn't had this problem in > about 4 years. > > To be complete, though, I took Angelo's original code and compiled it > with a GCC built with the change I suggested, and I received this: > > $ ./a.exe > 9.50646996E-02 0.435180306 0.939791977 0.851783216 > 0.308901191 0.447312355 0.766363919 0.163527727 > 1.25432014E-02 > > $ ./a.exe > 0.445786893 9.30446386E-03 0.880381405 0.123394549 > 1.23693347E-02 0.485788047 0.623361290 0.921140671 > 0.119319797 > > $ ./a.exe > 0.378171027 0.439836979 0.440582573 1.17767453E-02 > 0.427448869 0.530438244 0.182108700 0.147965968 > 0.668991745 > > $ ./a.exe > 2.73125172E-02 0.916011810 0.854288757 0.913502872 > 0.508077919 0.210798383 8.76839161E-02 0.120695710 > 0.337186754 > > > I then tried Janus' enhanced version from > https://gcc.gnu.org/ml/fortran/2013-04/msg00034.html > > $ ./a.exe > n= 33 > clock: 744091787 > seed: 744091787 744091824 744091861 744091898 744091935 > 744091972 744092009 744092046 744092083 744092120 744092157 > 744092194 744092231 744092268 744092305 744092342 744092379 > 744092416 744092453 744092490 744092527 744092564 > 744092601 744092638 744092675 744092712 744092749 744092786 > 744092823 744092860 744092897 744092934 744092971 > random: 0.801897824 0.180594921 0.280960143 > 8.65536928E-03 0.122029424 0.473693788 0.161536098 > 0.228073180 0.441518903 > > $ ./a.exe > n= 33 > clock: 744093409 > seed: 744093409 744093446 744093483 744093520 744093557 > 744093594 744093631 744093668 744093705 744093742 744093779 > 744093816 744093853 744093890 744093927 744093964 744094001 > 744094038 744094075 744094112 744094149 744094186 > 744094223 744094260 744094297 744094334 744094371 744094408 > 744094445 744094482 744094519 744094556 744094593 > random: 0.164107382 0.762304962 0.511664748 > 0.700617492 0.692375600 0.207925439 0.920203805 > 0.160881400 0.339902878 > > $ ./a.exe > n= 33 > clock: 744094930 > seed: 744094930 744094967 744095004 744095041 744095078 > 744095115 744095152 744095189 744095226 744095263 744095300 > 744095337 744095374 744095411 744095448 744095485 744095522 > 744095559 744095596 744095633 744095670 744095707 > 744095744 744095781 744095818 744095855 744095892 744095929 > 744095966 744096003 744096040 744096077 744096114 > random: 0.433895171 0.989695787 0.305223107 > 0.387590647 0.673205614 0.539944470 0.849159062 > 0.811356246 0.888609290 > > $ ./a.exe > n= 33 > clock: 744096561 > seed: 744096561 744096598 744096635 744096672 744096709 > 744096746 744096783 744096820 744096857 744096894 744096931 > 744096968 744097005 744097042 744097079 744097116 744097153 > 744097190 744097227 744097264 744097301 744097338 > 744097375 744097412 744097449 744097486 744097523 744097560 > 744097597 744097634 744097671 744097708 744097745 > random: 0.224010825 0.763803065 0.111726880 > 0.500481725 6.07219338E-02 0.413555145 0.896298766 > 0.142876744 0.286089420 > > > And finally, the output of > https://gcc.gnu.org/ml/fortran/2013-04/msg00038.html which you > requested in https://gcc.gnu.org/ml/fortran/2013-04/msg00207.html as a > test for the original fix: > > $ ./a.exe > 744248371 1000 2147483647 > 744248371346087 1000000000 9223372036854775807 > > $ ./a.exe > 744249100 1000 2147483647 > 744249100677540 1000000000 9223372036854775807 > > $ ./a.exe > 744249678 1000 2147483647 > 744249678546099 1000000000 9223372036854775807 > > $ ./a.exe > 744250116 1000 2147483647 > 744250116491405 1000000000 9223372036854775807 > > > > So............ I know this was a long email, but it seems to me that > the better upstream fix went in a few weeks before these other > libgfortran changes, and they allow for a libgfortran that's untainted > by windows.h. I really think this is the better, safer approach that > will prevent future errors, and remove the need for CYGWIN macro > checks in multiple places.
Ok for trunk. You said on IRC that cygwin is a rolling release system and they don't support old releases, and the bug that prompted this was fixed 4 years ago, so I agree we don't need to support that anymore. -- Janne Blomqvist