[fortran, patch] Substring simplification fix
Substrings can't have length of less than zero (i.e. negative length). For example, "foo"(:-1) is the same as "foo"(:0) and "foo"(2:1); all are zero-length strings. The patch for PR 48876 fixed part of an issue of substring simplification, making sure the endpoint of the substring is never less than zero. But, the actual condition should be: never less than the starting point. So, the attached patch corrects the fix for PR 48876 and fixes PR 50409. It expands the original testcase to include the code triggering the new PR. Regtested on x86_64-apple-darwin11, OK to commit to trunk? FX substring.ChangeLog Description: Binary data substring.diff Description: Binary data
[fortran, patches] Two short patches to review
PRs 50540 and 50404 each contain a short patch, written by Steve. Both patches are straightforward: -- 50404: refuse to have a CLOSE statement without a UNIT (F2008's C908 "A file-unit-number shall be specified in a close-spec-list") (http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50404) -- 50540: issue a normal error instead of an internal error, in conditions that could reasonably be triggered by invalid code (http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50540) Both patches seem fine to me, and I have regtested them. Could someone review them and approve them for trunk? I'll then add the testcase from the PR and commit. Cheers, FX
Re: [fortran, patches] Two short patches to review
>> -- 50404: refuse to have a CLOSE statement without a UNIT >> (F2008's C908 "A file-unit-number shall be specified in a >> close-spec-list") (http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50404) > > jerry already approved this one. And I committed it as rev. , with a slight modification to add a decent locus (otherwise, the current locus is at a totally wrong place) and a testcase. Committed patch is attached. close.diff Description: Binary data >> -- 50540: issue a normal error instead of an internal error, in >> conditions that could reasonably be triggered by invalid code >> (http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50540) > > Given that you've read the patch and my comments in the PR, > and that you've contributed/reviewed/committed hundreds of > patches, I think that this is sufficient review. I certainly > have no problems with committing the patch. Well, given the amount of compliment, I'm sure to screw up and then hide :) I'll get to it tomorrow morning, after some sleep. Thanks, FX
Re: [fortran, patch] Add DREAL simplification (last remaining elemental required for initialization expressions)
> PS: Any chance of wrapping the long line to less than 80 characters? Committed as rev. 181198, with the long line in intrinsic.c wrapped. Thanks for reviewing, FX
Re: [fortran, patches] Two short patches to review
> Although I suspect you've been lurking in the background, > welcome back to the land of gfortran hacking. Your first > screw up is free, additional screw ups require you to > fix your screw up and fix an additional bug as your reward. Attached patch committed as revision 181200. FX convert.diff Description: Binary data
[fortran, patch] Fix handling of backtrace options in the library
Hi all, I'll offer my first patch to the new 4.8 trunk. I noticed that the -fbacktrace option and the GFORTRAN_ERROR_BACKTRACE environment variable are somewhat inconsistently handled. Currently, the environment variabled doesn't actually override the compilation option, as it should. So, I set to change that (checking both options.backtrace and compile_options.backtrace instead of only the latter) when I realized that we could improve this further by moving the checking code: it is currently located in the set_options() function, which is called from the main Fortran routine (call generated by the front-end). This means that it's not triggered if the main program is C. So, by moving it in the main initialization routine, we can ensure that an executable with a C main program but run using GFORTRAN_ERROR_BACKTRACE=Y will actually generate backtraces for Fortran errors, which I think is the cool thing to do. Full disclosure: I removed the function maybe_find_addr2line() because it's not needed anymore now the code was regrouped. However, I'll note that I don't understand why it was useful before, and that I think the comment on top of it was wrong (variable "options" could be seen in the function, it was just shadowed by the local function argument!). Thus, while I was at it, I renamed the options argument to set_options(), just to make sure we didn't have a problem of this sort in the future. The change was bootstrapped and regtested on x86_64-apple-darwin11, but is an area not covered by the testsuite. I have manually checked that running various types of abort, both in a pure Fortran code and a mixed C/Fortran (with C main), with all combinations -fno-backtrace/GFORTRAN_ERROR_BACKTRACE, behaved as expected. OK to commit to trunk? FX traceback.diff Description: Binary data traceback.ChangeLog Description: Binary data
[fortran, patch] Allow displaying backtraces from user code
PR 36044 (http://gcc.gnu.org/bugzilla/show_bug.cgi?id=36044) is an enhancement request for a way to display backtraces from user code. I'm against adding yet another nonstandard intrinsic for this purpose (which is how Intel Fortran does it), but I would like to offer the following solution to the issue, as I think it can be useful in some cases (and the way I suggest should not be a maintainance burden for us): -- export _gfortran_show_backtrace() from libgfortran (instead of it being an internal function) -- add documentation on how to call this function from user-code using BIND(C) Patch was bootstrapped and regtested on x86_64-apple-darwin11, also tested with "make info html pdf". OK for trunk? FX traceback2.ChangeLog Description: Binary data traceback2.diff Description: Binary data
[fortran, patch] Improve module version error messages
Hi all, Attached patch was triggered by PR 52313 (http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52313), and tries to improve error messages for module reading. It fixes one spelling of "GFORTRAN" --> "GNU Fortran" and improves the error reporting of old *unversioned* format from: > Parse error when checking module version for file 'blah.mod' opened at (1) to: > Cannot read module file 'blah.mod' opened at (1), because it was created by > an older version of GNU Fortran In addition, I looked at the message emitted for old, *versioned* format: > Wrong module version '6' (expected '9') for file 'blah.mod' opened at (1) I think this message is not actually clearer to the user than the new one, so I substituted the new message to it too. That means we don't present the version strings (which are currently numbers) to the user, and it also seems better to me: those are for internal use anyway, and not documented. They're useful only to us, and they're written in the module file anyway. So, bootstrapped and regtested on x86_64-apple-darwin11, OK for trunk? FX module.ChangeLog Description: Binary data module.diff Description: Binary data
[fortran, patch] Fix display of locus when source contains wide characters
The attached patch fixes PR 36160 (http://gcc.gnu.org/bugzilla/show_bug.cgi?id=36160). It should correctly account for wide characters when display error loci. I'm not sure if we can check that in the testsuite harness, but you can manually see it at work on the attached test.f90. Bootstrapped and regtested on x86_64-apple-darwin11, OK for trunk? FX widechar_locus.ChangeLog Description: Binary data widechar_locus.diff Description: Binary data test.f90 Description: Binary data
Re: [Patch, Fortran] Add -Wc-binding-type warning
> Thus, this patch adds a warning flag for this purpose - and it also excludes > those warnings from the default setting. That's a bit in line with Fortran > 2008 and TS 29113, which remove more and more constraints and force the users > to ensure themselves that the variables are interoperable. However, keeping > it as default warning is also fine with me. No, please really remove them, I find them really annoying (and I think I argued against it at some point during ISO_C_BINDING merge!). They warn you not about something that is wrong, but something that is unportable and could be wrong, but probably is OK in most cases. It can have value, but not as a default warning I think. I don't think I can approve patches given that I'm not following gfortran development very closely, but if I could I would approve it, it seems OK :) FX
Re: [fortran, patch] Improve module version error messages
> What if the .mod file has been created by another compiler? Or do we have a > separate error message for that? For that, we get (only spelling was fixed by my patch, the message already existed): > Fatal Error: File 'foo.mod' opened at (1) is not a GNU Fortran module file I committed the patch based on Steve approval. Thanks for the review! FX
Re: [fortran, patch] Allow displaying backtraces from user code
> I think that this approach is a mistake. The patch > starts us down a slippery slope. Why not export all > the nonstandard intrinsics? In additions, the > _gfortran_ prefix was used to separate the libgfortran > namespace from userspace. Providing a means to > circumvent this separation seems to asking for more > PR. Well, the mean exists. All _gfortran_* functions can already be called, they're part of libgfortran's public (and versioned) API. I'm just saying adding a simple backtrace function to that list is useful, and documenting it too. > I would rather see a new intrinsic procedure add to > gfortran. The standard does not prevent a vendor > from offer additional intrinsic procedures not found > in the standard. I just think multiplicating vendor intrinsics makes our life harder. I'd like to hear other's opinions on this issue, but I'll implement the new intrinsic if that's the consensus. Thanks, FX
Re: [fortran, patch] Fix display of locus when source contains wide characters
> Looks OK to me except for: > >> - for (; i > 0; i--) >> + for (; i > 0;) > > Might as well just make that a while loop. Indeed! Committed with a while loop, thanks for the review! FX
Re: [Patch, libfortran] Reduce default precision for list-directed and G0 real output
> Thus, I suggest that the choice of d should be based on readability > and usefulness for the common case rather than guaranteeing an exact > roundtrip. I'm not sure I see the value in this choice. In addition to standard requirements, quality of implementation issues shouldn't be neglected. At least, I'd suggest surveying how other compilers handle the issue (Sun, which I have at hand, does precise round-trip). FX
[fortran, patch] Follow-up "widechar error" patch
This patch fixes PR 52559 (http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52559), which was due to my earlier patch for displaying error loci in lines containing wide characters (http://gcc.gnu.org/ml/fortran/2012-03/msg00015.html). In preexisting code, a tab is displayed as a single space when an error is printed. I didn't handle it consistently, which should now be fixed. Bootstrapped and regtested on x86_64-apple-darwin11. OK to commit? FX errorfix.ChangeLog Description: Binary data errorfix.diff Description: Binary data
Re: Web page for binaries
Hi, I’m suggesting we apply the following patch to provide links to macOS package managers, where users can download binaries for GCC on macOS. I have included the two major ones, Homebrew and MacPorts. FX mac.diff Description: Binary data mac.ChangeLog Description: Binary data
Re: Web page for binaries
> Just omit the slashes at the ends of the two URLs please. (If you > paste them into Firefox, say, you'll notice they end up getting stripped. > I don't have the RFC handy right now, but for domain names without > directories the / hasn't been necessary for fifteen or so years.) Done. Does the website get rebuilt automatically, or does it need to be triggered? FX
Re: [patch,libgomp] Make libgomp Fortran modules multilib-aware
Given lack of review of this Fortran-specific patch for libgomp, can a Fortran maintainer approve it please? FX Index: libgomp/Makefile.am === --- libgomp/Makefile.am (revision 235843) +++ libgomp/Makefile.am (working copy) @@ -10,7 +10,7 @@ config_path = @config_path@ search_path = $(addprefix $(top_srcdir)/config/, $(config_path)) $(top_srcdir) \ $(top_srcdir)/../include -fincludedir = $(libdir)/gcc/$(target_alias)/$(gcc_version)/finclude +fincludedir = $(libdir)/gcc/$(target_alias)/$(gcc_version)$(MULTISUBDIR)/finclude libsubincludedir = $(libdir)/gcc/$(target_alias)/$(gcc_version)/include vpath % $(strip $(search_path)) > *ping* > > This patch from May makes libgomp install its Fortran modules in the correct > multilib-aware directories, following what libgfortran does. > > > > >> The attached patch allows libgomp to install its Fortran modules in the >> correct multilib-aware directories, just like libgfortran does. >> Without it, multilib Fortran OpenMP code using the modules fails to compile >> because the modules are not found: >> >> $ gfortran -fopenmp a.f90 >> $ gfortran -fopenmp a.f90 -m32 >> a.f90:1:6: >> >> use omp_lib >> 1 >> Fatal Error: Can't open module file ‘omp_lib.mod’ for reading at (1): No >> such file or directory >> compilation terminated. >> >> >> >> Bootstrapped and tested on x86_64-apple-darwin15. OK to commit? >> >> FX >> >> >> >> >> >> >> 2016-05-03 Francois-Xavier Coudert >> >> PR libgomp/60670 >> * Makefile.am: Make fincludedir multilib-aware. >> * Makefile.in: Regenerate.
Re: [PATCH] PR 78534 Change character length from int to size_t
> I was finally able to get a 32-bit i686 compiler going (my attempts to > do this on a x86_64-pc-linux-gnu host failed, in the end I resorted to > running 32-bit builds/tests on a i686 container). At least on i686, > the patch below on top of the big charlen->size_t patch fixes the > failures Patch approved. The old code used gfc_extract_int, which bails out if a non-constant expression is passed, so this is the right thing to do. FX
Re: [PATCH] Fix libgfortran bootstrap error on x86_64-mingw32 (PR target/79127)
> 2017-01-19 Jakub Jelinek > > PR target/79127 > * acinclude.m4 (LIBGFOR_CHECK_AVX512F): Ensure the test clobbers > some zmm16+ registers to verify they are handled by unwind info > properly if needed. > * configure: Regenerated. OK to commit, with an additional comment and a link to the PR. Thanks! FX
Re: [patch, fortran] Warn about integer**(negative integer)
> 2017-01-22 Thomas Koenig > >* arith.c (arith_power): If simplifying integer power expression >to zero, warn if -Winteger-division is given. > > 2017-01-22 Thomas Koenig > >* gfortran.dg/integer_exponentiation_7.f90: New test. The idea seems reasonable, but I am not sure about the wording (“truncated” in particular). “Integer exponentiation truncated to constant 0”? Why not “Negative integer exponent has result value zero”? Other than that, OK to commit. FX
Re: [PATCH] gfortran -- Map REAL128 to REAL kind type with widest precision
Hi Steve, I see Mikael has okayed the patch, but I did not have any time to comment prior. I wanted to note that: - the choice was deliberate, as the standard allows us to choose which real kind REAL128 corresponds to when there are several matching choices. The idea behind the current choice was to avoid forcing the use of slower soft-float arithmetic when a hardware type existed. I don’t have a strong opinion myself on the issue. - This is a breakage of the ABI, so we want to write it in the release notes, and in the doc about ISO_FORTRAN_ENV. - Is this appropriate for stage 4? FX
Re: [PATCH] PR 78534 Change character length from int to size_t
Hi Janne, This is an ABI change, so it is serious… it will require people to recompile older code and libraries with the new compiler. Do we already plan to break the ABI in this cycle, or is this the first ABI-breaking patch of the cycle? And do we have real-life examples of character strings larger than 2 GB? > Also, as there are some places in the frontend were negative character > lengths are used as special flag values, in the frontend the character > length is handled as a signed variable of the same size as a size_t, > although in the runtime library it really is size_t. First, I thought: we should really make it size_t, and have the negative values be well-defined constants, e.g. (size_t) -1 On the other hand, there is the problem of the case where the front-end has different size_t than the target: think 32-bit on 64-bit i386 (front-end size_t larger than target size_t), or cross-compiling for 64-bit on a 32-bit machine (front-end size_t smaller than target size_t). So the charlen type bounds need to be determined when the front-end runs, not when it is compiled (i.e. it is not a fixed type). In iresolve.c, the "Why is this fixup needed?” comment is kinda scary. > I haven't changed the character length variables for the co-array > intrinsics, as this is something that may need to be synchronized with > OpenCoarrays. Won’t that mean that coarray programs will fail due to ABI mismatch? FX
Re: Unreviewed patch
> The following patch has remained unreviewed for a week: > > [build] Disable hwcaps on libgfortran >https://gcc.gnu.org/ml/gcc-patches/2016-12/msg00336.html > > It is required to unbreak bootstrap on Solaris/x86 and, though touching > both libgfortran and libitm, probably needs primarily a build > maintainer or global reviewer. The Fortran part is OK, but I can’t approve the libitm part. FX
Re: [PATCH] Remove unused libgfortran functions
A few questions: 1. Regarding gfortran.map, shouldn’t we just flatten out all the symbols from GFORTRAN_1.0 to GFORTRAN_1.7 into a single new GFORTRAN_2.0 group (while removing the ones we are getting rid of)? 2. While we are breaking the ABI, shouldn’t we reorder the array argument to the libgfortran_set_options to remove unused fields? 3. What will happen to people who try to run older compiled codes with newer library versions? Will it fail with an error message? Segfault? Do we have a way to make it fail in a better way? Cheers, FX
Re: [PATCH] Remove unused libgfortran functions
> Yes, I agree (in general, though I was thinking of making the new one > "GFORTRAN_7" to match the release series). Given that there will not be a 1-to-1 mapping of release series with major ABI versions (hopefully!), I don’t think this is a good idea. It will make people confused. > There's also other things, > like e.g. ISO_C_BINDING helper functions living under the > __iso_c_binding namespace, instead of under _gfortran like everything > else. Agreed. > And while we're at it, should we place everything under > "__gfortran" or "_GFortran", that is, with two underscores or one > underscore followed by a capital letter which in the C world is > reserved for the implementation? Though it's not clear to me whether > libgfortran can claim to be part of "the implementation" vs. being > generic user code. Another issue is that we have some documented, user-callable functions that currently live in the __gfortran_ “namespace”, e.g. the mixed-language routines (https://gcc.gnu.org/onlinedocs/gfortran/Non-Fortran-Main-Program.html). We want to avoid changing those for no reason, and so for consistency I think we should keep everything under __gfortran_ FX
[committed, libgfortran]
In the case where CHMOD is called with a numeric mode, the current code assumes that “mode_t” corresponds to an unsigned int. This is true on linux/glibc, but not true on macOS (where mode_t is an unsigned short) and thus creates a warning and possibly a runtime error. This had been spotted earlier on Windows and was corrected, but behind a __MINWG32__ check, although the fixed code is actually correct for all platforms. I have thus committed the attached fix (revision 243796), removing the target-specific branch. Will work on all platforms now. FX chmod.ChangeLog Description: Binary data chmod.diff Description: Binary data
[libgfortran, patch] Remove iso_c_binding runtime functions
The ISO_C_BINDING procedures have been emitted directly by the front-end since 2012 (see for example https://gcc.gnu.org/ml/fortran/2012-06/msg00152.html and https://gcc.gnu.org/bugzilla/show_bug.cgi?id=32600). Having now broken the library ABI, we can remove them from the library, where they were kept only for compatibility. Bootstrapped and regtested on x86_64-apple-darwin16.3.0 OK to commit? FX iso_c_binding.ChangeLog Description: Binary data iso_c_binding.diff Description: Binary data
[libgfortran, patch] Rename numeric STOP runtime functions
When support for F2008 requirements on numeric STOP statements was implemented, the old _gfortran_stop_numeric() runtime function was made obsolete and a new _gfortran_stop_numeric_f08() function was created, which is the only one used in the front-end nowadays. The old _gfortran_stop_numeric() was kept in libgfortran for ABI compatibility. Now that we are breaking the ABI, the attached patch removes the older _gfortran_stop_numeric() function, and renames the _gfortran_stop_numeric_f08() function into _gfortran_stop_numeric(). That way, it is in line with the names of all other PAUSE/STOP/ERROR STOP runtime functions. Bootstrapped and regtested on x86_64-apple-darwin16.3.0. OK to commit? FX stop.ChangeLog Description: Binary data stop.diff Description: Binary data
[libgfortran, patch] Remove runtime TRANSPOSE support functions
Since 2010, gfortran does not rely on library support functions to handle TRANSPOSE, but instead emits code directly in the front-end (https://gcc.gnu.org/ml/fortran/2010-09/msg00109.html). We have since kept the various _gfortran_transpose_* functions in libgfortran for ABI compatbility, but we can now remove them. Attached patch bootstrapped and regtested on x86_64-apple-darwin16.3.0. OK to commit? FX transpose.ChangeLog Description: Binary data transpose.diff Description: Binary data
Re: [PATCH] Remove unused libgfortran functions
> Now that the libgfortran ABI major version has been bumped, we can > remove functions for which the frontend nowadays generates inline > code. > > This removes the malloc, free, exponent, fraction, nearest, rrspacing, > spacing, set_exponent and transpose intrinsics. Also the unused > store_exe_path function is removed. > > Regtested on x86_64-pc-linux-gnu, Ok for trunk? I’m okaying the patch as is. While we continue cleaning up the library (I have submitted a number of patches now) we should discuss how we will arrange the symbols in gfortran.map at the end. FX
[libgfortran, patch] Remove runtime clz() and ctz() bit intrisic functions
We implement the LEADZ and TRAILZ intrinsics in terms of the built-in clz() and ctz() functions. Since these are not available for 128-bit integer types, we used to have support functions _gfortran_clz128() and _gfortran_ctz128() in libgfortran. In 2010, I applied a patch to avoid this and emit the necessary arithmetic directly from the front-end (https://gcc.gnu.org/ml/fortran/2010-09/msg00181.html). The support functions were kept in libgfortran for backward compatibility, but we can now remove them as we break the ABI. Attached patch is bootstrapped and regtested on x86_64-apple-darwin13.6.0. OK to commit? FX bitint.ChangeLog Description: Binary data bitint.diff Description: Binary data
Re: [libgfortran, patch] Remove runtime TRANSPOSE support functions
> No, this was actually part of r243799 which I just committed (after you Ok'd > it. :) ) Oops. Sorry!
Re: [PATCH] Remove unused libgfortran functions
> Thanks, committed as r243799. I think something went wrong in your commit, as none of the “removed” files were removed: https://gcc.gnu.org/viewvc/gcc?view=revision&revision=243799 FX
Re: [libgfortran, patch] Rename numeric STOP runtime functions
> The patch you posted contains some apparently unrelated changes to > gfortran.map. Without those, Ok. Thanks for the reviews. Patches committed. Wiki updated. I’ll work tonight on some of the remaining items on the “abi cleanup” list. > As a minor cosmetic improvement, you could fold_convert the argument > to integer_type_node instead of gfc_int4_type_node and change the > library functions to take plain C int arguments, since the exit() > argument is a C int anyways. Then this would need to be done with coarray functions too, so I would not feel too comfortable. I’ll leave that as an improvement if there is agreement on that side. FX
[fortran, patch] Remove unused elements in array argument to set_options
For ABI compatibility, we kept some unused elements in the array argument to _gfortran_set_options (options that we have removed). With the current ABI breakage, we might as well remove those. Bootstrapped and regtested on x86_64-apple-darwin16.3.0 OK to commit? FX set_options.ChangeLog Description: Binary data set_options.diff Description: Binary data
Re: [fortran, patch] Remove unused elements in array argument to set_options
> Thinking out loud here. I wonder, however, if we want > to future proof the library against changes to the > options passed by having a few spare unused entried > available. This of course only helps if a new option > needs to be added. It does nothing for removal. That’s actually the way gfortran_set_options() works, to ensure that we can add options in the future: in main() we allocate an array of values, and pass that array and its size to gfortran_set_options(). gfortran_set_options() then deals with the number of options passed, meaning if an older program calls a newer library, the runtime will simply use default values for all options there were not passed. And when we remove options, we simply pass zero values in their stead — until we break the ABI, when we clean up everything. Patch committed, thanks for reviewing. FX
Re: [PATCH] PR 78534 Change character length from int to size_t
Dear Bob, First, regarding the ABI vs. API question: there is no consistent API for how to pass between Fortran and C strings, unless one uses Fortran 2003’s ISO_C_BINDING. It’s an ABI detail, in the sense that every compiler will choose to do things their own way: most compilers who pass a hidden length parameter, although its size (32-bit or 64-bit or size_t) and position (either after the char pointer, or at the end of the argument list) are variable between compilers. So, any code that does this is already compiler-specific. Second, there are good reasons we might want to change this. One is possible use cases (although there are few, by definition, because we simply don’t support those right now). The second one is compatibility with C string-handling functions, who operate on size_t arguments, which means we can now use those functions without casting types around all the time. Finally, if we’re making this change, we welcome any feedback on how to make it as easy as possible to handle in user code. Documentation, preprocessor macros, etc. In particular, one of the things we will need to address is on helping widely used code to adapt to the change, so that. One example I am thinking of, that uses old-style C/Fortran interfaces, is MPI libraries (openmpi & mpich). We definitely need to test those to make sure nothing breaks if we are going to proceed — or they need to be fixed upstream well before we release, and with due note of the incompatibility in our release notes. Cheers, FX
Re: [PATCH] Remove unused libgfortran functions
> I don't understand. Why would it imply a 1:1 mapping of release series > with major ABI versions? OK, I thought you meant to map libgfortran version numbers (libgfortran.so.7 with GCC 7). If it’s the gfortran.map node names, I’m happy with that indeed. Attached patch regtested on x86_64-apple-darwin16.3.0. OK to commit? > Currently we have _gfortran_, that is with a single underscore in the > beginning, so it's not in the "C/POSIX reserved for the implementation > namespace". But yes, I agree that at least those functions documented > under the non-Fortran main program section in the manual should be > kept as is. Then, if we keep some functions under _gfortran_, I say let’s keep them all there. It’s not hurting, and the few users who care have come to expect it. Cheers, FX map.ChangeLog Description: Binary data map.diff Description: Binary data
Re: [PATCH] PR 78867 Function returning string ICE with -flto
> 2016-12-21 Janne Blomqvist > > PR fortran/78867 > * trans-expr.c (gfc_conv_procedure_call): Emit DECL_EXPR also for > non-pointer character results. > > testsuite ChangeLog: > > 2016-12-21 Janne Blomqvist > > PR fortran/78867 > * gfortran.dg/string_length_4.f90: New test. OK
Re: [PATCH] Remove unused libgfortran functions
I followed up with the following patch, committed as revision 243841, removing the old _gfortran_ftell and renaming the modern _gfortran_ftell2. It was bootstrapped and regtested on x86_64-apple-darwin16.3.0. This is the last item from https://gcc.gnu.org/wiki/LibgfortranAbiCleanup <https://gcc.gnu.org/wiki/LibgfortranAbiCleanup> which I feel comfortable addressing. FX ftell.ChangeLog Description: Binary data ftell.diff Description: Binary data
[libgfortran, committed] Include when using strncasecmp
The POSIX function strncasecmp() is used in three libgfortran files. It is located in , but these files include (singular vs. plural). Apparently most implementations (linux, darwin, …) are lenient and allow that, but not mingw32, causing PR 70311 (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70311) The attached patch, bootstrapped and regtested on x86_64-apple-darwin16.3.0, fixes the issue by including the proper header. Committed as revision 243843. FX strings.diff Description: Binary data
[libgfortran, patch] Remove unused headers
The attached patch removes some unused headers from libgfortran sources. It removes 320 assert.h, 31 stdlib.h, 12 string.h, 4 errno.h, and the occasional math.h, unistd.h, and limits.h. I have audited all files manually, checking for function calls. Patch was bootstrapped and regtested on x86_64-apple-darwin16.3.0. OK to commit? FX PS: after that patch, the most often used header (not included in libgfortran.h) is stdlib.h, used in 351 out of 436 library source files. Maybe that warrants including it in libgfortran.h? (next are string.h with 129 includes and limits.h with 113) headers.ChangeLog Description: Binary data headers.diff Description: Binary data
Re: [libgfortran, patch] Remove unused headers
> Ok. Watch out for regressions on other targets though. I have more > than once caused regressions by using some function that happened to > be included in the existing headers on glibc, but needed some other > header on other targets. Committed: https://gcc.gnu.org/viewcvs/gcc?view=revision&revision=243844 I’ve tried to choose based on standards and not glibc, but yes, I will watch out for regressions. > Yes, sounds reasonable to include stdlib.h, yes. I have committed a patch handling stdlib.h: https://gcc.gnu.org/viewcvs/gcc?view=revision&revision=243846 I have also committed a patch removing some unused limits.h in minloc/maxloc, limits.h and errno.h from runtime/minimal.c, and some includes of headers already pulled in from libgfortran.h (math.h from norm and parity, sys/types.h from random.c). Commit message: https://gcc.gnu.org/viewcvs/gcc?view=revision&revision=243847 I have not touched caf/* files, as duplication of includes there seems intentional. FX
Re: [PATCH] PR 78534 Change character length from int to size_t
Hi Janne, Thanks for the patch, it is hard and tedious work. Here is the formal review. I don’t want to be a pain, but I have several questions about the patch, and given its size and the importance I think we should be double-sure :) > I also changed the _size member in vtables from int to size_t, as > there were some cases where character lengths and sizes were > apparently mixed up and caused regressions otherwise. Although I > haven't tested, this might enable very large derived types as well. Regarding that one, why are you making it an explicit size_t and not a charlen type? I know the two will be the same, at least for now, but given that it’s explicitly a character length we should use that variable type. This is a preexisting issue with the front-end and library, where we generally use a mix of types (because they end up being the same anyway, such as C int and GFC_INTEGER_4). Regarding the introduction of is_charlen in gfc_typespec, I am unclear as to why it is needed. It is used exclusively in arith.c, which is not where we should be checking character lengths I think. It is visible by the fact that we normally shouldn’t need access to middle-end headers (tree.h and trans-types.h) at that level. So, can’t we make the check where we currently do it, i.e. later when we translate the constant string? That sounds more reasonable that introducing a new special-cased entity. There are other cases (resolve.c, simplify.c) where you introduce a dependency on middle-end entities (tree.h, trans-types.h) in what are pure Fortran front-end stages. This breaks the separation that currently exists, and which I strongly think we should keep. ** libgfortran ** - in io/write.c, the “for” clauses in in namelist_write() have weird spacing around their semicolons (i.e. space should be after, not before) - in intrinsics/extends_type_of.c, use gfc_charlen_type instead of size_t vtype->size ** front-end ** - class.c: use gfc_charlen_int_kind instead of gfc_size_kind - class.c, in find_intrinsic_vtab(): in the call to gfc_get_int_expr, the third argument cannot be cast from (size_t) to (long), as this would fail on LLP64 hosts - expr.c, regarding gfc_extract_long(): we could definitely extract an host wide int or an mpz value. This new function is called twice: once in resolve_charlen() where we could use the GMP function mpz_sgn() to check if the constant value is negative; the second time in gfc_simplify_repeat, where we should simply bail out (return NULL) if the integer is too big to fit into a long (we would bail out a few lines later anyway, see “semi-arbitrary limit”). - iresolve.c, extra space after NULL in call to gfc_get_int_expr() in gfc_resolve_repeat() - match.c, in select_intrinsic_set_tmp(), charlen should be a gfc_charlen_t and mpz_get_si will break for long string sizes - in resolve.c, like in arith.c, we should not use tree.h and trans-types.h. We should do the comparison by looking at integer kinds, not through the charlen_type_node - in resolve.c, in resolve_select_type(), another case of mpz_get_si() that will break for long string sizes - in simplify.c, again, we should not use tree.h and trans-types.h - trans-decl.c seems like unrelated changes - trans-types.h: why do we now need to include trans.h? Thanks again for working on that! FX
Re: [patch,libgomp] Make libgomp Fortran modules multilib-aware
*ping* This patch from May makes libgomp install its Fortran modules in the correct multilib-aware directories, following what libgfortran does. Index: libgomp/Makefile.am === --- libgomp/Makefile.am (revision 235843) +++ libgomp/Makefile.am (working copy) @@ -10,7 +10,7 @@ config_path = @config_path@ search_path = $(addprefix $(top_srcdir)/config/, $(config_path)) $(top_srcdir) \ $(top_srcdir)/../include -fincludedir = $(libdir)/gcc/$(target_alias)/$(gcc_version)/finclude +fincludedir = $(libdir)/gcc/$(target_alias)/$(gcc_version)$(MULTISUBDIR)/finclude libsubincludedir = $(libdir)/gcc/$(target_alias)/$(gcc_version)/include vpath % $(strip $(search_path)) > The attached patch allows libgomp to install its Fortran modules in the > correct multilib-aware directories, just like libgfortran does. > Without it, multilib Fortran OpenMP code using the modules fails to compile > because the modules are not found: > > $ gfortran -fopenmp a.f90 > $ gfortran -fopenmp a.f90 -m32 > a.f90:1:6: > > use omp_lib > 1 > Fatal Error: Can't open module file ‘omp_lib.mod’ for reading at (1): No such > file or directory > compilation terminated. > > > > Bootstrapped and tested on x86_64-apple-darwin15. OK to commit? > > FX > > > > > > > 2016-05-03 Francois-Xavier Coudert > > PR libgomp/60670 > * Makefile.am: Make fincludedir multilib-aware. > * Makefile.in: Regenerate.
Re: [PATCH, Fortran] Allow warnings given through gfc_error to associate with warning flags
> 2016-12-27 Jakub Jelinek > > * gfortran.h (gfc_error): Rename overload with OPT argument to... > (gfc_error_opt): ... this. > * error.c (gfc_error): Rename overloads with OPT argument to... > (gfc_error_opt): ... this. Adjust callers. > (gfc_notify_std, gfc_error): Adjust callers. > * resolve.c (resolve_structure_cons, resolve_global_procedure): Use > gfc_error_opt instead of gfc_error. > * interface.c (argument_rank_mismatch, compare_parameter, > gfc_check_typebound_override): Likewise. Fix up formatting. OK. Thanks for taking care of that. FX
Re: [PATCH] PR 78534 Change character length from int to size_t
> The _size member is the size of the object; for polymorphic objects
> the size isn't known at compile time, so it is to be stored in the
> vtable (in principle, since the vtable tells the exact class of a
> polymorphic object, it should be possible to figure out the size
> without an explicit _size member, but I'll let the OOP experts handle
> that, if they want). There is another vtable member _len which is used
> for character lengths, and that is indeed of type
> gfc_charlen_type_node.
Understood, thanks.
> I think when the dust settles, I might make sense to get rid of
> gfc_charlen_type_node/gfc_charlen_type and just use
> size_type_node/size_t also for string lengths, but at least for now I
> think it's clearer to use separate names.
I agree with keeping them for now.
> So strictly speaking it's not necessary, as long as
> gfc_charlen_type_node is a signed integer.
>
> OTOH, if/when one wants to make gfc_charlen_type_node unsigned,
> perhaps some more far-reaching changes are needed and that work is not
> necessary anymore. Say, introducing BT_UNSIGNED_INTEGER (??) and
> teaching gfc_typespec to handle unsigned integers? Or maybe it's
> better to put a tree node specifying the type in the typespec and use
> that instead of the bt type + kind to tell what type it is?
>
> Do you want me to remove that for the time being?
Let’s remove that, at least for now.
>> There are other cases (resolve.c, simplify.c) where you introduce a
>> dependency on middle-end entities (tree.h, trans-types.h) in what are pure
>> Fortran front-end stages. This breaks the separation that currently exists,
>> and which I strongly think we should keep.
>
> These changes are similar to the above, i.e. a check that uses
> get_type_static_bounds() and works also if gfc_charlen_type_node is
> changed to be an unsigned type.
OK then let’s remove them too.
> - Should I remove the so-far preliminary work to handle
> gfc_charlen_type_node being unsigned?
I think it makes more sense.
> - Should I fix the uses of mpz_{get,set}_{s,u}i?
I think so, otherwise there is little reason to break the ABI and not support
long strings :)
>> - trans-types.h: why do we now need to include trans.h?
>
> IIRC this was due to some of the new .c files including trans-types.h
> but not trans.h and failing to compile. AFAIU the convention is that
> headers should include whatever is necessary to use said header. So
> this is some latent bug that has been exposed by my other changes.
If, with the final version of the patch, you can remove it, please do. And if
you don’t, please remove the trans.h includes from source files that already
include trans-types.h
FX
Re: [PATCH] Use the middle-end boolean_type_node
> The gfc_init_types change is an ABI change, at least if the fortran FE > bool type is ever stored in memory and accessed by multiple TUs, or > passed as argument etc. And the difference between the C/C++ _Bool/bool > and fortran FE bool has caused lots of issues in the past, so if it can be > the same type, it is preferrable. The patch committed doesn’t change the way Fortran LOGICAL types are emitted. The fact that the default LOGICAL kind is different (on most platforms) from C/C++ boolean is due to the standard, and not our own choice of ABI. As Jane says, boolean_type_node in the Fortran front-end is only used for intermediate values and temporaries; it is used every time we build a COND_EXPR. It is not part of the ABI. FX
Re: [PATCH] Use the middle-end boolean_type_node
> The regression with 8 bit boolean types surfaced with the z10 machine. The > ABI is much older. Since > we cannot change it anymore we should rather talk to the hardware guys to add > the instruction we > need. So for now we probably have to live with the regression in the Fortran > cases since as I > understand it your change fixes an actual problem. As far as I understand (and Jane will correct me if I am wrong), the patch does not fix anything in particular. The idea was that, by transitioning from having all boolean expressions from “int” to “bool” (in C terms), and thus from 32-bit to 8-bit on “typical” targets, the optimizer might be able to emit more compact code. I am not sure this was tested. So: maybe it is a case of "Profile. Don't speculate.” FX
Re: [PATCH] PR 78534 Change character length from int to size_t
> r244027 reverts r244011. Sorry for the breakage. It seems to affect > all i686 as well in addition to power, maybe all 32-bit hosts. The breakage is surprising, as the rejects-valid does not involve character length at all. Jane, any chance you might have accidentally committed some unrelated change along? FX
[libstdc++, patch] Fix build on APFS file system
Parallel builds of libstdc++ on APFS filesystem (with 1 ns granularity) on macOS 10.13 often fail (failure rate for “make -j2” to “make -j8” is about 60% from my own builds and results reported by others): https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81797 This is reproducible with several versions of GNU make. Changing libstdc++’s makefile to mark install-headers with .NOTPARALLEL fixes the issue. We've carried that patch in Homebrew (https://brew.sh) for a few months now, and have had no report of build issues since then. Bootstrapped and regtested on x86_64-apple-darwin17 (as well as other platforms). OK to commit? FX parallel.ChangeLog Description: Binary data parallel.diff Description: Binary data
Re: [libstdc++, patch] Fix build on APFS file system
> From supplied info not follow that problem is on gcc build side. > I can suspect that APFS has problem with direntry intensive > modification, for example. As part of Homebrew, we have built 4000+ open source codes on this new filesystem, with parallel compilation. Some of them pretty intensive (clang, llvm, rust, ghc, and a ton of compilers, libraries, databases, etc.). Many of them several times (for testing, etc.). Macports has done the same, probably other projects as well. We have not seen any evidence of a generic issue related to this filesystem. Apple is not aware of such an issue either, apparently. Yet, libstdc++ parallel builds have a very high failure rate. Other GCC libraries build fine, too. I do not know how to debug parallel makefiles, otherwise I would do it. I have asked help on the GNU Make mailing-list (http://lists.gnu.org/archive/html/bug-make/2017-08/msg00034.html), but didn’t get any. So I cannot prove it (and fix it), but there is overwhelming evidence that the problem is in libstdc++. FX
Re: [libstdc++, patch] Fix build on APFS file system
> Could you test using .PHONY: install-headers instead? > That target *is* phony, so telling make that seems sensible. I’ve tried adding it to the existing .PHONY list: Index: libstdc++-v3/include/Makefile.in === --- libstdc++-v3/include/Makefile.in(revision 253855) +++ libstdc++-v3/include/Makefile.in(working copy) @@ -1449,6 +1449,7 @@ uninstall-am: distclean-libtool dvi dvi-am html html-am info info-am install \ install-am install-data install-data-am install-data-local \ install-dvi install-dvi-am install-exec install-exec-am \ + install-freestanding-headers install-headers \ install-html install-html-am install-info install-info-am \ install-man install-pdf install-pdf-am install-ps \ install-ps-am install-strip installcheck installcheck-am \ But that doesn’t work: In file included from /Users/fx/devel/gcc/ibin/x86_64-apple-darwin17.0.0/i386/libstdc++-v3/include/bits/exception_ptr.h:39:0, from /Users/fx/devel/gcc/trunk/libstdc++-v3/libsupc++/exception:143, from /Users/fx/devel/gcc/ibin/x86_64-apple-darwin17.0.0/i386/libstdc++-v3/include/ios:39, from /Users/fx/devel/gcc/ibin/x86_64-apple-darwin17.0.0/i386/libstdc++-v3/include/istream:38, from /Users/fx/devel/gcc/ibin/x86_64-apple-darwin17.0.0/i386/libstdc++-v3/include/sstream:38, from /Users/fx/devel/gcc/ibin/x86_64-apple-darwin17.0.0/i386/libstdc++-v3/include/complex:45, from /Users/fx/devel/gcc/ibin/x86_64-apple-darwin17.0.0/i386/libstdc++-v3/include/ccomplex:39, from /Users/fx/devel/gcc/trunk/libstdc++-v3/include/precompiled/stdc++.h:52: /Users/fx/devel/gcc/trunk/libstdc++-v3/libsupc++/typeinfo:36:10: fatal error: bits/hash_bytes.h: No such file or directory #include ^~~
Re: [libstdc++, patch] Fix build on APFS file system
> So the issue is PCH generation (why's that re-generated at install time?). As suggested by Marc in the PR, I've removed the @ from include/Makefile.in, and removed the leading - for lines with LN_S. The result of "make -d --trace -j8 all-target-libstdc++-v3", in a build where x86_64-apple-darwin17.0.0/libstdc++-v3 was entirely removed, can be found here: https://gist.github.com/fxcoudert/b621465a794d968593bc7ed90c0fc1fb FX
Re: [libstdc++, patch] Fix build on APFS file system
> The patch seems like a rough bandaid to hide the real bug. Better to > identify the real bug. If there is a missing dependency, then I'd like to > think that adding the right dependency should resolve the issue. So far, apart from a suggestion from Marc, I haven’t received any help or advice in identifying or debugging the issue. FX
Re: [libstdc++, patch] Fix build on APFS file system
Thanks Jonathan. I tried, and it does not work, I still get the same error: Making all in include mkdir -p ./x86_64-apple-darwin17.0.0/bits/stdc++.h.gch mkdir -p ./x86_64-apple-darwin17.0.0/bits/stdc++.h.gch /Users/fx/devel/gcc/ibin/./gcc/xgcc -shared-libgcc -B/Users/fx/devel/gcc/ibin/./gcc -nostdinc++ -L/Users/fx/devel/gcc/ibin/x86_64-apple-darwin17.0.0/i386/libstdc++-v3/src -L/Users/fx/devel/gcc/ibin/x86_64-apple-darwin17.0.0/i386/libstdc++-v3/src/.libs -L/Users/fx/devel/gcc/ibin/x86_64-apple-darwin17.0.0/i386/libstdc++-v3/libsupc++/.libs -B/Users/fx/devel/gcc/irun/x86_64-apple-darwin17.0.0/bin/ -B/Users/fx/devel/gcc/irun/x86_64-apple-darwin17.0.0/lib/ -isystem /Users/fx/devel/gcc/irun/x86_64-apple-darwin17.0.0/include -isystem /Users/fx/devel/gcc/irun/x86_64-apple-darwin17.0.0/sys-include -m32 -x c++-header -nostdinc++ -g -O2 -m32 -I/Users/fx/devel/gcc/ibin/x86_64-apple-darwin17.0.0/i386/libstdc++-v3/include/x86_64-apple-darwin17.0.0 -I/Users/fx/devel/gcc/ibin/x86_64-apple-darwin17.0.0/i386/libstdc++-v3/include -I/Users/fx/devel/gcc/trunk/libstdc++-v3/libsupc++ -O2 -g -std=gnu++0x /Users/fx/devel/gcc/trunk/libstdc++-v3/include/precompiled/stdc++.h \ -o x86_64-apple-darwin17.0.0/bits/stdc++.h.gch/O2ggnu++0x.gch /Users/fx/devel/gcc/ibin/./gcc/xgcc -shared-libgcc -B/Users/fx/devel/gcc/ibin/./gcc -nostdinc++ -L/Users/fx/devel/gcc/ibin/x86_64-apple-darwin17.0.0/i386/libstdc++-v3/src -L/Users/fx/devel/gcc/ibin/x86_64-apple-darwin17.0.0/i386/libstdc++-v3/src/.libs -L/Users/fx/devel/gcc/ibin/x86_64-apple-darwin17.0.0/i386/libstdc++-v3/libsupc++/.libs -B/Users/fx/devel/gcc/irun/x86_64-apple-darwin17.0.0/bin/ -B/Users/fx/devel/gcc/irun/x86_64-apple-darwin17.0.0/lib/ -isystem /Users/fx/devel/gcc/irun/x86_64-apple-darwin17.0.0/include -isystem /Users/fx/devel/gcc/irun/x86_64-apple-darwin17.0.0/sys-include -m32 -x c++-header -nostdinc++ -g -O2 -m32 -I/Users/fx/devel/gcc/ibin/x86_64-apple-darwin17.0.0/i386/libstdc++-v3/include/x86_64-apple-darwin17.0.0 -I/Users/fx/devel/gcc/ibin/x86_64-apple-darwin17.0.0/i386/libstdc++-v3/include -I/Users/fx/devel/gcc/trunk/libstdc++-v3/libsupc++ -O2 -g /Users/fx/devel/gcc/trunk/libstdc++-v3/include/precompiled/stdc++.h -o x86_64-apple-darwin17.0.0/bits/stdc++.h.gch/O2g.gch /Users/fx/devel/gcc/trunk/libstdc++-v3/include/precompiled/stdc++.h:44:10: fatal error: cstdarg: No such file or directory #include ^ compilation terminated. make[7]: *** [x86_64-apple-darwin17.0.0/bits/stdc++.h.gch/O2ggnu++0x.gch] Error 1
Re: [libstdc++, patch] Fix build on APFS file system
The output of “make -j4” in a build at stage1, where I have just run "rm -rf x86_64-apple-darwin17.0.0/**/libstdc++-v3”, with the patch applied: https://gist.github.com/fxcoudert/46f0026c44eb3db2937ac0e92a477338 FX
Re: [libstdc++, patch] Fix build on APFS file system
> Presumably for the i386 and x86_64 multilibs (does it make any > difference if you do --disable-multlib? It would be easier to debug the > output if each command was only done once). Same problem without multilib. > I can't really do much more by email, somebody with access to one of > these failing systems needs to debug this. The problem has nothing to > do with C++ so I don't see why it needs to be solved by libstdc++ > people -- just modify the makefile and inspect the results. Thanks for the help so far. If I could debug this I would, but the complexity of this build machinery puts it out of my reach. FX
[build,darwin] Fix toplevel configure test for LTO on darwin
When LTO was introduced, on macOS it needed darwin9 to work. Over time, most tests for “*-apple-darwin9” in the toplevel configure were changed to also include later darwin versions: *-darwin[[912]]* However, the check for LTO was not updated. As far as I know, this is merely an oversight: LTO works fine on Darwin, and most vendors (homebrew, macports, etc.) have been shipping with --enable-lto for years. The attached patch makes it build LTO by default on darwin >= 9. I realize it’s late but I hope the very restricted nature (and ultra-low risk) of the patch make it a candidate for inclusion in trunk nonetheless. Bootstrapped and regtested on x86_64-apple-darwin16. OK to commit? FX lto_darwin.ChangeLog Description: Binary data lto_darwin.diff Description: Binary data
Re: [build,darwin] Fix toplevel configure test for LTO on darwin
*ping* When LTO was introduced, on macOS it needed darwin9 to work. Over time, most tests for “*-apple-darwin9” in the toplevel configure were changed to also include later darwin versions: *-darwin[[912]]* However, the check for LTO was not updated. As far as I know, this is merely an oversight: LTO works fine on Darwin, and most vendors (homebrew, macports, etc.) have been shipping with --enable-lto for years. The attached patch makes it build LTO by default on darwin >= 9. I realize it’s late but I hope the very restricted nature (and ultra-low risk) of the patch make it a candidate for inclusion in trunk nonetheless. Bootstrapped and regtested on x86_64-apple-darwin16. OK to commit? FX lto_darwin.ChangeLog Description: Binary data lto_darwin.diff Description: Binary data
Re: New prerequisites to support multi image COARRAY in gfortran
> We choose mpich as a default only because it is very stable. Why are why tying ourselves to one MPI implementation? FX
Re: [PATCH 1/4] libsanitizer: merge from upstream (c425db2eb558c263)
> I see that the fix was applied locally (and my bootstraps on various Darwin > versions worked OK), > but I’m not clear if you submitted a PR upstream (or just the bug report). > If the fix is to remain > local-only, it should be added to the list in LOCAL_PATCHES. Patch was submitted upstream: https://github.com/llvm/llvm-project/pull/72642 FX
[RFC, patch] Detect lack of 32-bit devel environment on x86_64-linux targets
Given that I did not receive any feedback on my earlier email on this topic, I
would like to send this patch for RFC. I'm not expert at this configury-stuff,
so please try to comment on both the test proposed and my actual implementation
:)
The idea is to find a patch which both catches probable issues early on for
most x86_64-linux users, yet does not make build more complex for our power
users. So, I propose to include a specific check in toplevel configure:
The cumulative conditions I suggest, in order to make it as unobtrusive as
possible for current users, are:
1. if we build a native compiler,
2. on x86_64-linux (and possible other x86_64 targets whose maintainers want
to opt in),
3. and neither --enable-multilib nor --disable-multilib were passed
then:
a. we check that the native compiler can handle 32-bit, by compiling a test
executable with the "-m32" option
b. if we fail, we error out of the configure process, indicating that this can
be overriden with --{enable,disable}-multilib
I suspect this might catch (at configure time) the large majority of users who
currently get stuck at stage 2 with the "gnu/stubs-32.h" error, while being
invisible to a large majority of the power users.
So, what do you think?
FX
Index: configure.ac
===
--- configure.ac(revision 201292)
+++ configure.ac(working copy)
@@ -2861,6 +2861,26 @@ case "${target}" in
;;
esac
+# Special user-friendly check for native x86_64-linux build, if
+# multilib is not explicitly enabled.
+case "$target:$have_compiler:$host:$target:$enable_multilib" in
+ x86_64-*linux*:yes:$build:$build:)
+# Make sure we have a developement environment that handles 32-bit
+dev64=no
+echo "int main () { return 0; }" > conftest.c
+${CC} -m32 -o conftest ${CFLAGS} ${CPPFLAGS} ${LDFLAGS} conftest.c
+if test $? = 0 ; then
+ if test -s conftest || test -s conftest.exe ; then
+ dev64=yes
+ fi
+fi
+rm -f conftest*
+if test x${dev64} != xyes ; then
+ AC_MSG_ERROR([I suspect your system does not have 32-bit developement
libraries (libc and headers). If you have them, rerun configure with
--enable-multilib. If you do not have them, and want to build a 64-bit-only
compiler, rerun configure with --disable-multilib.])
+fi
+;;
+esac
+
# Default to --enable-multilib.
if test x${enable_multilib} = x ; then
target_configargs="--enable-multilib ${target_configargs}"
Fwd: [RFC, patch] Detect lack of 32-bit devel environment on x86_64-linux targets
ping
> Given that I did not receive any feedback on my earlier email on this topic,
> I would like to send this patch for RFC. I'm not expert at this
> configury-stuff, so please try to comment on both the test proposed and my
> actual implementation :)
>
> The idea is to find a patch which both catches probable issues early on for
> most x86_64-linux users, yet does not make build more complex for our power
> users. So, I propose to include a specific check in toplevel configure:
>
> The cumulative conditions I suggest, in order to make it as unobtrusive as
> possible for current users, are:
>
> 1. if we build a native compiler,
> 2. on x86_64-linux (and possible other x86_64 targets whose maintainers want
> to opt in),
> 3. and neither --enable-multilib nor --disable-multilib were passed
>
> then:
>
> a. we check that the native compiler can handle 32-bit, by compiling a test
> executable with the "-m32" option
> b. if we fail, we error out of the configure process, indicating that this
> can be overriden with --{enable,disable}-multilib
>
> I suspect this might catch (at configure time) the large majority of users
> who currently get stuck at stage 2 with the "gnu/stubs-32.h" error, while
> being invisible to a large majority of the power users.
>
> So, what do you think?
>
> FX
>
Index: configure.ac
===
--- configure.ac(revision 201292)
+++ configure.ac(working copy)
@@ -2861,6 +2861,26 @@ case "${target}" in
;;
esac
+# Special user-friendly check for native x86_64-linux build, if
+# multilib is not explicitly enabled.
+case "$target:$have_compiler:$host:$target:$enable_multilib" in
+ x86_64-*linux*:yes:$build:$build:)
+# Make sure we have a developement environment that handles 32-bit
+dev64=no
+echo "int main () { return 0; }" > conftest.c
+${CC} -m32 -o conftest ${CFLAGS} ${CPPFLAGS} ${LDFLAGS} conftest.c
+if test $? = 0 ; then
+ if test -s conftest || test -s conftest.exe ; then
+ dev64=yes
+ fi
+fi
+rm -f conftest*
+if test x${dev64} != xyes ; then
+ AC_MSG_ERROR([I suspect your system does not have 32-bit developement
libraries (libc and headers). If you have them, rerun configure with
--enable-multilib. If you do not have them, and want to build a 64-bit-only
compiler, rerun configure with --disable-multilib.])
+fi
+;;
+esac
+
# Default to --enable-multilib.
if test x${enable_multilib} = x ; then
target_configargs="--enable-multilib ${target_configargs}"
[fortran,doc] Fix typo in doc
I think the doc says “assumed-shape” where it means “assumed-rank”. Is that OK?
FX
Index: gfortran.texi
===
--- gfortran.texi (revision 204389)
+++ gfortran.texi (working copy)
@@ -2624,7 +2624,7 @@ other hand, assumed-type assumed-rank du
(@code{TYPE(*), DIMENSION(..)}) allow for both scalars and arrays, but
require special code on the callee side to handle the array descriptor.
-@item Assumed-shape arrays (@code{DIMENSION(..)}) as dummy argument
+@item Assumed-rank arrays (@code{DIMENSION(..)}) as dummy argument
allow that scalars and arrays of any rank can be passed as actual
argument. As the Technical Specification does not provide for direct
means to operate with them, they have to be used either from the C side2013-11-05 Francois-Xavier Coudert
* gfortran.texi: Fix typo.
[patch,libgfortran] Fix binary128 ERFC_SCALED
This patch fixes libgfortran’s binary128 [aka real(kind=16)] variant of ERFC_SCALED. The original code, which I had lifted from netlib, gives only 18 significant decimal digits, which is not enough for binary128 (33 decimal digits). I thus implemented a new variant for binary128. For arguments < 12, it simply calls erfcq() then multiplies by expq(x*x). For larger arguments, it uses a power expansion in 1/x. The new implementation provides answers within to 2 ulp of the correct value. Regtested on x86_64-apple-darwin13, comes with a testcase. OK to commit? FX erfc_scaled.ChangeLog Description: Binary data erfc_scaled.diff Description: Binary data
[libgfortran,patch] Silence a warning
This attach patch adds an assert() in the library to fix PR 51828, i.e. silence a “may be used uninitialized” warning. Built and regtested on x86_64-apple-darwin13. OK to commit? FX libwarning.ChangeLog Description: Binary data libwarning.diff Description: Binary data
Re: [patch,libgfortran] Fix binary128 ERFC_SCALED
> There is a missed optimization in
>
> + if (x < 12)
> +{
> + /* Compute directly as ERFC_SCALED(x) = ERFC(x) * EXP(X**2).
> +This is not perfect, but much better than netlib. */
> + return erfcq(x) * expq(x*x);
> +}
>
> If x is less than approximately -8192, then erfc(x)*exp(x*x)
> overflows.
Hum, I get roughly -106 where you have -8192, so I’ll not commit immediately
with the value I have, and let you check it first.
OK to commit?
FX
erfc_scaled.diff
Description: Binary data
Re: [patch,libgfortran] Fix binary128 ERFC_SCALED
> So, yeah, you're correct. My suggestion was based on the not > so careful mistake of replacing x*x by x+x and dropping log(2). > That is, I had x+x = -emax --> x = - emax / 2. Committed as rev. 205151, thanks for the review! FX
Re: [patch,libgfortran] Fix binary128 ERFC_SCALED
> ../../../libgfortran/intrinsics/erfc_scaled.c:59:1: error: unknown type name > ‘GFC_REAL_16' I’m really sorry about that, I should have tested on a system without binary128, that would have caught it. Attached patch committed as rev. 205193 after checking it on system both with and without binary128. Sorry again, FX 2013-11-20 Francois-Xavier Coudert PR libfortran/59227 * intrinsics/erfc_scaled.c (erfc_scaled_r16): Don't define if __float128 is not available. fix.diff Description: Binary data
Re: [fortran, patch] Add Fortran 2003 IEEE intrinsic modules
> That's a reasonable decision, but it is actually used at least ten times > as much as everything else put together, possibly a hundred times as much. I believe we are in pretty different parts of the community. Around me I rarely see it used, while people check for nans, infinities, and exception flags often. Also, aborting on certain floating-point exceptions is widely used as a debugging aid. > However, it is used in the form of selecting hard underflow using a > compilation option, and not within the program. You certainly DO have > targets where it would work, even dynamically within the program, and I > think that it could be done even on x86. That isn't the same as it > should be done, of course! Indeed, 387/SSE has flush-to-zero modes. But other APIs do not (glibc, SysV, AIX). I’m perfectly willing to add it, especially to 387/SSE, if given a bit of help (someone to write the assembly code). Thanks for your feedback, FX
Re: [fortran, patch] Add Fortran 2003 IEEE intrinsic modules
>> --- gcc/testsuite/lib/target-supports.exp(revision 205019)
>> +++ gcc/testsuite/lib/target-supports.exp(working copy)
>> proc add_options_for_ieee { flags } {
> ...
>> +set extra "-fno-unsafe-math-optimizations -frounding-math
>> -fsignaling-nans -fintrinsic-modules-path $specpath/libgfortran/"
>
> That part looks wrong: I think you do not want to add -fintrinsic-modules-path
> for all IEEE functions, e.g. C and C++ compilers do not handle that option,
> nor does the Ada compiler.
Hum. That’s unfortunate, because I haven’t found any other suitable place :)
I do not see how to specify compiler flags only for Fortran.
> You could also ask Mike Stump to review the testsuite changes.
Mike, in your understanding, is there any place where Fortran-only flags could
be specified in the testsuite?
FX
Re: [fortran, patch] Add Fortran 2003 IEEE intrinsic modules
Hi Uros!
Thanks for lookin at this. I am a real newcomer to 387, and it took me a long
time perusing the Intel doc, as well as glibc sources, to come up with that.
The “reference” implementation of these FPU functions, the one I am confident
in, is that in config/fpu-glibc.h: i.e., the functions in config/fpu-i387.h
should have the same effect that config/fpu-glibc.h on i386/x86_64 hardware.
I’ll reply to your comments, but in some cases I was not sure exactly what you
were saying… thanks for your help, and patience!
> @@ -136,16 +165,54 @@ set_fpu (void)
> __asm__ __volatile__ ("%vstmxcsr\t%0" : "=m" (cw_sse));
>
> /* The SSE exception masks are shifted by 7 bits. */
> - cw_sse |= _FPU_MASK_ALL << 7;
> - cw_sse &= ~(excepts << 7);
> -
> - /* Clear stalled exception flags. */
> - cw_sse &= ~_FPU_EX_ALL;
>
> You have to clear stalled SSE exceptions here. Their flags are in LSB
> bits, so their position is different than the position of exception
> mask bits in the control word.
So, if I get you right, I should restore the "cw_sse &= ~_FPU_EX_ALL”, which I
had mistakenly removed.
But I’m looking at glibc-2.18/sysdeps/x86_64/fpu/feenablxcpt.c and
fedisblxcpt.c, and it doesn’t seem to be done there.
> + __asm__ __volatile__ ("fnstenv\t%0" : "=m" (*&temp));
> [...]
> + __asm__ __volatile__ ("fldenv\t%0" : : "m" (*&temp));
>
> Why do you need "*&" here?
I don’t.
> fldenv will also trigger exceptions with set flags on the next x87 FP insn ...
>
> +__asm__ __volatile__ ("%vstmxcsr\t%0" : "=m" (cw_sse));
> +
> +cw_sse &= ~exc_clr;
> +cw_sse |= exc_set;
> +
> +__asm__ __volatile__ ("%vldmxcsr\t%0" : : "m" (cw_sse));
>
> ... and ldmxcsr won't trigger exceptions, neither with SSE insn.
> Please see Intel documentation on FP exceptions.
This code should be equivalent to glibc’s:
feclearexcept (exc_clr);
feraiseexcept (exc_set);
So yes, raising an exception is the except action. I’ve attached a new version
of config/fpu-387.h, along with the glibc version (fpu-glibc.h). I’d be glad if
you could review it.
Thanks a lot!
FX
fpu-387.h
Description: Binary data
fpu-glibc.h
Description: Binary data
[fortran,patch] Remove unused gfc_open_intrinsic_module()
>> A quick grep through the front-end indicates that >> gfc_open_intrinsic_module() is never used. Should it have been >> removed when module file reading/writing was overhauled? > > I suspect the answer is "yes”. Here’s a patch that removes it. It buils fine. OK to commit? FX a.ChangeLog Description: Binary data a.diff Description: Binary data
Re: [fortran,patch] Remove unused gfc_open_intrinsic_module()
> OK. Thanks for the patch! Committed as rev. 205335.
Re: [PATCH, libgfortran]: Fix PR59313, gfortran.dg/erf_3.F90 FAILs
> Currently, gfortran.dg/erf_3.F90 FAILs on targets with 128bit > (quadruple) long double, since high-precision erfc_scaled_r16 gets > defined only for __float128 quadruple precision. I can’t approve it, but yes, it makes more sense than what I did earlier. FX
Re: fatal error: gnu/stubs-32.h: No such file
> As a consensual first step toward addressing this issue, I suggest the
> following patch to the doc. I hope it is clear enough, but suggestions are
> obviously welcome. (I haven't even compiled the docs with it, as I'm on my
> laptop with little battery.)
Given that I received no feedback, I'd like to submit this doc patch formally.
Tested by doing "make info html pdf".
OK to commit to trunk? What about other active release branches?
FX
Index: install.texi
===
--- install.texi(revision 201292)
+++ install.texi(working copy)
@@ -255,6 +255,26 @@ may need to use @option{--disable-stage1
bootstrapping the compiler with such earlier compilers is strongly
discouraged.
+@item C standard library and headers
+
+In order to build GCC, the C standard library and headers must be present
+for all target variants for which target libraries will be built (and not
+only the variant of the host C++ compiler).
+
+This affects the popular @samp{x86_64-unknown-linux-gnu} platform (among
+other multilib targets), for which 64-bit (@samp{x86_64}) and 32-bit
+(@samp{i386}) libc headers are usually packaged separately. If you do a
+build of a native compiler on @samp{x86_64-unknown-linux-gnu}, beware of
+either:
+
+@itemize @bullet
+@item having 32-bit libc developer package properly installed (the exact
+name of the package depends on your distro); otherwise, you may encounter an
+error such as @samp{fatal error: gnu/stubs-32.h: No such file}
+@item building GCC as a 64-bit only compiler, by configuring with the
+option @option{--disable-multilib}
+@end itemize
+
@item GNAT
In order to build the Ada compiler (GNAT) you must already have GNAT
Re: libsanitizer merge from upstream r196090
> > Well, it regresses against 4.8, so it still is a P1 regression. > > Does anyone care? Well, you’re one of the maintainers of libsanitizer for GCC, so if you do not care about regressions in your code, it makes little sense for GCC (the whole project) to keep libsanitizer. I’ve posted this regression a month ago, it was not addressed. I’m not sure under what specific arrangement libsanitizer was added to GCC, but in general there is a responsibility of maintainers not to break bootstrap in their code. Yes, it’s a cost, and if you are not willing to do it, why did you contribute in the first place? Or is it a “hit and run” approach to maintainership? FX
Re: libsanitizer merge from upstream r196090
> I believe this is a case where the GCC project gets more benefit from > libsanitizer than libsanitizer gets from being part of the GCC > project. We should work with the libsanitizer developers to make this > work, not just push everything back on them. You’re vastly better qualified than me to make this assessment, of course. My point is: unless someone (or multiple someones) is actually responsible for the thing, it cannot just work out of a sense of “someone should really do something about it”. The merge model of “we can break any target, except the single one we’re testing, every time we merge” seems poised for failure. FX
Re: libsanitizer merge from upstream r196090
> I think libsanitizer should be disabled automatically if kernel or glibc are > too old. I think pretty much everyone agrees. But noone’s willing to put forward a patch, and so far the libsanitizer maintainers have failed to even document the requirements. (There are also binutils requirements, as I learnt the hard way.) FX
Re: [RFC, patch] Detect lack of 32-bit devel environment on x86_64-linux targets
> I'm not sure why this should be different for x86_64 compared to all > other bi-arch toolchains? It’s not, but it’s a particularly common one and has been reported multiple times here and on gcc-help. If we can help these users early, we spare ourselves the time to reply to such reports. (Also, documentation and this patch are not exclusive: in fact, I have also submitted a doc patch to make things clearer.) > I think the right place for this is a "Non-bugs" section in the > installation manual. Look at this as a diagnostics bug: our current diagnostics for this pretty common situation sucks. It comes late in the compilation, and the message itself isn’t helpful. FX
Re: fatal error: gnu/stubs-32.h: No such file
> Were you waiting for further approval? If so: okay with the change > proposed by Andrew. Thanks, committed as rev. 205802 with Andrew’s change. FX
Re: [RFC, patch] Detect lack of 32-bit devel environment on x86_64-linux targets
> The patch is okay, but if other architecture maintainers could add > similar checks for their ports (SPARC and PPC, I guess), it would be nice. Thanks, committed as rev. 205975 Adding other systems to the list of checks will be easy, once the maintainers confirm that they want to opt in into it. FX
Re: [PATCH] jit: fix link on OS X and Solaris (PR jit/64089 and PR jit/84288)
> Does this fix the jit linker issues on OS X and Solaris? The patch fails to bootstrap on x86_64-apple-darwin17. gcc/config.log says: gcc_cv_ld_version_script=no ld_version_script_option='--version-script’ gcc/Makefile says: LD_VERSION_SCRIPT_OPTION = --version-script LD_SONAME_OPTION = -install_name which makes it try to link with: -Wl,--version-script,../../trunk/gcc/jit/libgccjit.map \ -Wl,-install_name,libgccjit.so.0 which fails with: ld: unknown option: --version-script I think the patch to gcc/configure.ac should be: +AC_MSG_CHECKING(linker --version-script option) +gcc_cv_ld_version_script=no +ld_version_script_option='' FX
Re: [PATCH] jit: fix link on OS X and Solaris (PR jit/64089 and PR jit/84288)
I can confirm that, with the attached revised patch, a bootstrap with --enable-languages=c,c++,jit --enable-host-shared is successful on macOS. FX patch Description: Binary data
Re: [PATCH] jit: fix link on OS X and Solaris (PR jit/64089 and PR jit/84288)
Hi David, Any news on that patch? Cheers, FX
Re: [PATCH] Testsuite, DWARF2: adjust regexp to match darwin output
ping**2 > Hi, > > This was a painful one to fix, because I hate regexps, especially when they > are quoted. On darwin, we have this failure: > >FAIL: gcc.dg/debug/dwarf2/inline4.c scan-assembler > DW_TAG_inlined_subroutine[^(]*([^)]*)[^(]*(DIE > (0x[0-9a-f]*) DW_TAG_formal_parameter[^(]*(DIE > (0x[0-9a-f]*) DW_TAG_variable > > That hideous regexp is trying to match (generated on Linux): > >>.uleb128 0x4# (DIE (0x5c) DW_TAG_inlined_subroutine) >>.long 0xa0# DW_AT_abstract_origin >>.quad .LBI4 # DW_AT_entry_pc >>.byte .LVU2 # DW_AT_GNU_entry_view >>.quad .LBB4 # DW_AT_low_pc >>.quad .LBE4-.LBB4 # DW_AT_high_pc >>.byte 0x1 # DW_AT_call_file (u.c) >>.byte 0xf # DW_AT_call_line >>.byte 0x14# DW_AT_call_column >>.uleb128 0x5# (DIE (0x7d) DW_TAG_formal_parameter) >>.long 0xad# DW_AT_abstract_origin >>.long .LLST0 # DW_AT_location >>.long .LVUS0 # DW_AT_GNU_locviews >>.uleb128 0x6# (DIE (0x8a) DW_TAG_variable) > > It is using the parentheses to check what is between > DW_TAG_inlined_subroutine, DW_TAG_formal_parameter and DW_TAG_variable. > There’s only one block of parentheses in the middle, that "(u.c)”. However, > on darwin, the generated output is more compact: > >>.uleb128 0x4; (DIE (0x188) DW_TAG_inlined_subroutine) >>.long 0x1b8 ; DW_AT_abstract_origin >>.quad LBB4; DW_AT_low_pc >>.quad LBE4; DW_AT_high_pc >>.uleb128 0x5; (DIE (0x19d) DW_TAG_formal_parameter) >>.long 0x1c6 ; DW_AT_abstract_origin >>.uleb128 0x6; (DIE (0x1a2) DW_TAG_variable) > > I think that’s valid as well, and the test should pass (what the test really > wants to check is that there is no DW_TAG_lexical_block emitted there, see > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=37801 for its origin). It could > be achieved in two ways: > > 1. making darwin emit the DW_AT_call_file > 2. adjusting the regexp to match, making the internal block of parentheses > optional > > I chose the second approach. It makes the test pass on darwin. If someone can > test it on linux, it’d be appreciated :) I don’t have ready access to such a > system right now. > > Once that passes, OK to commit? > FX 0001-Testsuite-DWARF2-adjust-regexp-to-match-darwin-outpu.patch Description: Binary data
Re: [PATCH] Libquadmath: update doc for some constants
kind ping Give it’s a doc patch, I think it might fall under the obvious rule, and will commit in a week if there is no objection. FX > As reported by Peter Randall, the description of three constants in > libquadmath is wrong. Attached patch fixes them. > > OK to push? > > FX > > > libquadmath/ChangeLog: > > * libquadmath.texi (M_LOG2Eq, M_LOG10Eq, M_2_PIq): Fix > description of these constants. > > 0001-Libquadmath-update-doc-for-some-constants.patch Description: Binary data
Re: [PATCH] Libquadmath: update doc for some constants
> I agree. I don't have a preference for which is better, but being consistent > with other documentation might be a winning argument. Pushed as attached. FX 0001-Libquadmath-update-doc-for-some-constants.patch Description: Binary data
Re: Darwin: Replace environment runpath with embedded [PR88590]
Thanks a lot Alexandre for the review! FX
[PATCH] Testsuite, Darwin: skip PIE test
Hi, The recent commit of https://gcc.gnu.org/pipermail/gcc-patches/2023-October/634347.html has made this test invalid. We now don’t emit __PIE__, and the test should be skipped on darwin. Fixes the new failure on x86_64-apple-darwin21. OK to push? FX 0001-Testsuite-Darwin-skip-PIE-test.patch Description: Binary data
[PATCH] Testsuite, i386: Fix test by passing -march
Hi, The newly introduced test gcc.target/i386/pr111698.c currently fails on Darwin, where the default arch is core2. Andrew suggested in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112287 to pass a recent value to -march, and I can confirm that it fixes the testsuite failure on x86_64-apple-darwin21. OK to push? FX 0001-Testsuite-i386-Fix-test-by-passing-march.patch Description: Binary data
[PATCH] Testsuite, Darwin: Fix trampoline warning
Heap-based trampolines are enabled on darwin20 and later, meaning that no warning is emitted. Fixes the test failure on x86_64-apple-darwin21 OK to push? FX 0001-Testsuite-Darwin-Fix-trampoline-warning.patch Description: Binary data
[PATCH] Testsuite, i386: Mark test as requiring dfp
Hi, The test is currently failing on x86_64-apple-darwin with "decimal floating-point not supported for this target”. Marking the test as requiring dfp fixes the issue. OK to push? FX 0001-Testsuite-i386-Mark-test-as-requiring-dfp.patch Description: Binary data
[PATCH] Testsuite, i386: Mark test as requiring ifunc
Hi, The test is currently failing on x86_64-apple-darwin. Marking the test as requiring ifunc fixes the issue. OK to push? FX 0001-Testsuite-i386-Mark-test-as-requiring-ifunc.patch Description: Binary data
Re: Darwin: Replace environment runpath with embedded [PR88590]
Hi, > +enable_darwin_at_rpath_$1=no I actually don’t understand why this one would have $1 in the name, unlike all other regenerated configure files. What value do we expect for $1 at this point in the file? That’s just plain weird. FX
Re: [PATCH] Testsuite, i386: Fix test by passing -march
> Well It can fail on x86_64-linux-gnu too if GCC was configured with > --with-arch=core2 for an example. > So having it, in this case, not being darwin specific would be > beneficial for all x86_64/i?86 targets. I pushed it as-is, meaning it will indeed apply to all x86_64/i?86 targets. FX
[PATCH] Darwin, testsuite: -bind_at_load is deprecated
With Xcode 15, gcc.dg/darwin-ld-2.c fails due to a warning: ld: warning: -bind_at_load is deprecated on macOS The patches ignores the warning when present. OK to push? FX 0001-Darwin-testsuite-bind_at_load-is-deprecated.patch Description: Binary data
[PATCH] Darwin, testsuite: -multiply_defined is obsolete
With Xcode 15, gcc.dg/ssp-2.c fails due to a warning: -multiply_defined is obsolete The patches ignores the warning when present. OK to push? FX 0001-Darwin-testsuite-multiply_defined-is-obsolete.patch Description: Binary data
