On Tue, Mar 14, 2017 at 10:05 AM, Janne Blomqvist <blomqvist.ja...@gmail.com> wrote: > 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.
As 'NightStrike' doesn't have commit access, I have committed r246162. https://gcc.gnu.org/viewcvs/gcc?view=revision&revision=246162 Don't use Win32 functions on CYGWIN. This was a workaround for a cygwin bug which was fixed 4 years ago, and cygwin hasn't supported affected versions for a long time. 2017-03-15 NightStrike <nightstr...@gmail.com> Janne Blomqvist <j...@gcc.gnu.org> * intrinsics/random.c (getosrandom): Remove check for __CYGWIN__ preprocessor flag. * intrinsics/system_clock.c: Likewise. (system_clock_4): Likewise. (system_clock_8): Likewise. * intrinsics/time_1.h: Don't include windows.h if __CYGWIN__ is defined. -- Janne Blomqvist
