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
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}"
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: 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] Fix-up for PR fortran/66724 and fortran/66725
> 2015-07-16 Steven G. Kargl > > * io.c (is_char_type): Call gfc_resolve_expr(). > (match_open_element, match_dt_element, match_inquire_element): Fix > ASYNCHRONOUS case. OK to commit
Re: [PATCH, libgfortran]: Avoid left shift of negative value warning
> 2015-07-29 Uros Bizjak > >PR libgfortran/66650 >* libgfortran.h (GFC_DTYPE_SIZE_MASK): Rewrite to avoid >"left shift of negative value" warning. OK. Thanks for the patch. FX
Re: [Bug fortran/52846] [F2008] Support submodules - part 3/3
> Why do you initialize a static variable to false? You mean because false is equal to zero and it will be the default initialization anyway? I quite like that the default value is explicit. FX
Re: Patch for fortran/62536 and fortran/66175
> This patch cleans up nested blocks when there's an unexpected end of a > compilation unit (66175), and it handles cleaned-up blocks gracefully > (62536). I've run "make check-fortran" with the attached test cases. The patch is OK. But the question I missed in my earlier email was: do you have a copyright assignment in place with the FSF? FX
Re: Patch for fortran/62536 and fortran/66175
> No, I don't, but I would be happy to assign copyright. How do I do that? I've > read this: https://www.fsf.org/licensing/assigning.html > and it says I should "email the maintainer of the program communicating [my] > desire to assign copyright.” As I understand it, you need to fill this form: http://git.savannah.gnu.org/cgit/gnulib.git/plain/doc/Copyright/request-assign.future (with “GCC” being the “package you’re contributing to”) and send it by email to ass...@gnu.org to get the process started. In past times, it used to take a little time. I don’t know what the current situation is. FX
[libquadmath, patch] Add logbq() to libquadmath
The attached patch adds logbq() to libquadmath, with code lifted from glibc. It is made necessary by an upcoming patch for gfortran, adding full support for the IEEE modules on __float128, and requires logbq(). Bootstrapped and regtested on x86_64-apple-darwin14. OK to commit to trunk? FX logbq.ChangeLog Description: Binary data logbq.diff Description: Binary data
[fortran,patch] Extend IEEE support to all real kinds
The attached patch extends the IEEE modules to all floating-point kinds. Last time, when I added IEEE support, I restricted it to the float and double types (real kinds 4 and 8), to be extra safe. After discussion with Uros Bizjak and some reading, I’ve come to the conclusion that on most hardware where we support IEEE at all, we do support enough IEEE features on extended and quad prec (long double and __float128, on x86_64 hardware) to satisfy the Fortran standard. So, this enables full IEEE support for all real kinds. Nothing changes to the underlying architecture, it’s almost exclusively mechanical changes (adding the necessary variants to the interfaces, etc.). Bootstrapped and regtested on x86_64-apple-darwin14 (with associated libquadmath patch: https://gcc.gnu.org/ml/gcc-patches/2015-08/msg00124.html). OK to commit to trunk? FX PS: Once this is in, I intend to focus on the next item: allowing all standard-mandated IEEE functions in constant expressions. Then, I believe our IEEE support will be entirely bug-free (in the sense that there will be no known bugs in it!). ieee.ChangeLog Description: Binary data ieee.diff Description: Binary data
Re: [fortran,patch] Extend IEEE support to all real kinds
> FAIL: gfortran.dg/ieee/large_1.f90 -O0 (test for excess errors) > Excess errors: > large_1.f90:(.text+0x1792): undefined reference to `logbq’ Fixed by the patch there: https://gcc.gnu.org/ml/gcc-patches/2015-08/msg00124.html Waiting for review. FX
Re: [libquadmath, patch] Add logbq() to libquadmath
> AFAICT there is something missing in the patch: I do not see any compilation > of math/logbq.c and indeed no trace of logbq in libquadmath. What I am > missing? Maybe you didn’t regenerate the Makefile.in? The patch was sent without this regenerated file, as is (as I understand) the custom on gcc-patches. Attached is the full diff, including Makefile.in. FX x.diff Description: Binary data
Re: [libquadmath, patch] Add logbq() to libquadmath
> With the updated patch the test gfortran.dg/ieee/large_1.f90 compiles, but > fails at run time due to the lines > > if (.not. ieee_support_underflow_control(x1)) call abort > > and > > if (.not. ieee_support_underflow_control(x2)) call abort > > IIRC Uros said that underflow id not supported for __float128. Indeed. I fixed the code, but forgot to commit the testcase. Now done as revision 226665. FX
Re: [fortran,patch] Extend IEEE support to all real kinds
> Can you please also introduce tests for ieee exceptions for long > double and __float128 types (as is done for real and double in > gfortran.dg/ieee/ieee_1.F90) as well as test for supported rounding > modes as done for real and double in gfortran.dg/ieee/rounding_1.f90 ? > > On x86_64, these new tests will exercise exceptions and rounding modes > for __float128 soft-fp library, in addition to x87 long double tests. I’ve now introduced the tests for rounding (large_2.f90) and exceptions (large_3.F90): Adding gfortran.dg/ieee/large_2.f90 Adding gfortran.dg/ieee/large_3.F90 Transmitting file data ... Committed revision 226670.
[fortran,patch] Allow some IEEE functions in constant and specification expressions
The attached patch fixes the only remaining IEEE bug (to my knowledge) in gfortran. A Fortran 2008 interpretation request was submitted two years ago about which of the IEEE functions were supposed to be used in constant and specification expressions. The interp, voted on by J3 in Nov 2014, made corrections to the standard (see my comment at https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64104#c3 for the detail of this). This front-end patch makes gfortran conforming with the standard. There is a minor caveat: since support for various kinds (results of the IEEE_SELECTED_REAL_KIND, IEEE_SUPPORT_ROUNDING, IEEE_SUPPORT_FLAG and IEEE_SUPPORT_HALTING functions) are detected at runtime, we need in the future a mechanism to communication the support detected in libgfortran back to the front-end. The current approach does not do that, but instead assumes that if IEEE modules are present, support is enabled for all flags and rounding modes. This is true on the common platforms (x86 and x86_64), so I guess it’s good enough to enable it now. In the meantime, I’m thinking about how best to achieve the long-term goal (spec file? secret logical constants in the IEEE modules?) for the future. Bootstrapped and regtested on x86_64-apple-darwin14. OK to commit to trunk? FX ieee.ChangeLog Description: Binary data ieee.diff Description: Binary data
Re: PR pretty-print/67567 do not pass NULL as a string
> PING: https://gcc.gnu.org/ml/gcc-patches/2015-09/msg01219.html Given that this comes from submodules, which were recently introduced by Paul, I hoped he could comment. Paul? FX
Re: PR pretty-print/67567 do not pass NULL as a string
> If you can fix the Fortran part, then that would be nice, but it > should not hold up my patch since my patch changes nothing in the > output of Fortran. It just allows catching this type of errors when > checking is enabled. The patch includes a Fortran part, and if we can get it fixed when this is still fresh (submodules were just implemented few weeks ago), then it’s better! If Paul doesn’t reply in 2 days, then OK to commit. FX
Re: [PATCH, libgfortran] Fix FIND_FILE decls and use.
Dear Kirill, > When libgfortran is configured w/ HAVE_WORKING_STAT undefined > *and* current system is not MinGW - FIND_FILE_[DECL|ARGS} still > trying to use Windows's handles (id). Well, if HAVE_WORKING_STAT is not defined, then it means some other mechanism has to be used. If your target is not mingw32 and stat() is not reliable, we’ll need to provide an alternate way of handling things. Your patch would default to the unusable stat(). What’s your target? FX
Re: [PATCH] PR fortran/67615 -- check for scalar expression
> 2015-09-21 Steven G. Kargl > > PR fortran/67615 > * resolve.c (gfc_resolve_code): Check for scalar expression in > arithmetic-if. > > > 2015-09-21 Steven G. Kargl > > PR fortran/67615 > * gfortran.dg/pr67615.f90: new test. OK
Re: [PATCH, fortran] Revival of AUTOMATIC patch
> I think I appreciate what you are trying to do here. I don't intend to sound > negative here, but if the keyword AUTOMATIC does nothing The testcase given is not an example of useful AUTOMATIC. I think it is meant to be used to oppose an implied SAVE attribute, e.g. a variable with explicit initialization or the BIND attribute. Indeed, in the case of implied SAVE by initialization, there it is a little bit more work because you have to move the initialization to the executable part of the code. But that’s not impossible. All in all I’m skeptical of adding even more old language extensions with little demand when we have a hard time filling up gaps in the standard. Each addition adds to maintainance load, especially as they might not interact too well with more modern features. (For example coarrays or BIND attribute, which were not around when AUTOMATIC was in use.) I don’t find any request for this feature in the whole bugzilla database. FX
Re: [PATCH, fortran] Revival of AUTOMATIC patch
>> All in all I’m skeptical of adding even more old language extensions with >> little demand when we have a hard time filling up gaps in the standard. Each >> addition adds to maintainance load, especially as they might not interact >> too well with more modern features. (For example coarrays or BIND attribute, >> which were not around when AUTOMATIC was in use.) >> >> I don’t find any request for this feature in the whole bugzilla database. > > That's understandable. We'll maintain this feature in our own delta. I felt > it > was in the spirit of open source to offer it in case it was useful. > > Thanks for taking the time to review it. I definitely appreciate your contribution! And because it is now archived in the mailing-list archives, if people are interested in the future they can definitely pick it up. It is a rather “standalone” patch, I don’t think it would bitrot fast. But maybe other developers feel differently about it, in which case we’ll have a more “technical” review (mostly of the testcases needed, I think). Thanks again, FX
Re: Add checkpoint to libgomp dg-shouldfail tests
> Hi! > Ping. OK for the Fortran part, though I suspect you need Jakub to approve it as well. FX
Re: [PATCH] fortran/67802 -- Numeric constant character length must ...
> 2015-10-01 Steven G. Kargl > > PR fortran/67802 > * decl.c (add_init_expr_to_sym): Numeric constant for character > length must be an INTEGER. > > 2015-10-01 Steven G. Kargl > > PR fortran/67802 > * gfortran.dg/pr67802.f90: New test. OK, but not with that error message. We currently don’t use the “shorthand” standard notation (like "scalar-int-expr”) in our error messages, and we should keep that consistent. So I would go with “character length should be of integer type” or something like that. FX
Re: [PATCH] fortran/67802 -- Numeric constant character length must ...
> Well, ahem, gfortran does have several error messages that use the > standard notation. I know. I wrote some of them. :-) > > I'll simply change it to "Expecting an INTEGER at %L” Thanks. I have no objections to using the full standard terminology (scalar integer expression), but not the shorthand (scalar-int-expr) which few people outside the language lawyers know :) FX
Re: [Patch, fortran, 5] Bakport Andre's r222477 deep copy fix for PR67818
> In other words, > consider youself a reviewer for patches in an area > of the compiler that you are comfortable. Seconded. FX
Re: [PATCH] PR fortran/67987 -- character lengths cannot be negative
> The attach patch enforces the Fortran Standard's requirement > that character length must be great than or equal to zero. We've got to be careful about this. The standard (F2008) has this to say about character lengths: 4.4.3.1. "The number of characters in the string is called the length of the string. The length is a type parameter; its kind is processor dependent and its value is greater than or equal to zero.” but 4.4.3.2. "If the character length parameter value evaluates to a negative value, the length of character entities declared is zero." So while strings cannot have negative length, they can be declared with a length parameter value which is itself negative, leading to the string having zero length. Or, said otherwise: character(len=-2) :: s is legal and declares a string of zero length, being thus equivalent to: character(len=0) :: s Thus: not OK to commit. FX
Re: [PATCH] PR fortran/67987 -- character lengths cannot be negative
> 2015-10-16 Steven G. Kargl > > PR fortran/67987 > * decl.c (char_len_param_value): Unwrap unlong line. If LEN < 0, > then force it to zero pre Fortran Standards. > * resolve.c (gfc_resolve_substring_charlen): Unwrap unlong line. > If 'start' is larger than 'end', then length of string is negative, > so explicitly set it to zero. > (resolve_charlen): Remove -Wsurprising warning. Update comment to > text from F2008 standard. > > 2015-10-16 Steven G. Kargl > > PR fortran/67987 > * gfortran.dg/char_length_2.f90: Add declaration from PR to testcase. The patch is now mostly OK to me. Minor remarks: - I’m thinking you mean “force it to zero per [not pre] Fortran standards” - why remove the -Wsurprising warning? it seems a good case for -Wsurprising: legal code, but dubious anyway OK after you ponder that second point. FX
Re: [PATCH] PR fortran/67987 -- character lengths cannot be negative
> F90 is over 26 years old. There has been 3 major revisions that > have superceded F90 (F95, F03, and F08). All of those revisions > include the text that you pointed out to me. Why is it surprising > that a compiler conforms to the standard? > > "Simplify, simplify, simplify." Henry David Thoreau OK, go ahead.
Re: [PATCH] PR fortran/67939 -- Fix zero length strings in DATA statement
> 2015-10-21 Steven G. Kargl > > PR fortran/67939 > * data.c (create_character_initializer): Deal with zero length string. > > 2015-10-21 Steven G. Kargl > > PR fortran/67939 > * gfortran.dg/pr67939.f90: New test. OK, thanks
Re: [Patch, fortran] PR58754 - [4.9/5 Regression] ICE on allocating character array with source
> 2015-10-22 Paul Thomas > >PR fortran/58754 >* trans-stmt.c (gfc_trans_allocate): Do not use the scalar >character assignment if the allocate expression sis an array >descriptor. > > 2015-10-22 Paul Thomas > >PR fortran/58754 >* gfortran.dg/pr58754.f90: New test OK, apart from typo in the ChangeLog (“sis an array”).
Re: [PATCH] PR fortran/36192 -- Check for valid BT_INTEGER
> 2015-10-25 Steven G. Kargl > > PR fortran/36192 > * array.c (gfc_ref_dimen_size): Check for BT_INTEGER before calling > mpz_set. > > > 2015-10-25 Steven G. Kargl > > PR fortran/36192 > * gfortran.dg/pr36192.f90: New test. OK. But I don’t understand why the testcase’s dg-error pattern has this form: a regex “or” (|) of two identical strings? FX
Re: Possible patch for PR fortran/66056
> The problem: Statement labels within a type declaration are put in the > statement label tree belonging to the type declaration's namespace's (instead > of the current namespace). When the line is otherwise empty and an error is > issued, gfc_free_st_label tries to delete the label from the label tree > belonging to the current namespace and then frees the label structure, > leaving an invalid statement label pointer in the type declaration's > namespace's label tree. When that namespace is cleaned up, bad things can > happen. Seems OK. Please post patches with full ChangeLog entry, stating how they were tested (“bootstraped and regtested on x86_64-linux”, for example). And of course, do an actual full regression-test :) If that was done, then OK to commit. Thanks, FX
Re: Possible patch for PR fortran/66056
> I ran "make && make install && make check-fortran" at the top level on > x86_64-pc-linux-gnu. (I always do that before I post a patch and again > before I actually check anything in. I sometimes forget to include files > when I finally commit, but I'm always confident that the ones I remember are > good :) OK to commit. FX
Re: [PATCH] PR fortran/36192 -- Check for valid BT_INTEGER
> Because the code issues two errors, one for each dimension. Then shouldn’t it be “string.*string” to match two occurences of the string, with some stuff (incl. newline) in the middle? 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
[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: [Fortran, Patch] Implement IMPLICIT NONE
> Patch Patch cleaning up the testsuite (while Tobias is curing is cold :) is pre-approved. It comes from the last-minute wording change I suggested, I suppose. FX
[libquadmath,committed] Fix typo in doc
Committed as trivial, rev. 216006
2014-10-08 Francois-Xavier Coudert
PR libquadmath/63487
* libquadmath.texi (sincosq): Fix typo.
Index: libquadmath.texi
===
--- libquadmath.texi(revision 215645)
+++ libquadmath.texi(working copy)
@@ -199,7 +199,7 @@ The following mathematical functions are
@item @code{scalblnq}: compute exponent using @code{FLT_RADIX}
@item @code{scalbnq}: compute exponent using @code{FLT_RADIX}
@item @code{signbitq}: return sign bit
-@item @code{sincosq}: calculate sine and cosine simulataneously
+@item @code{sincosq}: calculate sine and cosine simultaneously
@item @code{sinhq}: hyperbolic sine function
@item @code{sinq}: sine function
@item @code{sqrtq}: square root function
[patch] Don't install libquadmath.info if libquadmath is not actually built
The attached patch prevents libquadmath from installing its info file if the library is not actually built. Tested on x86_64-linux, both on a built with libquadmath (normal) and without (tweaking configure so it thinks _float128 is not supported). OK to commit? quadmath.ChangeLog Description: Binary data quadmath.diff Description: Binary data
Re: [patch] Don't install libquadmath.info if libquadmath is not actually built
> The attached patch prevents libquadmath from installing its info file if the > library is not actually built. > Tested on x86_64-linux, both on a built with libquadmath (normal) and without > (tweaking configure so it thinks _float128 is not supported). > > OK to commit? Again, with correct ChangeLog entry including regeneration of Makefile.in quadmath.ChangeLog Description: Binary data quadmath.diff Description: Binary data
Re: [fortran,patch] Emit code for some IEEE functions in the front-end
> Ok, thanks for the patch! Committed as rev. 216036. Thanks for the review. FX
Re: [patch] Add -static-libquadmath option
Version 2 of the patch, now handling the darwin case (thanks Iain) and expressely noting in the documentation the implications on redistribution (thanks Joseph). Bootstrapped and regtested on x86_64-apple-darwin14. OK to commit? I need a C/driver options maintainer, or global reviewer, to OK the C changes (but they really are obvious). As Iain suggested the darwin.h change, I consider it pre-approved by him :) > We have a -static-libgfortran option, but on targets where we support > quad-prec math through libquadmath, we didn’t have an equivalent > -static-libquadmath so far. This patch adds it, in what I think is a rather > straightforward manner. > > The only minor complication comes from the fact that previously, linking > libquadmath was conditionally through libgfortran.spec. So the spec was > modified: when we use -static-libquadmath, the driver itself includes the > -lquadmath (surrounded by the necessary static linking directives), so that > libgfortran.spec shouldn’t do it again. > > Bootstrapped and regtested on x86_64 linux. OK to commit? static_quad.ChangeLog Description: Binary data static_quad.diff Description: Binary data
Re: [patch] Add -static-libquadmath option
> But it still needs to be OK'd by either a global reviewer or one of the > listed Darwin maintainers ;) ... > ... (ccing Mike) Duh me. I thought you were a darwin maintainer. Sorry. FX
[patch,fortran] Handle (signed) zeros, infinities and NaNs in some intrinsics
The attached patch fixes the compile-time simplification of special values (positive and negative zeros, infinities, and NaNs) in intrinsics EXPONENT, FRACTION, RRSPACING, SET_EXPONENT, SPACING. Those are all the intrinsics in the Fortran 2008 standard that say anything about these special values, so it makes sense to fix them. This is the compile-time part of PR 48979 (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=48979). Some notes: - We’re not technically required to do anything about infinities and NaNs unless IEEE_ARITHMETIC is accessible. My view is that it makes sense, as a quality of implementation issue, to handle them correctly anyway. I’ve done so here for simplification, and intent to do the same later for code generation in trans-intrinsic.c - For FRACTION, the 2003 standard says FRACTION(inf) = inf, while Fortran 2008 says FRACTION(inf) = NaN. I agree with Tobias, who said in the PR we shouldn’t emit different code based on -std=f2003/f2008. Instead, we use the Fortran 2008 intepretation here. It makes more sense anyway. - While digging into MPFR doc, I realized that the test (mpfr_sgn (x->value.real) == 0) used a few times in simplify.c is not only true for zeros, but also for NaNs! I thus replaced it with mpfr_zero_p (x->value.real). It affects only some (invalid) warnings. For example, before my patch, the code LOG((nan,nan)) would emit an error "Complex argument of LOG cannot be zero”, which makes little sense. Regtested on x86_64-apple-darwin14. OK to commit? FX intrinsics.ChangeLog Description: Binary data intrinsics.diff Description: Binary data
[fortran,patch]
After the compile-time simplification, this patch fixes the handling of special values (infinities and NaNs) by intrinsics EXPONENT, FRACTION, SPACING, RRSPACING & SET_EXPONENT on the code generation side. Bootstrapped and regtested on x86_64-linux. OK to commit? intrinsics.ChangeLog Description: Binary data intrinsics.diff Description: Binary data
Re: [patch] Add -static-libquadmath option
ping Patch needs: - C/driver options maintainer, or global reviewer, to OK the C changes (outside darwin). They are really simple - Fortran maintainer to OK the Fortran part > Version 2 of the patch, now handling the darwin case (thanks Iain) and > expressely noting in the documentation the implications on redistribution > (thanks Joseph). > Bootstrapped and regtested on x86_64-apple-darwin14. > > OK to commit? > > I need a C/driver options maintainer, or global reviewer, to OK the C changes > (but they really are obvious). > As Iain suggested the darwin.h change, I consider it pre-approved by him :) > > > > >> We have a -static-libgfortran option, but on targets where we support >> quad-prec math through libquadmath, we didn’t have an equivalent >> -static-libquadmath so far. This patch adds it, in what I think is a rather >> straightforward manner. >> >> The only minor complication comes from the fact that previously, linking >> libquadmath was conditionally through libgfortran.spec. So the spec was >> modified: when we use -static-libquadmath, the driver itself includes the >> -lquadmath (surrounded by the necessary static linking directives), so that >> libgfortran.spec shouldn’t do it again. >> >> Bootstrapped and regtested on x86_64 linux. OK to commit? > > static_quad.ChangeLog Description: Binary data static_quad.diff Description: Binary data >
[fortran,patch] Handle infinities and NaNs in intrinsics code generation
ping > After the compile-time simplification, this patch fixes the handling of > special values (infinities and NaNs) by intrinsics EXPONENT, FRACTION, > SPACING, RRSPACING & SET_EXPONENT on the code generation side. > > Bootstrapped and regtested on x86_64-linux. > OK to commit? intrinsics.ChangeLog Description: Binary data intrinsics.diff Description: Binary data
Re: [fortran,patch] Handle infinities and NaNs in intrinsics code generation
> Looks good to me. Thanks for taking care of F2003's IEEE support. Committed as rev. 216443, thanks for the review. > PS: You might want to browse through the current (F2008 + corrigenda > + first F2015 additions) draft at > http://j3-fortran.org/doc/year/14/14-007r2.pdf > > See especially the list at the beginning under the item > "Changes to the intrinsic modules IEEE_ARITHMETIC, IEEE_EXCEPTIONS, and > IEEE_FEATURES for conformance with ISO/IEC/IEEE 60559:2011: [...]" > and then later in that file. Thanks for the link. I’d rather wait until later in the process, and let the existing F2003 / F2008 parts mature & be tested for now. FX
Re: Avoid calls to realloc for nvptx
Hi Bernd, I’m afraid I don’t understand the reasoning here: > Since malloc and free are magically provided by the ptx environment, but > realloc is missing, it's nontrivial to provide an implementation for it. The > Fortran frontend likes to generate calls to realloc, but in one case it seems > like we can compute the old size, and call a function that does > malloc/memcpy/free instead. Does "nontrivial to provide” mean that you don’t provide a realloc() implementation in libc or libgcc? If so, I’m afraid the Fortran compiler will be terminally broken, and fixing just one of the use cases is not sufficient. FX
Re: [PATCH, Fortran] PR61234: -Wuse-no-only
> I think it is really weird if a coding style warning is included in -Wall. Same here. It’s not a very commonly shared coding style, so I don’t think it should be included in -Wall. Other than that, I like the idea (but cannot review the patch). FX
Re: [PATCH, Fortan] fix initialization of flag_errno_math and flag_associative_math
> Why do you want -fno-math-errno to be the default for gfortran? > AFAICT such default is not documented and the flag works > (checked on your test gfortran.dg/errnocheck_1.f90). It should be the default, and it used to be. Fortran doesn’t care about math routines setting errno (its existence is specified in C & C++ standards, but not in the Fortran standard). If I understand correctly, Joost’s patch just adapts this to a new flag handling mechanism. FX
Re: [PATCH] Add support for OpenMP fortran user defined reductions
> As discussed with Tobias on IRC yesterday, the fact that I'd like to > eventually backport the Fortran OpenMP 4.0 support to 4.9 branch > poses a problem with the module.c changes But this is by design, because we’re not supposed to add new features (especially API-changing or module-changing ones) in a release branch. The compatibility fixes you propose will increase the complexity of the module reader code, and creates some precedent. I don’t think there’s much pressure from the “general public” for Fortran OpenMP 4.0, so having it in 4.10 only rather than 4.9 is probably not such a big deal. FX
Re: [fortran, patch] IEEE intrinsic modules
Hi Janne, Thanks for the quick feedback. > - Wrt. libgfortran/gfortran.map: You have added the GFORTRAN_1.6 > symbol node, as you're the first one to export new symbols in the 4.10 > cycle. I've seen occasional confusion from users when they have symbol > version mismatches and e.g. "1.4" doesn't match any version they've > seen before. So I think it might be better to switch to a scheme where > the symbol node name matches the compiler version, i.e. GFORTRAN_4.10. I don’t have an opinion on that, I’ll follow what you settle on. > - Many of the intrinsics are just thin wrappers around > __builtin_foo(). Couldn't these be generated inline instead? They could, but… having them in the library allows to rely on its mechanism for detection of features, providing IEEE modules and functions only when we actually support them. I’m open to moving some of the IEEE handling towards the front-end, but we need to think clearly about that… FX
Re: [fortran, patch] IEEE intrinsic modules
> Please look at libgcc/config/i386/crtfastmath.c for how to set
> MXCSR_FTZ from mxcsr. You already have all necessary bits in place,
> the function is basically only:
>
> + if (has_sse())
> + {
> +unsigned int cw_sse;
> +
> +__asm__ __volatile__ ("%vstmxcsr\t%0" : "=m" (cw_sse));
> +cw_sse |= MXCSR_DAZ;
> +__asm__ __volatile__ ("%vldmxcsr\t%0" : : "m" (cw_sse));
> + }
Thanks for the suggestion!
> Please note, that FTZ applies only to SSE math. x87 and (IIRC) soft-FP
> don't handle this setting.
Yeah, that’s also why I prefer for now to have it declared as unsupported: the
Fortran standard doesn’t really allow for partial support such as this, so I’m
still trying to figure out what The Right Thing To Do is.
FX
[fortran, patch] F2003 "non-default kind specifiers" compliance
Hi all,
Our Fortran 2003 status page [1] says gfortran does not support "Kind type
parameters of integer specifiers”. This item is defined thusly (item 4.9 in
[2]):
> Some of the integer specifiers (e.g. NEXTREC) were limited to default kind in
> Fortran 95. Any kind of integer is permitted in Fortran 2003.
I wanted to fix this, so I combed through the 95, 2003 and 2008 standards, and
listed these changes. F2003 lifted all requirements on integer specifiers, and
F2008 lifted requirements on logical specifiers. However, it appears that all
of these are actually already handled in current trunk! So I’m proposing a
simple two-fold action:
- update the Fortran 2003 status to indicate our compliance
- commit the attached testcase (bootstrapped and regtested on
x86_64-apple-darwin) which will make sure we stay that way.
OK?
[1] https://gcc.gnu.org/wiki/Fortran2003Status
[2] ftp://ftp.nag.co.uk/sc22wg5/N1551-N1600/N1579.pdf
Index: gfortran.dg/nondefault_int_kind.f90
===
--- gfortran.dg/nondefault_int_kind.f90 (revision 0)
+++ gfortran.dg/nondefault_int_kind.f90 (working copy)
@@ -0,0 +1,26 @@
+! { dg-do compile }
+!
+! Test our conformance to item 4.9 ("Kind type parameters of integer
+! specifiers") of the Fortran 2003 status document at
+! ftp://ftp.nag.co.uk/sc22wg5/N1551-N1600/N1579.pdf
+!
+! The non-default logical variables are allowed since Fortran 2008.
+
+ integer(kind=8) :: i, j, k, n
+ logical(kind=8) :: l1, l2, l3
+
+ open(unit=10, status="scratch", iostat=i)
+
+ backspace(10, iostat=i)
+ endfile(10, iostat=i)
+ rewind(10, iostat=i)
+
+ read(*, '(I2)', advance='no', iostat=i, size=j) k
+
+ inquire(iolength=i) "42"
+ inquire(10, iostat=i, number=j, recl=k, nextrec=n)
+ inquire(10, exist=l1, opened=l2, named=l3)
+
+ close(unit=10, iostat=i)
+
+end
[fortran, patch] Audit and patch of F95 and F2003 "non-default kind specifiers" warnings
> It seems that some of these extensions are not caught by -std=f95 I’ve now audited the I/O specifiers for such warnings too. A warning existed only for EXIST, which was introduced way back by Steve: > 2010-07-05 Steven G. Kargl > > PR fortran/44797 > * fortran/io.c (resolve_tag): Check EXIST tag is a default logical. I’ve extended the logical warning to the NAMED, OPENED, and PENDING specifiers. I’ve also audited the integer specifiers, and extended the warning to NUMBER, NEXTREC, and RECL, which were missing. Bootstrapped and regtested on x86_64-apple-darwin13, OK to commit? FX io_diagnostics.ChangeLog Description: Binary data io_diagnostics.diff Description: Binary data
Re: [fortran, patch] Audit and patch of F95 and F2003 "non-default kind specifiers" warnings
> I’ve extended the logical warning to the NAMED, OPENED, and PENDING > specifiers. I’ve also audited the integer specifiers, and extended the > warning to NUMBER, NEXTREC, and RECL, which were missing. > > Bootstrapped and regtested on x86_64-apple-darwin13, OK to commit? Committed as rev. 211323. FX
[libgfortran, patch] Fix compilation on HP/UX 10
Attached patch should fix compilation of libgfortran on hppa1.1-hp-hpux10.20 (see PR60468: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60468), by adding a missing header to the configure checks. I’ve tested that it doesn’t break bootstrap on x86_64-apple-darwin, but not on hpux10. It seems safe enough to me to proceed first, and let John test in his next build of trunk (bootstrap on those machines probably isn’t fast). OK to commit? FX hpux.diff Description: Binary data hpux.ChangeLog Description: Binary data
[fortran,patch] Fix Cray pointers in modules
The attached one-line patch fixes PR45187 (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=45187), where we would sometimes try to create the backend_decl of Cray-pointees twice. We should simply bail out early of the procedure, following what is done in the similar situation in gfc_finish_var_decl(). Bootstrapped and regtested on x86_64-apple-darwin13, includes a testcase. OK to commit? FX cray.diff Description: Binary data cray.ChangeLog Description: Binary data
[fortran,patch] Binding label can be any initialisation expression
Well, only scalar character of the default kind, but still… This patch achieves this goal by following the obvious plan, which has not changed since I wrote it in PR 36275 in 2008 :) The custom matcher for binding label, in gfc_match_bind_c(), is removed and the generic matcher gfc_match_init_expr() is called, followed by checks that the expression obtained fulfills the constraints of a C identifier. So, now is the time to think about PR 38839 (what characters to allow as a binding label): right now, I allow alphadecimals, underscore and dollar. From the PR comments, it seems like @ and ` might be also allowed for universal-character names, but they are not supported by GCC on platforms I tested right now. Let me know what you think, but I don’t think we should worry to much about it. Bootstrapped and regtested on x86_64-apple-darwin13, comes with testcases. OK to commit? FX PS: you may notice I have had some time to hack at gfortran these past few days... it feels good! bind_c.ChangeLog Description: Binary data bind_c.diff Description: Binary data
Re: [libgfortran, patch] Fix compilation on HP/UX 10
ping > Attached patch should fix compilation of libgfortran on hppa1.1-hp-hpux10.20 > (see PR60468: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60468), by adding > a missing header to the configure checks. > > I’ve tested that it doesn’t break bootstrap on x86_64-apple-darwin, but not > on hpux10. It seems safe enough to me to proceed first, and let John test in > his next build of trunk (bootstrap on those machines probably isn’t fast). > > OK to commit? > > FX hpux.ChangeLog Description: Binary data hpux.diff Description: Binary data
Re: [libgfortran, patch] Fix compilation on HP/UX 10
Committed as rev. 211685, thanks for the review. FX
Re: [fortran,patch] Binding label can be any initialisation expression
ping To reinforce the message in my earlier email: we can fine-tune the list of allowed characters in identifiers later, but I’d like to get this patch in already (so it gets large exposure before the 4.10 release). FX > Binding label can be any initialisation expression. Well, only scalar > character of the default kind, but still… > > This patch achieves this goal by following the obvious plan, which has not > changed since I wrote it in PR 36275 in 2008 :) > The custom matcher for binding label, in gfc_match_bind_c(), is removed and > the generic matcher gfc_match_init_expr() is called, followed by checks that > the expression obtained fulfills the constraints of a C identifier. > > So, now is the time to think about PR 38839 (what characters to allow as a > binding label): right now, I allow alphadecimals, underscore and dollar. > >From the PR comments, it seems like @ and ` might be also allowed for > universal-character names, but they are not supported by GCC on platforms I > tested right now. Let me know what you think, but I don’t think we should > worry to much about it. > > Bootstrapped and regtested on x86_64-apple-darwin13, comes with testcases. > OK to commit? > > FX > > > PS: you may notice I have had some time to hack at gfortran these past few > days... it feels good! bind_c.ChangeLog Description: Binary data bind_c.diff Description: Binary data
[fortran,patch] One-line fix to PR61454 (init expression simplification)
In expr.c:scalarize_intrinsic_call(), we don't deal correctly with intrinsics that have an optional kind argument, while simplifying initialization expressions. The attached one-line patch fixes it, and adds a testcase so we don’t regress. Bootstrapped and regtested on x86_64-apple-darwin13. OK to commit? FX pr61454.diff Description: Binary data pr61454.ChangeLog Description: Binary data
Re: [fortran,patch] One-line fix to PR61454 (init expression simplification)
> Not only is it 'obvious' but it can do no harm in any circumstances > :-) OK to commit True! Committed as rev. 211822 FX
Re: [fortran, patch] IEEE intrinsic modules (ping)
>> 3. Does the attached updated patch (libgfortran only, without >> regenerated files) fix the problem? > > I'll test it when my regtesting is completed. But, a scan of > the configure.host re-arrangement suggests that it should work. OK. If you have some spare cycles, could you then also check it by modifying configure.host so that it uses the updated config/fpu-sysv.h in my patch? I would like to make sure I don’t break anything, but I don’t have access to a Solaris system (and my earlier calls for someone to test it for me were unanswered, so I don’t have much hope there). Thanks again, FX
Re: [fortran,patch] Binding label can be any initialisation expression
ping*2 > ping > > To reinforce the message in my earlier email: we can fine-tune the list of > allowed characters in identifiers later, but I’d like to get this patch in > already (so it gets large exposure before the 4.10 release). > > FX > > > >> Binding label can be any initialisation expression. Well, only scalar >> character of the default kind, but still… >> >> This patch achieves this goal by following the obvious plan, which has not >> changed since I wrote it in PR 36275 in 2008 :) >> The custom matcher for binding label, in gfc_match_bind_c(), is removed and >> the generic matcher gfc_match_init_expr() is called, followed by checks that >> the expression obtained fulfills the constraints of a C identifier. >> >> So, now is the time to think about PR 38839 (what characters to allow as a >> binding label): right now, I allow alphadecimals, underscore and dollar. >> From the PR comments, it seems like @ and ` might be also allowed for >> universal-character names, but they are not supported by GCC on platforms I >> tested right now. Let me know what you think, but I don’t think we should >> worry to much about it. >> >> Bootstrapped and regtested on x86_64-apple-darwin13, comes with testcases. >> OK to commit? >> >> FX bind_c.diff Description: Binary data bind_c.ChangeLog Description: Binary data
Re: [fortran, patch] IEEE intrinsic modules (ping)
> This may raise inexact, see C11 7.6.2.3. Installed as obvious. Thanks! FX
Re: [fortran,patch] Binding label can be any initialisation expression
> Works as advertized and even fixes pr38838. > Thanks for the patch. Thanks for the review, committed to trunk as rev. 212123 FX
Re: [PATCH, libgfortran]: Fix support_fpu_rounding_mode and add -mieee flags for alpha
Dear Uros,
Thanks very much for the patch. I have a few questions:
> the patch removes -O0 from dg-additiona-options in IEEE testsuite, as this
> always override default optimization flag.
That was my purpose: this test can fail with optimization (on x86_64, IIRC),
hence the -O0 should override the default optimization flags in the testsuite.
>* gfortran.dg/ieee/ieee_rounding_1.f90 (dg-additional-options): Add.
If -mfp-rounding-mode=d is required on alpha for IEEE conformance, it should be
added to add-ieee-options in lib/fortran-torture.exp, shouldn’t it? Rather than
enabled on a case-by-case basis. Also, we might want to document these
target-specific options here:
https://gcc.gnu.org/onlinedocs/gfortran/IEEE-modules.html
> BTW: On alpha, it is possible to enable underflow handling:
>
> #ifdef __USE_GNU
> /* On later hardware, and later kernels for earlier hardware, we can forcibly
> underflow denormal inputs and outputs. This can speed up certain programs
> significantly, usually without affecting accuracy. */
> enum
> {
>FE_MAP_DMZ =1UL << 12, /* Map denorm inputs to zero */
> #define FE_MAP_DMZ FE_MAP_DMZ
>
>FE_MAP_UMZ =1UL << 13, /* Map underflowed outputs to zero */
> #define FE_MAP_UMZ FE_MAP_UMZ
> };
> #endif
>
> FX, if you care for this option, I can help test the patch and
> corresponding testcases.
Yes, that’s interesting. What libc function do you call with those
FE_MAP_{D,U}MZ values?
I would also like to enable FTZ for i386/x86_64 at some point, but my issue
there is that it’s not “universal”, ie it’s only for SSE math if I understand
correctly. One point of view is that it’s better than nothing (especially now
that SSE math is probably the most used mode), but another point of view is
that it wouldn’t be standard conforming.
FX
[fortran,patch] Support for IEEE underflow control on x86/x86_64
Hi all, The attached patch provides support for underflow control in the IEEE_ARITHMETIC module, for x86/x86_64 targets (our main user base). Bootstrapped and regtested on x86_64-apple-darwin13. Comes with a testcase. OK to commit? FX underflow.ChangeLog Description: Binary data underflow.diff Description: Binary data
Re: [fortran,patch] Support for IEEE underflow control on x86/x86_64
> (I don't think -O0 is needed, but have to check with a testsuite run.) On x86_64-apple-darwin, -O0 or -O1 are needed: at -O2 my “use_real” call is optimized out anyway, and the division simplified at compile time. FX
Re: [fortran,patch] Support for IEEE underflow control on x86/x86_64
> I'd suggest to name this fie ieee_underflow_1.f90 for consistency. In fact, since the directory is called ieee/, I think I’ll rename the others so they don’t all start with ieee_ > BTW: underflow control also works on alpha, using following code: Could you test the attached libgfortran/config/fpu-glibc.h file on alpha? > You can mark variables with “volatile” Indeed, I should have thought of that. Once you report the results of the alpha modification, I’ll propose an updated patch with all of those remarks. Thanks, FX fpu-glibc.h Description: Binary data
Re: [fortran,patch] Support for IEEE underflow control on x86/x86_64
Here’s an updated patch, providing support for underflow control in the IEEE_ARITHMETIC module, for x86/x86_64 targets and alpha-glibc. Bootstrapped and regtested on x86_64-apple-darwin13, tested by Uros on alpha. OK to commit? underflow.ChangeLog Description: Binary data underflow.diff Description: Binary data
Re: [wwwdocs,Fortran] Announce IEEE intrinsic modules support for Fortran
> Also, for that page having 2-3(-4) words as a short title are necessary. > What would be appropriate here? "Fortran IEEE intrinsic modules”? Sounds good to me. FX
Re: [fortran, patch] IEEE intrinsic modules (ping)
Dear Rainer, > Unfortunately, while the patch works fine on Solaris/x86, it broke > Solaris/SPARC bootstrap for trivial reasons: contrary to the ChangeLog, > configure and config.h.in weren't regenerated, thus FPSETSTICKY > wasn't defined. I apologize. Thanks for checking in the fix. > FAIL: gfortran.dg/ieee/ieee_6.f90 -Os execution test > > The test aborts at l.47, but unfortunately I cannot print mode in gdb 7.7. That’s weird, especially if that one fails but ieee_rounding_1.f90 works. Let me know if I can do anything to help debug this. > The following patch corrects this, at the same time fixing this warning: > > /fpu-target.h:451:3: warning: implicit declaration of function 'assert' > [-Wimplicit-function-declaration] > assert (sizeof(fpu_state_t) <= GFC_FPE_STATE_BUFFER_SIZE); Actually, it makes a lot of sense to change these into static assertions: this way, any target-specific issues with FP-state buffer size will show up at libgfortran-building-time, rather than be swept under the rug. Since libgfortran is compiled with GCC, which supports _Static_assert since 4.6, I propose the attached patch. Built and tested on x86_64-linux, OK to commit? FX static_assert.ChangeLog Description: Binary data static_assert.diff Description: Binary data
Re: [fortran, patch] IEEE intrinsic modules (ping)
> Furthermore, on 2014-05-12 I committed a patch changing libgfortran to > be built with -std=gnu11 instead of -std=gnu99, so that we can make > use of C11 functionality; see > https://gcc.gnu.org/ml/fortran/2014-04/msg00101.html . Committed as rev. 212323, thanks for the review. I now propose the attached patch, which performs a small cleaning up: - Use the new _Noreturn language feature (supported in GCC since 2011) instead of the old attribute. This makes prototypes shorter and more generic. - Move the complex-related REALPART, IMAGPART and COMPLEX_ASSIGN macros from libgfortran.h to c99_intrinsics.c, which is the only place they’re ever used. Built and tested on x86_64-linux, OK to commit? FX clean.ChangeLog Description: Binary data clean.diff Description: Binary data PS: I didn’t touch libcaf, as I assume this might be compiled with a different compiler. Am I right? PS2: A third issue I’ve though about is: should we get rid of the following __GNUC__ test? libgfortran is not used as a standalone Fortran runtime library, and I think it is (and never will) be built by something else than a stage3 compiler. > #ifndef __GNUC__ > #define __attribute__(x) > #define likely(x) (x) > #define unlikely(x) (x) > #else > #define likely(x) __builtin_expect(!!(x), 1) > #define unlikely(x) __builtin_expect(!!(x), 0) > #endif
Re: [fortran, patch] IEEE intrinsic modules (ping)
Hi Rainer, > while the i386/amd64 values are the usual ones. Unfortunately, > gcc/fortran/libgfortran.h hardcodes the more common values for > GFC_FPE_*, and libgfortran/Makefile.am extracts them from there into > fpu-target.inc. I'm unsure what's the best way to handle this. No, we don’t hardcode any values (unless I misunderstand what you are saying). Look at libgfortran/config/fpu-sysv.h get_fpu_rounding_mode() and set_fpu_rounding_mode(): we have two switches, to translate between the GFC_FPE_* values and the FP_R* values. So this should work, really. The only thing I can see is that libgfortran/config/fpu-sysv.h assumes that FP_RM and others are macros, checking them with "#ifdef FP_RM”. Is that the reason? If so, we might just want to use them unconditionally… unless it creates a mess on some other SysV target! FX
Re: [fortran, patch] IEEE intrinsic modules (ping)
> FWIW, those FP_* values are also enum values in IRIX 6.5 , the > only other SysV target I have around. Seems this file is common between > all of them, so the risk should be manageable. > > The following patch does away with the #ifdef stuff and lets all > gfortran.dg/ieee tests PASS on sparc-sun-solaris2.11. Google for a few more targets (BSD, cygwin, etc.) confirms that there is little variation in this part of the file. Given that your patch fixes a target, and sounds good to both you and me, I suggest you commit it in 24 hours unless someone objects (or you get an actual review). Also, related to that: could you also confirm that FP_X_INV (and others) are indeed macros, on solaris? FX
Re: [fortran, patch] IEEE intrinsic modules (ping)
> What about --disable-bootstrap? Does it just skip stage1 and stage2, > and stage3 is used to compile libgfortran, or is the host compiler > used to build libgfortran as well? If the former, yes I guess we can > remove #ifnderf __GNUC__ stuff? Even with --disable-bootstrap, libgfortran is compiled with the freshly-built compiler, not the host compiler. FX
Re: [fortran, patch] IEEE intrinsic modules (ping)
> Right, that's what I (vaguelly) remembered. Please consider a patch > removing the ifndef __GNUC__ stuff from libgfortran.h pre-approved. The only occurrence (outside of libcaf) is in libgfortran.h. Committed attached patch as rev. 212328, after building on x86_64-linux. FX x Description: Binary data
Re: [patch, fortran] fix for PR 60780, PR 40958
> The change may be small enough that an assignment isn't needed. > We (ie, the gfortran developers) will need to check. I think that’s small enough, compared to what we’ve accepted as such in the past. If not, a disclaimer by Russell putting his change in the public domain would also be a quick way: https://gcc.gnu.org/contribute.html#legal > Having an assignment will help when you submit additional patches. :-) Indeed! Regarding the patch itself, it seems OK. (I first wondered if the strcmp() is necessary, but it appears module strings at this point are not GCC identifiers, but normal strings.) Russell, you said “tested on x86_64-linux”. Could you explicitly confirm that you have bootstrapped it and regression-tested the full gfortran testsuite ? Cheers, FX
