Re: Updated Sourceware infrastructure plans
On Thu, Apr 18, 2024 at 11:15 AM FX Coudert wrote:
>
> > I regenerate auto* files from time to time for libgfortran. Regenerating
> > them has always been very fragile (using --enable-maintainer-mode),
> > and difficult to get right.
>
> I have never found them difficult to regenerate, but if you have only a non
> maintainer build, it is a pain to have to make a new maintainer build for a
> minor change.
>
> Moreover, our m4 code is particularly painful to use and unreadable. I have
> been wondering for some time: should we switch to simpler Python scripts? It
> would also mean that we would have fewer files in the generated/ folder:
> right now, every time we add new combinations of types, we have a
> combinatorial explosion of files.
>
> $ ls generated/sum_*
> generated/sum_c10.c generated/sum_c17.c generated/sum_c8.c
> generated/sum_i16.c generated/sum_i4.c generated/sum_r10.c
> generated/sum_r17.c generated/sum_r8.c
> generated/sum_c16.c generated/sum_c4.c generated/sum_i1.c
> generated/sum_i2.c generated/sum_i8.c generated/sum_r16.c generated/sum_r4.c
>
> We could imagine having a single file for all sum intrinsics.
>
> How do Fortran maintainers feel about that?
For the time being I'm not an active maintainer, so my opinion doesn't
per se have weight, but back when I was active I did think about this
issue. IMHO the best of my ideas was to convert these into C++
templates. What we're essentially doing with the M4 stuff and the
proposed in-house Python reimplementation is to make up for lack of
monomorphization in plain old C. Rather than doing some DIY templates,
switch the implementation language to something which has that feature
built-in, in this case C++. No need to convert the entire libgfortran
to C++ if you don't want to, just those objects that are generated
from the M4 templates. Something like
template
void matmul(T* a, T* b, T* c, ...)
{
// actual matmul code here
}
extern "C" {
// Instantiate template for every type and export the symbol
void matmul_r4(gfc_array_r4* a, gfc_array_r4* b, gfc_array_r4* c, ...)
{
matmul(a, b, c, ...);
}
// And so on for other types
}
--
Janne Blomqvist
Re: [Fortran] half-cycle trig functions and atan[d] fixes
On Wed, Jan 24, 2024 at 10:28 AM FX Coudert wrote: > > Now, if > > the OS adds cospi() to libm and it's in libm's symbol map, then the > > cospi() used by gfortran depends on the search order of the loaded > > libraries. > > We only include the fallback math functions in libgfortran when they are not > available on the system. configure detects what is present in the libc being > targeted, and conditionally compiles the necessary fallback functions (and > only them). Exactly. However, there is the (corner?) case when libgfortran has been compiled, and cospi() not found and thus the fallback implementation is included, and then later libc is updated to a version that does provide cospi(). I believe in that case which version gets used is down to the library search order (i.e. the order that "ldd /path/to/binary" prints the libs), it will use the first symbol it finds. Also, it's not necessary to do some ifdef tricks with gfortran.map, if a symbol listed there isn't found in the library it's just ignored. So the *pi() trig functions can be unconditionally added there, and then depending on whether the target libm includes those or not they are then included in the exported symbol list. It's possible to override this to look for specific symbol versions etc., but that probably goes deep into the weeds of target-specific stuff (e.g. are we looking for cospi@FBSD_1.7, cospi@GLIBC_X.Y.Z, or something else?). I'm sure you don't wanna go there. -- Janne Blomqvist
Re: [Fortran] half-cycle trig functions and atan[d] fixes
On Tue, Jan 23, 2024 at 11:09 AM FX Coudert wrote: > > Hi Steve, Hello, long time no see. > Thanks for the patch. I’ll take time to do a proper review, but after a first > read I had the following questions: > > - "an OS's libm may/will contain cospi(), etc.”: do those functions conform > to any standard? Are there plans to implement them outside FreeBSD at this > point? acospi, asinpi, atan2pi, atanpi, cospi, sinpi, tanpi are all in IEEE 754-2019, and are slated to be part of C23. I would assume actively developed libm's will eventually catch up and implement them. > - If I get this right, to take one example, the Fortran front-end will emit a > call to gfortran_acospi_r4(), libgfortran provides this as a wrapper calling > acospif(), which is called either from libm or from libgfortran. This is > different from other math library functions, like ACOS() where the acosf() > call is generated directly from the front-end (and then the implementation > comes either from libm or from libgfortran). Why not follow our usual way of > doing things? Good point, that's what we've done in c99_functions.c in libgfortran. We should probably do this so we can skip jumping via libgfortran when libm implements these directly. > - On most targets, cospi() and friends are not available. Therefore, we end > up doing the fallback (with limited precision as you noted) but with a lot of > indirection. We could generate that code directly in the front-end, couldn’t > we? The frontend generally doesn't know what the target libm implements, hence it's better to just generate the call, and if necessary have a barebones implementation in libgfortran if libm doesn't implement it properly. -- Janne Blomqvist
Re: [PATCH][stage1] Remove conditionals around free()
On Wed, Mar 1, 2023 at 11:31 PM Bernhard Reutner-Fischer via Fortran wrote: > > Hi! > > Mere cosmetics. > > - if (foo != NULL) > free (foo); > > With the caveat that coccinelle ruins replacement whitespace or i'm > uneducated enough to be unable to _not_ run the diff through > sed -e 's/^+\([[:space:]]*\)free(/+\1free (/' > at least. If anybody knows how to improve replacement whitespace, > i'd be interrested but didn't look nor ask. ISTM that leading > whitespace is somewhat ruined, too, so beware (8 spaces versus tab as > far as i have spot-checked). > > Would touch > gcc/ada/rtinit.c |3 +-- > intl/bindtextdom.c |3 +-- > intl/loadmsgcat.c|6 ++ > intl/localcharset.c |3 +-- > libbacktrace/xztest.c|9 +++-- > libbacktrace/zstdtest.c |9 +++-- > libbacktrace/ztest.c |9 +++-- > libgfortran/caf/single.c |6 ++ > libgfortran/io/async.c |6 ++ > libgfortran/io/format.c |3 +-- > libgfortran/io/transfer.c|6 ++ > libgfortran/io/unix.c|3 +-- > libgo/runtime/go-setenv.c|6 ++ > libgo/runtime/go-unsetenv.c |3 +-- > libgomp/target.c |3 +-- > libiberty/concat.c |3 +-- > zlib/contrib/minizip/unzip.c |2 +- > zlib/contrib/minizip/zip.c |2 +- > zlib/examples/enough.c |6 ++ > zlib/examples/gun.c |2 +- > zlib/examples/gzjoin.c |3 +-- > zlib/examples/gzlog.c|6 ++ > > coccinelle script and invocation inline. > Would need to be split for the respective maintainers and run through > mklog with subject changelog and should of course be compiled and > tested before that. > > Remarks: > 1) We should do this in if-conversion (?) on our own. >I suppose. Independently of -fdelete-null-pointer-checks > 2) Maybe not silently, but raise language awareness nowadays. >By now it's been a long time since this was first mandated. > 3) fallout from looking at something completely different > 4) i most likely will not remember to split it apart and send proper >patches, tested patches, in stage 1 to maintainers proper, so if >anyone feels like pursuing this, be my guest. I thought i'd just >mention it. > > cheers, Back in 2011 Jim Meyering applied a patch doing this, see https://gcc.gnu.org/legacy-ml/gcc-patches/2011-03/msg00403.html , and the gcc/README.Portability snippet added then which is still there. Per analysis done then, it seems SunOS 4 was the last system where free() of a NULL pointer didn't behave per the spec. Also in Jim's patch intl/ and zlib/ directories were not touched as those are imported from other upstreams. -- Janne Blomqvist
Re: [PATCH 3/5] Fortran: Narrow return types [PR78798]
On Sun, Nov 13, 2022 at 1:47 AM Bernhard Reutner-Fischer via Fortran
wrote:
> --- a/gcc/fortran/arith.cc
> +++ b/gcc/fortran/arith.cc
> @@ -1135,7 +1135,7 @@ compare_complex (gfc_expr *op1, gfc_expr *op2)
> strings. We return -1 for a < b, 0 for a == b and 1 for a > b.
> We use the processor's default collating sequence. */
>
> -int
> +signed char
> gfc_compare_string (gfc_expr *a, gfc_expr *b)
> {
>size_t len, alen, blen, i;
> @@ -1162,7 +1162,7 @@ gfc_compare_string (gfc_expr *a, gfc_expr *b)
> }
Hmm, really? PR 78798 mentions changing int to bool, where
appropriate, which I think is uncontroversial, but this?
--
Janne Blomqvist
Re: Handling of large stack objects in GPU code generation -- maybe transform into heap allocation?
On Fri, Nov 11, 2022 at 4:13 PM Thomas Schwinge wrote: > For example, for Fortran code like: > > write (*,*) "Hello world" > > ..., 'gfortran' creates: > The issue: the stack object 'dt_parm.0' is a half-KiB in size (yes, > really! -- there's a lot of state in Fortran I/O apparently). > Any other clever ideas? There's a lot of potential options to set during Fortran I/O, but in the vast majority of cases only a few are used. So a better library interface would be to transfer only those options that are used, and then let the full set of options live in heap memory managed by libgfortran. Say some kind of simple byte-code format, with an 'opcode' saying which option it is, followed by the value. See also https://gcc.gnu.org/bugzilla/show_bug.cgi?id=48419 for some rough ideas in this direction, although I'm not personally working on GFortran at this time so somebody else would have to pick it up. -- Janne Blomqvist
Re: Lowest i386 CPU Model with proper C++ atomics
On Fri, Sep 11, 2020 at 6:52 PM Joel Sherrill wrote: > > Hi > > Over at RTEMS, we ran into a case where the C++ atomics may not be right > for one of the lower level x86 models. We will investigate whether it can > be made right but this has led to the discussion of dropping older models > and setting a new minimum model. Right now, our base is a i386 w/FPU. The > best rationale we have for selecting a new floor is GCC atomics support. > > With that in mind, what's the lowest/oldest i386 CPU model we > should consider as the new base model? We sort-of discussed this issue back when the Linux kernel dropped support for i386. See thread starting at https://gcc.gnu.org/legacy-ml/gcc/2012-12/msg00079.html (a thread where you participated as well) -- Janne Blomqvist
Re: [PATCH] libgfortran: Fix and simplify IO locking [PR92836]
On Sat, Feb 1, 2020 at 6:37 PM Thomas Koenig wrote: > > Hi Janne, > > > Simplify IO locking in libgfortran. The new IO implementation avoids > > accessing units without locks, as seen in PR 92836. It also avoids > > lock inversion (except for a corner case wrt namelist query when > > reading from stdin and outputting to stdout), making it easier to > > verify correctness with tools like valgrind or threadsanitizer. It is > > also simplified as the waiting and closed variables are not needed > > anymore, making it easier to understand and analyze. > > > > Regtested on x86_64-pc-linux-gnu, Ok for master? > > I'll look into it, this might take a bit of time. Thanks for looking into this. > What are you planning to use as a test case? You can put > multighreading programs into libgomp/testsuite/libgomp.fortran > where they will be executed. In this case I just ran the test program in comment #14 in PR 92836 under valgrind --tool=helgrind (there is still a remaining problem with the locking of unit_cache, but that can be handled separately, and I think it's benign). I'm not sure how one would run such a test as part of the testsuite. But if anyone has tests that reliably deadlock within a relatively short time, then yeah, such testcases would be awesome. -- Janne Blomqvist
Re: [PATCH] libgfortran: Fix and simplify IO locking [PR92836]
On Fri, Jan 31, 2020 at 3:38 PM Janne Blomqvist wrote: > > Simplify IO locking in libgfortran. The new IO implementation avoids > accessing units without locks, as seen in PR 92836. It also avoids > lock inversion (except for a corner case wrt namelist query when > reading from stdin and outputting to stdout), making it easier to > verify correctness with tools like valgrind or threadsanitizer. It is > also simplified as the waiting and closed variables are not needed > anymore, making it easier to understand and analyze. > > Regtested on x86_64-pc-linux-gnu, Ok for master? And, I forgot the ChangeLog entry. Here it is: libgfortran/ChangeLog: 2020-01-31 Janne Blomqvist PR libfortran/92836 * io/close.c (st_close): Close unit with unit_lock held. * io/io.h (gfc_unit): Remove waiting and closed members. (find_unit_locked): New prototype. (inc_waiting_locked): Remove. (predec_waiting_locked): Remove. (dec_waiting_unlocked): Remove. * io/list_read.c (nml_query): Avoid deadlock due to lock inversion. * io/unit.c (newunit_lock): New variable. (get_gfc_unit): Add new argument, use it. Remove logic waiting for closing logic.. (find_unit): Use new argument for get_gfc_unit. (find_unit_locked): New function. (find_or_create_unit): Use new argumet for gfc_get_unit. (get_unit): Likewise. (init_units): Initialize newunit_lock. (close_unit_1): Change meaning of locked argument, remove waiting for closing logic. (close_unit): Adapt to new close_unit_1 arguments. (close_units): Likewwise. (newunit_alloc): Use newunit_lock to protect. (newunit_free): Likewise. * io/unix.c (find_file0): Lock unit before accessing. (find_file): Remove waiting for closing logic. (flush_all_units_1): Likewise. (flush_all_units): Likewise. -- Janne Blomqvist
[PATCH] libgfortran: Fix and simplify IO locking [PR92836]
Simplify IO locking in libgfortran. The new IO implementation avoids
accessing units without locks, as seen in PR 92836. It also avoids
lock inversion (except for a corner case wrt namelist query when
reading from stdin and outputting to stdout), making it easier to
verify correctness with tools like valgrind or threadsanitizer. It is
also simplified as the waiting and closed variables are not needed
anymore, making it easier to understand and analyze.
Regtested on x86_64-pc-linux-gnu, Ok for master?
---
libgfortran/io/close.c | 17 ++---
libgfortran/io/io.h| 57 +-
libgfortran/io/list_read.c | 26 ++-
libgfortran/io/unit.c | 150 +++--
libgfortran/io/unix.c | 89 --
5 files changed, 109 insertions(+), 230 deletions(-)
diff --git a/libgfortran/io/close.c b/libgfortran/io/close.c
index 8aaa00393e7..3b176285149 100644
--- a/libgfortran/io/close.c
+++ b/libgfortran/io/close.c
@@ -61,20 +61,15 @@ st_close (st_parameter_close *clp)
find_option (>common, clp->status, clp->status_len,
status_opt, "Bad STATUS parameter in CLOSE statement");
- u = find_unit (clp->common.unit);
+ LOCK (_lock);
+ u = find_unit_locked (clp->common.unit);
if (ASYNC_IO && u && u->au)
if (async_wait (&(clp->common), u->au))
- {
- library_end ();
- return;
- }
+ goto done;
if ((clp->common.flags & IOPARM_LIBRETURN_MASK) != IOPARM_LIBRETURN_OK)
- {
-library_end ();
-return;
- }
+goto done;
if (u != NULL)
{
@@ -123,6 +118,8 @@ st_close (st_parameter_close *clp)
#endif
}
- /* CLOSE on unconnected unit is legal and a no-op: F95 std., 9.3.5. */
+ done:
+ /* CLOSE on unconnected unit is legal and a no-op: F95 std., 9.3.5. */
+ UNLOCK (_lock);
library_end ();
}
diff --git a/libgfortran/io/io.h b/libgfortran/io/io.h
index ab4a103787c..ae0ed1b10a9 100644
--- a/libgfortran/io/io.h
+++ b/libgfortran/io/io.h
@@ -681,16 +681,6 @@ typedef struct gfc_unit
struct async_unit *au;
__gthread_mutex_t lock;
- /* Number of threads waiting to acquire this unit's lock.
- When non-zero, close_unit doesn't only removes the unit
- from the UNIT_ROOT tree, but doesn't free it and the
- last of the waiting threads will do that.
- This must be either atomically increased/decreased, or
- always guarded by UNIT_LOCK. */
- int waiting;
- /* Flag set by close_unit if the unit as been closed.
- Must be manipulated under unit's lock. */
- int closed;
/* For traversing arrays */
array_loop_spec *ls;
@@ -780,6 +770,9 @@ internal_proto(stash_internal_unit);
extern gfc_unit *find_unit (int);
internal_proto(find_unit);
+extern gfc_unit *find_unit_locked (int);
+internal_proto(find_unit_locked);
+
extern gfc_unit *find_or_create_unit (int);
internal_proto(find_or_create_unit);
@@ -973,50 +966,6 @@ internal_proto(size_from_complex_kind);
extern void free_ionml (st_parameter_dt *);
internal_proto(free_ionml);
-static inline void
-inc_waiting_locked (gfc_unit *u)
-{
-#ifdef HAVE_ATOMIC_FETCH_ADD
- (void) __atomic_fetch_add (>waiting, 1, __ATOMIC_RELAXED);
-#else
- u->waiting++;
-#endif
-}
-
-static inline int
-predec_waiting_locked (gfc_unit *u)
-{
-#ifdef HAVE_ATOMIC_FETCH_ADD
- /* Note that the pattern
-
- if (predec_waiting_locked (u) == 0)
- // destroy u
-
- could be further optimized by making this be an __ATOMIC_RELEASE,
- and then inserting a
-
- __atomic_thread_fence (__ATOMIC_ACQUIRE);
-
- inside the branch before destroying. But for now, lets keep it
- simple. */
- return __atomic_add_fetch (>waiting, -1, __ATOMIC_ACQ_REL);
-#else
- return --u->waiting;
-#endif
-}
-
-static inline void
-dec_waiting_unlocked (gfc_unit *u)
-{
-#ifdef HAVE_ATOMIC_FETCH_ADD
- (void) __atomic_fetch_add (>waiting, -1, __ATOMIC_RELAXED);
-#else
- __gthread_mutex_lock (_lock);
- u->waiting--;
- __gthread_mutex_unlock (_lock);
-#endif
-}
-
static inline void
memset4 (gfc_char4_t *p, gfc_char4_t c, int k)
diff --git a/libgfortran/io/list_read.c b/libgfortran/io/list_read.c
index 77d61421a0f..c337b3c8600 100644
--- a/libgfortran/io/list_read.c
+++ b/libgfortran/io/list_read.c
@@ -2793,7 +2793,31 @@ nml_query (st_parameter_dt *dtp, char c)
/* Store the current unit and transfer to stdout. */
temp_unit = dtp->u.p.current_unit;
- dtp->u.p.current_unit = find_unit (options.stdout_unit);
+
+ /* Since we already hold the lock for stdin (temp_unit), trying to
+ acquire the unit_lock in order to find the stdout unit would be a
+ lock order inversion, which is not allowed as it could cause a
+ deadlock. Hence first try to lock unit_lock without blocking.
+ If that fails, fall back to unlocking temp_unit and then block
+ waiting for the lock in order to avoid the lock order inversion.
+ This is strictly speaking not correct, as it's possible another
+
Re: Fixing gcc git logs
On Thu, Jan 2, 2020 at 4:00 AM Jerry wrote: > > In the following git log entry, I made a typo on the PR number in the > libgfortran ChangeLog file. I noticed this right after the git commit, > while editing the git log. > > So I quit the edit without saving and git reported that the commit was > aborted. > > Then I edited the my local ChangeLog file, did git add . and git commit > again. In this case the git log edit started as usual and I recopied > the correct text into my editor (VI) and saved it. The commit proceeded > as I expected. Then I did the git svn dcommit > > Well the git log ended up like this. How to fix? How to avoid? > > The 90274 should have been 90374. The actual committed files are OK. Not sure what you did wrong. I guess to fix it you need to make a separate commit and push that. In general, before you push your changes to some public tree, you can engage in various forms of history rewriting. For instance, if you forgot something or realized you made a mistake in your latest commit, you can fix the error, and then do a "git commit --amend" which instead of creating a new commit will add your changes to your previous commit. Another option is the interactive rebase (git rebase -i). That allows you to e.g. "squash" commits, that is, combine two or more commits into one. -- Janne Blomqvist
Re: [Patch, Fortran] PR92896 [10 Regression] Fix - Prevent character conversion in array constructor
On Tue, Dec 17, 2019 at 7:47 PM Steve Kargl
wrote:
>
> On Tue, Dec 17, 2019 at 05:28:05PM +, Mark Eggleston wrote:
> >
> > On 17/12/2019 17:06, Steve Kargl wrote:
> > > On Tue, Dec 17, 2019 at 03:41:41PM +, Mark Eggleston wrote:
> > >> gcc/fortran/ChangeLog
> > >>
> > >> Mark Eggleston
> > >>
> > >> PR fortran/92896
> > >> * array.c (walk_array_constructor): Replace call to
> > >> cfg_convert_type
> > > s/cfg_convert_type/gfc_convert_type
> > >
> > >> with call to gfc_convert_type_warn with new argument set to true.
> > >> (check_element_type): Replace call to cfg_convert_type with call to
> > >> gfc_convert_type_warn with new argument set to true.
> > >> * gfortran.h: Add argument "array" to gfc_convert_type_warn default
> > >> value set to false.
> > > Do all current uses of gfc_convert_type_warn need to be updated
> > > to account for the new parameter? That is, doesn't this introduce
> > > a mismatch in the prototype and existing code?
> >
> > I used a default value so all existing calls remain as they are and
> > default to false. So no mismatch.
> >
>
> % cat a.h
> #ifndef _STDBOOL_H_
> #include
> #endif
> float foo(int, float, bool tmp = false);
> % cat a.c
> #include "a.h"
> void
> bar(float x)
> {
> int n;
> n = 1;
> x = foo(n, x);
> }
> % /usr/home/sgk/work/x/bin/gcc -Wall -c a.c
> In file included from a.c:2:
> a.h:1:32: error: expected ';', ',' or ')' before '=' token
> 1 | float foo(int, float, bool tmp = false);
> |^
> a.c: In function 'bar':
> a.c:8:7: warning: implicit declaration of function 'foo'
> [-Wimplicit-function-declaration]
> 8 | x = foo(n, x);
> | ^~~
Well, frontends are nowadays C++, so
a) No need to include stdbool.h, bool is a builtin type.
b) optional arguments are a thing (they are also used elsewhere in the
Fortran frontend).
--
Janne Blomqvist
Re: *PING* – Re: [Patch, Fortran] PR 92793 - fix column used for error diagnostic
On Fri, Dec 6, 2019 at 10:02 AM Tobias Burnus wrote:
>
> *Ping*
Ok.
>
> Regarding Frederik's remark about the testsuite:
>
> I think the only test case in gfortran.dg/, which tests the column
> number, is use_without_only_1.f90. It has:
> { dg-warning "7:has no ONLY qualifier" }
> here, the "7" is the column number. — Hence, it is not surprising that
> changes do not affect the test suite.
>
> Cheers,
>
> Tobias
>
> On 12/4/19 2:37 PM, Tobias Burnus wrote:
> > As reported internally by Frederik, gfortran currently passes
> > LOCATION_COLUMN == 0 to the middle end. The reason for that is how
> > parsing works – gfortran reads the input line by line.
> >
> > For internal error diagnostic (fortran/error.c), the column location
> > was corrected – but not for locations passed to the middle end.
> > Hence, the diagnostic there wasn't optimal.
> >
> > Fixed by introducing a new function; now one only needs to make sure
> > that no new code will re-introduce "lb->location" :-)
> >
> > Build and regtested on x86-64-gnu-linux.
> > OK for the trunk?
> >
> > Tobias
> >
--
Janne Blomqvist
Re: [Patch, Fortran] PR92754 - fix an issue with resolving intrinsic functions
On Tue, Dec 3, 2019 at 6:41 PM Tobias Burnus wrote: > > The problem here is that one gets two symbols - one inside the block and > one outside and they do not really agree whether one has a function or a > variable – which later gives an ICE. As sym->module was "(intrinsic)" > and FL_VARIABLE, one was running into an assert. > > The problem is that when resolving the expression with "max", gfortran > detects that it got an intrinsic function and resolves the > expression/function call (in this case to a constant). However, the > information that "max" was regarded as intrinsic procedure never ends up > in the symbol itself, only in the expression. > > My first idea was to call gfc_resolve_intrinsic, which does a lot of > nice things like setting the attributes, warning that the type is > ignored with intrinsic procedures (-Wsurprsing, only) etc. However, that > gives a bunch of ICE. Even a more Spartan attr setting had similar > issues – hence, I moved it below the simplification, in the hope it > would work there. Well, gfc_resolve_intrinsic still gave tons of errors > and ICEs, but what I now have works. (I think it also works w/o the > FL_UNKNOWN condition, but, in any case, I am not sure which variant is > better) > > The settings I use with this patch seem to work and I have the hope that > it covers all valid/invalid code correctly (fingers crossed). > > Build and regtested on x86-64-gnu-linux. > OK for the trunk? Yeah, this seems a bit convoluted. But at least we now have yet another testcase to catch similar issues. So, Ok. -- Janne Blomqvist
Re: [Patch, gcc-wwdocs] Update to Fortran changes
On Wed, Nov 27, 2019 at 10:29 AM Tobias Burnus wrote: > > On 11/27/19 8:58 AM, Janne Blomqvist wrote: > > On Tue, Nov 26, 2019 at 1:12 PM Tobias Burnus > > wrote: > >> AUTOMATIC attribute > > Speaking of which, to avoid confusion maybe we should rename the > > -f[no-]automatic option to something like -f[no-]save, since the > > Fortran standard description of an "automatic data object" doesn't > > really have anything to do with the option, and neither does the > > option have anything to do specifically with the DEC AUTOMATIC > > specifier? > > While in principle I concur that the naming is bad, I fear this will > break several makefiles – hence, I am not sure it is worth the trouble – > and I am declined to say that it isn't. Yes, of course we can't remove it outright. But we could keep it as a deprecated alias for -f[no-]save (or whatever it should be called) and document it as such. -- Janne Blomqvist
Re: [Patch] PR 92463 - Cleanups due to minimum MPFR version bump to 3.1.0
On Tue, Nov 26, 2019 at 11:42 PM Tobias Burnus wrote:
>
> Recently, Janne bumped the minimal MPFR version from 2.4 to 3.1. As a
> follow-up cleanup, the mp_ data types can now be changed to mpfr_ and
> GMP_RNDx rounding to MPFR_RNDx (with an additional MPFR_RNDA).
>
> This patch changes the types in the middle end and a left over from
> previous patch round in Fortran. However, I have not touched the MPFR
> users in GO.
>
> See https://www.mpfr.org/mpfr-3.0.0/#changes for the 2.4 to 3.0 changes.
> ("mp_rnd_t may be removed in the future").
>
> Bootstrapped and regtested on x86-64-gnu-linux.
> OK for the trunk?
Fortran part is obviously Ok (sorry I missed that one in my previous patch).
For realmpfr.h, should we additionally poison GMP_RNDx to make sure we
don't accidentally add back uses of it?
Rest of the patch looks Ok, though not being a global reviewer I can't
approve it.
--
Janne Blomqvist
Re: [Patch, gcc-wwdocs] Update to Fortran changes
On Tue, Nov 26, 2019 at 1:12 PM Tobias Burnus wrote: > In particular, currently, the following item does not make clear that it > is about legacy support: "Variables with the AUTOMATIC > attribute can be used in EQUIVALENCE statements." > > I think it can remain as is with the -fdec umbrella item. But without it > really requires the background knowledge that the AUTOMATIC attribute is > only available with -fdec-static (or -fdec). Speaking of which, to avoid confusion maybe we should rename the -f[no-]automatic option to something like -f[no-]save, since the Fortran standard description of an "automatic data object" doesn't really have anything to do with the option, and neither does the option have anything to do specifically with the DEC AUTOMATIC specifier? -- Janne Blomqvist
Re: [Patch,Fortran] PR92629 - ICE in convert_mpz_to_unsigned, at fortran/simplify.c:173
On Sun, Nov 24, 2019 at 10:47 PM Harald Anlauf wrote: > > In convert_mpz_to_unsigned, an assert was triggered when a positive > argument larger than HUGE() of the respective kind was passed while > -fno-range-check was specified. The assert should be consistently > skipped in this case. > > Regtested on x86_64-pc-linux-gnu. > > OK for trunk / 9 / 8? > > Thanks, > Harald > > 2019-11-24 Harald Anlauf > > PR fortran/92629 > * simplify.c (convert_mpz_to_unsigned): Skip assert for argument > range when -fno-range-check is specified. > > > 2019-11-24 Harald Anlauf > > PR fortran/92629 > * gfortran.dg/pr92629.f90: New testcase. Ok, thanks. -- Janne Blomqvist
Re: [patch, fortran] Load scalar intent-in variables at the beginning of procedures
On Thu, Nov 21, 2019 at 12:31 AM Janne Blomqvist
wrote:
>
> On Thu, Nov 21, 2019 at 12:18 AM Janne Blomqvist
> wrote:
> >
> > On Wed, Nov 20, 2019 at 11:35 PM Thomas König wrote:
> > > (Why do we zero %eax
> > > before each call? It should not be a variadic call right?)
> >
> > Not sure. Maybe some belt and suspenders thing? I guess someone better
> > versed in ABI minutiae knows better. It's not Fortran-specific though,
> > the C frontend does the same when calling a void function.
>
> Ah, scratch that, it is some varargs-thing, I had forgot that a C
> function with no arguments or lacking a prototype is considered a
> varargs. The code
>
> void foo();
> void bar(void);
>
> void testfoo()
> {
> foo();
> }
>
> void testbar()
> {
> bar();
> }
>
> void testunprototyped()
> {
> baz();
> }
>
>
> generates code (elided scaffolding):
>
> testfoo:
> xorl %eax, %eax
> jmp foo
> testbar:
> jmp bar
> testunprototyped:
> xorl %eax, %eax
> jmp baz
>
>
> So probably this is due to the Fortran procedures lacking an interface
> being considered varargs by the caller. Starts to smell like some
> leftover from PR 87689?
Thinking some more about it, I'm thinking we should leave it as is,
even though it's a (small) code bloat. So Fortran calling a procedure
with an implicit interface most closely resembles the C unprototyped
function call. The problem is that we don't know much about the
callee. What if a Fortran procedure calls a C varargs procedure? In
the Fortran caller, we don't know whether the callee is varargs or
not, but if we don't zero %eax all hell could break loose if it
happened to be a varargs function. Yes, we could hide behind Fortran
not supporting varargs, and it's all the users fault, but is it really
worth it? I'd say no.
(in some cases zeroing a register is used to tell the OoO hw to kill a
dependency, so it can be beneficial for performance even if not
strictly required for correctness)
--
Janne Blomqvist
Re: [patch, fortran] Load scalar intent-in variables at the beginning of procedures
On Thu, Nov 21, 2019 at 12:18 AM Janne Blomqvist
wrote:
>
> On Wed, Nov 20, 2019 at 11:35 PM Thomas König wrote:
> > (Why do we zero %eax
> > before each call? It should not be a variadic call right?)
>
> Not sure. Maybe some belt and suspenders thing? I guess someone better
> versed in ABI minutiae knows better. It's not Fortran-specific though,
> the C frontend does the same when calling a void function.
Ah, scratch that, it is some varargs-thing, I had forgot that a C
function with no arguments or lacking a prototype is considered a
varargs. The code
void foo();
void bar(void);
void testfoo()
{
foo();
}
void testbar()
{
bar();
}
void testunprototyped()
{
baz();
}
generates code (elided scaffolding):
testfoo:
xorl %eax, %eax
jmp foo
testbar:
jmp bar
testunprototyped:
xorl %eax, %eax
jmp baz
So probably this is due to the Fortran procedures lacking an interface
being considered varargs by the caller. Starts to smell like some
leftover from PR 87689?
--
Janne Blomqvist
Re: [patch, fortran] Load scalar intent-in variables at the beginning of procedures
On Wed, Nov 20, 2019 at 11:35 PM Thomas König wrote: > > Am 20.11.19 um 21:45 schrieb Janne Blomqvist: > > BTW, since this is done for the purpose of optimization, have you done > > testing on some suitable benchmark suite such as polyhedron, whether > > it a) generates any different code b) does it make it go faster? > > I haven't run any actual benchmarks. > > However, there is a simple example which shows its advantages. > Consider > >subroutine foo(n,m) >m = 0 >do 100 i=1,100 > call bar > m = m + n > 100 continue >end > > (I used old-style DO loops just because :-) > > Without the optimization, the inner loop is translated to > > .L2: > xorl%eax, %eax > callbar_ > movl(%r12), %eax > addl%eax, 0(%rbp) > subl$1, %ebx > jne .L2 > > and with the optimization to > > .L2: > xorl%eax, %eax > callbar_ > addl%r12d, 0(%rbp) > subl$1, %ebx > jne .L2 > > so the load of the address is missing. (Why do we zero %eax > before each call? It should not be a variadic call right?) Not sure. Maybe some belt and suspenders thing? I guess someone better versed in ABI minutiae knows better. It's not Fortran-specific though, the C frontend does the same when calling a void function. AFAIK on reasonably current OoO CPU's xor'ing a register with itself is handled by the renamer and doesn't consume an execute slot, so it's in effect a zero-cycle instruction. Still bloats the code slightly, though. > Of course, Fortran language rules specify that the call to bar > cannot do anything to n Hmm, does it? What about the following modification to your testcase: module nmod integer :: n end module nmod subroutine foo(n,m) m = 0 do 100 i=1,100 call bar m = m + n 100 continue end subroutine foo subroutine bar() use nmod n = 0 end subroutine bar program main use nmod implicit none integer :: m n = 1 m = 0 call foo(n, m) print *, m end program main > So, a copy in / copy out for variables where we can not be sure that > no value is assigned? Does anybody see a downside for that?) In principle sounds good, unless my concerns above are real and affect this case too. > > Is there a risk of performance regressions due to higher register pressure? > > I don't think so. Either the compiler realizes that it can > keep the variable in a register (then it makes no difference), > or it has to load it fresh from its address (then there is > one additional register needed). Yes, true. Good point. -- Janne Blomqvist
Re: [patch, fortran] Load scalar intent-in variables at the beginning of procedures
On Sat, Nov 16, 2019 at 10:34 PM Thomas Koenig wrote: > > Hello world, > > here is an update to the patch. > > I have now included variables where the user did not specify INTENT(IN) > by checking that the dummy variables to be replaced by temporaries > are not, indeed, assigned a value. This also includes being passed > as an actual argument to a non-INTENT(IN) dummy argument. > > Extending this led to being able to catch a few more bugs. > > I have addes one test case to check where the new temporaries are added. > > Regression-tested. The only change I see in the testsuite now is > > XPASS: gfortran.dg/goacc/kernels-loop-n.f95 -O scan-tree-dump-times > parloops1 "(?n)__attribute__\\(\\(oacc kernels parallelized, oacc > function \\(, , \\), oacc kernels, omp target entrypoint\\)\\)" 1 BTW, since this is done for the purpose of optimization, have you done testing on some suitable benchmark suite such as polyhedron, whether it a) generates any different code b) does it make it go faster? Is there a risk of performance regressions due to higher register pressure? -- Janne Blomqvist
Re: [patch, fortran] Load scalar intent-in variables at the beginning of procedures
On Wed, Nov 20, 2019 at 8:00 PM Bernhard Reutner-Fischer wrote: > > On 19 November 2019 23:54:55 CET, Thomas Koenig wrote: > >Am 19.11.19 um 11:39 schrieb Bernhard Reutner-Fischer: > >> + char name[GFC_MAX_SYMBOL_LEN + 1]; > >> + snprintf (name, GFC_MAX_SYMBOL_LEN, "__dummy_%d_%s", var_num++, > >> +f->sym->name); > >> + > >> + if (gfc_get_sym_tree (name, ns, , false) != 0) > >> > >> (I) you should + sizeof(__dummy__) + 10 for unsigned long %d or the > >like. > > > >GFC_MAX_SYMBOL_LEN is the maximum length of a gfortran symbol. AFAIK, > > This is only true for user-provided names in the source code. > > Think e.g. class names as can be seen in the dumps.. We have GFC_MAX_MANGLED_SYMBOL_LEN for that. *Insert my standard pet peeve rant that we should use heap allocated unlimited length strings for these rather than copying stack allocated strings around, or preferable a symbol table* > >it > >is not possible to use a longer symbol name than that, so it needs to > >be > >truncated. Uniqueness of the variable name is guaranteed by the var_num > >variable. > > > >If the user puts dummy arguments Supercalifragilisticexpialidociousa > >and > >Supercalifragilisticexpialidociousb into the argument list of a > >procedure, he will have to look at the numbers to differentiate them > >:-) > > > >> (II) s/__dummy/__intent_in/ for clarity? > > > >It's moved away a bit from INTENT(IN) now, because an argument which > >cannot be modified (even by passing to a procedure with a dummy > >argument > >with unknown intent) is now also handled. > > So maybe __readonly_ , __rdonly_, __rd_or the like? dummy is really > misleading a name in the dumps.. Well, dummy is a term with a precise definition in the Fortran standard, so in a way it's good so one realizes it has something to do with a dummy argument. But yes, it's a bit misleading because it's not the dummy argument itself but rather a dereferenced copy of it. I suggest __readonly_dereferenced_dummy_copy_yes_this_is_a_really_long_name_. :) -- Janne Blomqvist
Re: [PATCH] Switch gcc ftp URL's to http
On Wed, Nov 20, 2019 at 7:53 PM Joseph Myers wrote: > > This patch is OK with http changed to https. (That is, with it changed > where the patch is already changing the URL. While changing http to https > makes sense more generally in the documentation whenever a site supports > https, that's probably best not mixed with the move from ftp.) Thanks for the review, done, and committed as r278526. Any opinions on whether I should apply this to the 8 and 9 branches as well? And the same question for my previous patch updating contrib/download_prerequisites at https://gcc.gnu.org/ml/gcc-patches/2019-11/msg00957.html . -- Janne Blomqvist
Re: [PATCH 1/2] PR 92463 MPFR modernization in GFortran
On Wed, Nov 20, 2019 at 5:46 PM Tobias Burnus wrote: > > Hi Janne, > > On 11/18/19 9:34 PM, Janne Blomqvist wrote: > > Now that we require a minimum of MPFR 3.1.0+ to build GCC, we can do > > some modernization of the MPFR usage in the GFortran frontend. > > OK – thanks for the cleanup. > > [For reference, those changes were introduced in MPFR 3.0.0, cf. > https://www.mpfr.org/mpfr-3.0.0/#changes ; 3.0 and 3.1 also added a > bunch of new functions, MPFR_RNDA and did remove some undefined behaviour.] > > Do you intent update the other places as well? Namely: > > > This patch replaces > > 1) GMP_RND* with MPFR_RND* > Also used by: configure.ac, gcc/builtins.c, gcc/fold-const-call.c, > gcc/gimple-ssa-sprintf.c, gcc/go/…, gcc/real.c, gcc/realmpfr.h, > gcc/ubsan.c. > > 2) mp_exp_t with mpfr_exp_t > > gcc/realmpfr.c + gcc/go/… > > > 3) mp_prec_t with mpfr_prec_t > (only used by gcc/fortran) > > 4) mp_rnd_t with mpfr_rnd_t > > gcc/builtins.c, gcc/fold-const-call.c, gcc/realmpfr.c > Thanks for the review, committed as r278523. My plan was that once the fortran-specific parts are done, I'll reassign the PR to the middle-end, and then me or somebody else can make a separate patch for that. However, since we're in stage3 now I believe this will have to wait until after the GCC 10 release as the middle-end people seem to take these rules more seriously. -- Janne Blomqvist
Re: [PATCH] Switch gcc ftp URL's to http
PING On Wed, Nov 13, 2019 at 11:05 PM Janne Blomqvist wrote: > > On Wed, Nov 13, 2019 at 10:41 PM Andrew Pinski wrote: > > > > On Wed, Nov 13, 2019 at 12:37 PM Janne Blomqvist > > wrote: > > > > > > The FTP protocol is getting long in the tooth, and we should emphasize > > > HTTP where that is available. This patch changes various gcc.gnu.org > > > URL's to instead use HTTP. > > > > May I suggest you use https instead of http here? Because it will be > > redirected anyways to use https. > > For me, when I use my normal web browser (firefox), it does redirect > to https. But I'm using the "HTTPS everywhere" extension, so I'm not > sure if it's the extension that does it, or if the server redirects > me, or if it's some other web-security-thingy that does it. When I use > curl, and if I manage to interpret the output correctly, it does not > redirect. > > That being said, it's probably a good idea to use https anyway. So > yes, consider it done (I'm not sending a new iteration of the patch > for this). > > > -- > Janne Blomqvist -- Janne Blomqvist
[PATCH 2/2] PR 92463 MPFR modernization: Revert r269139
Commit r269139 fixed an accidental dependency on MPFR 3.0. As we now
require at least MPFR 3.1.0+ we can revert it and instead use the
simpler MPFR 3.0+ code.
ChangeLog entry of the original commit was:
2019-02-23 David Malcolm
Jakub Jelinek
PR middle-end/88074
* simplify.c (norm2_do_sqrt, gfc_simplify_norm2): Use
mpfr_number_p && !mpfr_zero_p instead of mpfr_regular_p.
(norm2_add_squared): Likewise. Use mp_exp_t rather than mpfr_exp_t.
---
gcc/fortran/simplify.c | 14 +-
1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/gcc/fortran/simplify.c b/gcc/fortran/simplify.c
index a5c940ca2d5..b48bf014121 100644
--- a/gcc/fortran/simplify.c
+++ b/gcc/fortran/simplify.c
@@ -6023,8 +6023,8 @@ norm2_add_squared (gfc_expr *result, gfc_expr *e)
gfc_set_model_kind (result->ts.kind);
int index = gfc_validate_kind (BT_REAL, result->ts.kind, false);
- mp_exp_t exp;
- if (mpfr_number_p (result->value.real) && !mpfr_zero_p (result->value.real))
+ mpfr_exp_t exp;
+ if (mpfr_regular_p (result->value.real))
{
exp = mpfr_get_exp (result->value.real);
/* If result is getting close to overflowing, scale down. */
@@ -6038,7 +6038,7 @@ norm2_add_squared (gfc_expr *result, gfc_expr *e)
}
mpfr_init (tmp);
- if (mpfr_number_p (e->value.real) && !mpfr_zero_p (e->value.real))
+ if (mpfr_regular_p (e->value.real))
{
exp = mpfr_get_exp (e->value.real);
/* If e**2 would overflow or close to overflowing, scale down. */
@@ -6079,9 +6079,7 @@ norm2_do_sqrt (gfc_expr *result, gfc_expr *e)
if (result != e)
mpfr_set (result->value.real, e->value.real, GFC_RND_MODE);
mpfr_sqrt (result->value.real, result->value.real, GFC_RND_MODE);
- if (norm2_scale
- && mpfr_number_p (result->value.real)
- && !mpfr_zero_p (result->value.real))
+ if (norm2_scale && mpfr_regular_p (result->value.real))
{
mpfr_t tmp;
mpfr_init (tmp);
@@ -6120,9 +6118,7 @@ gfc_simplify_norm2 (gfc_expr *e, gfc_expr *dim)
result = simplify_transformation_to_scalar (result, e, NULL,
norm2_add_squared);
mpfr_sqrt (result->value.real, result->value.real, GFC_RND_MODE);
- if (norm2_scale
- && mpfr_number_p (result->value.real)
- && !mpfr_zero_p (result->value.real))
+ if (norm2_scale && mpfr_regular_p (result->value.real))
{
mpfr_t tmp;
mpfr_init (tmp);
--
2.17.1
[PATCH 1/2] PR 92463 MPFR modernization in GFortran
Now that we require a minimum of MPFR 3.1.0+ to build GCC, we can do
some modernization of the MPFR usage in the GFortran frontend.
This patch replaces
1) GMP_RND* with MPFR_RND*
2) mp_exp_t with mpfr_exp_t
3) mp_prec_t with mpfr_prec_t
4) mp_rnd_t with mpfr_rnd_t
gcc/fortran/ChangeLog:
2019-11-18 Janne Blomqvist
PR fortran/92463
* arith.c (gfc_mpfr_to_mpz): Change mp_exp_t to mpfr_exp_t.
(gfc_check_real_range): Likewise.
* gfortran.h (GFC_RND_MODE): Change GMP_RNDN to MPFR_RNDN.
* module.c (mio_gmp_real): Change mp_exp_t to mpfr_exp_t.
* simplify.c (degrees_f): Change mp_rnd_t to mpfr_rnd_t.
(radians_f): Likewise.
(fullprec_erfc_scaled): Change mp_prec_t to mpfr_prec_t.
(asympt_erfc_scaled): Likewise.
(gfc_simplify_nearest): Change mp_exp_t to mpfr_exp_t, and
GMP_RND* to MPFR_RND*.
---
gcc/fortran/arith.c| 8
gcc/fortran/gfortran.h | 2 +-
gcc/fortran/module.c | 2 +-
gcc/fortran/simplify.c | 20 ++--
4 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/gcc/fortran/arith.c b/gcc/fortran/arith.c
index a40163e8a2d..cf480779709 100644
--- a/gcc/fortran/arith.c
+++ b/gcc/fortran/arith.c
@@ -38,7 +38,7 @@ along with GCC; see the file COPYING3. If not see
void
gfc_mpfr_to_mpz (mpz_t z, mpfr_t x, locus *where)
{
- mp_exp_t e;
+ mpfr_exp_t e;
if (mpfr_inf_p (x) || mpfr_nan_p (x))
{
@@ -376,7 +376,7 @@ gfc_check_real_range (mpfr_t p, int kind)
}
else if (mpfr_cmp (q, gfc_real_kinds[i].tiny) < 0)
{
- mp_exp_t emin, emax;
+ mpfr_exp_t emin, emax;
int en;
/* Save current values of emin and emax. */
@@ -396,9 +396,9 @@ gfc_check_real_range (mpfr_t p, int kind)
/* Copy sign if needed. */
if (mpfr_sgn (p) < 0)
- mpfr_neg (p, q, GMP_RNDN);
+ mpfr_neg (p, q, MPFR_RNDN);
else
- mpfr_set (p, q, GMP_RNDN);
+ mpfr_set (p, q, MPFR_RNDN);
}
mpfr_clear (q);
diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h
index e962db59bc5..f4a2b99bdc4 100644
--- a/gcc/fortran/gfortran.h
+++ b/gcc/fortran/gfortran.h
@@ -2130,7 +2130,7 @@ gfc_intrinsic_sym;
#include
#include
-#define GFC_RND_MODE GMP_RNDN
+#define GFC_RND_MODE MPFR_RNDN
#define GFC_MPC_RND_MODE MPC_RNDNN
typedef splay_tree gfc_constructor_base;
diff --git a/gcc/fortran/module.c b/gcc/fortran/module.c
index 755f237a0e7..b11a30bea73 100644
--- a/gcc/fortran/module.c
+++ b/gcc/fortran/module.c
@@ -3343,7 +3343,7 @@ mio_gmp_integer (mpz_t *integer)
static void
mio_gmp_real (mpfr_t *real)
{
- mp_exp_t exponent;
+ mpfr_exp_t exponent;
char *p;
if (iomode == IO_INPUT)
diff --git a/gcc/fortran/simplify.c b/gcc/fortran/simplify.c
index 0461d31cd88..a5c940ca2d5 100644
--- a/gcc/fortran/simplify.c
+++ b/gcc/fortran/simplify.c
@@ -1768,7 +1768,7 @@ simplify_trig_call (gfc_expr *icall)
/* Convert a floating-point number from radians to degrees. */
static void
-degrees_f (mpfr_t x, mp_rnd_t rnd_mode)
+degrees_f (mpfr_t x, mpfr_rnd_t rnd_mode)
{
mpfr_t tmp;
mpfr_init (tmp);
@@ -1791,7 +1791,7 @@ degrees_f (mpfr_t x, mp_rnd_t rnd_mode)
/* Convert a floating-point number from degrees to radians. */
static void
-radians_f (mpfr_t x, mp_rnd_t rnd_mode)
+radians_f (mpfr_t x, mpfr_rnd_t rnd_mode)
{
mpfr_t tmp;
mpfr_init (tmp);
@@ -2681,7 +2681,7 @@ gfc_simplify_erfc (gfc_expr *x)
static void
fullprec_erfc_scaled (mpfr_t res, mpfr_t arg)
{
- mp_prec_t prec;
+ mpfr_prec_t prec;
mpfr_t a, b;
prec = mpfr_get_default_prec ();
@@ -2718,7 +2718,7 @@ asympt_erfc_scaled (mpfr_t res, mpfr_t arg)
{
mpfr_t sum, x, u, v, w, oldsum, sumtrunc;
mpz_t num;
- mp_prec_t prec;
+ mpfr_prec_t prec;
unsigned i;
prec = mpfr_get_default_prec ();
@@ -5914,7 +5914,7 @@ gfc_expr *
gfc_simplify_nearest (gfc_expr *x, gfc_expr *s)
{
gfc_expr *result;
- mp_exp_t emin, emax;
+ mpfr_exp_t emin, emax;
int kind;
if (x->expr_type != EXPR_CONSTANT || s->expr_type != EXPR_CONSTANT)
@@ -5928,20 +5928,20 @@ gfc_simplify_nearest (gfc_expr *x, gfc_expr *s)
/* Set emin and emax for the current model number. */
kind = gfc_validate_kind (BT_REAL, x->ts.kind, 0);
- mpfr_set_emin ((mp_exp_t) gfc_real_kinds[kind].min_exponent -
+ mpfr_set_emin ((mpfr_exp_t) gfc_real_kinds[kind].min_exponent -
mpfr_get_prec(result->value.real) + 1);
- mpfr_set_emax ((mp_exp_t) gfc_real_kinds[kind].max_exponent - 1);
- mpfr_check_range (result->value.real, 0, GMP_RNDU);
+ mpfr_set_emax ((mpfr_exp_t) gfc_real_kinds[kind].max_exponent - 1);
+ mpfr_check_range (result->value.real, 0, MPFR_RNDU);
if (mpfr_sgn (s->value.real) > 0)
{
mpfr_nextabove (result->value.real);
- mpfr_subnormalize (result->value.real, 0, GMP_RNDU);
+ mpfr_subnormalize (result->value.real, 0, MPFR_RNDU);
}
else
{
Re: [PATCH] Switch gcc ftp URL's to http
On Wed, Nov 13, 2019 at 10:41 PM Andrew Pinski wrote: > > On Wed, Nov 13, 2019 at 12:37 PM Janne Blomqvist > wrote: > > > > The FTP protocol is getting long in the tooth, and we should emphasize > > HTTP where that is available. This patch changes various gcc.gnu.org > > URL's to instead use HTTP. > > May I suggest you use https instead of http here? Because it will be > redirected anyways to use https. For me, when I use my normal web browser (firefox), it does redirect to https. But I'm using the "HTTPS everywhere" extension, so I'm not sure if it's the extension that does it, or if the server redirects me, or if it's some other web-security-thingy that does it. When I use curl, and if I manage to interpret the output correctly, it does not redirect. That being said, it's probably a good idea to use https anyway. So yes, consider it done (I'm not sending a new iteration of the patch for this). -- Janne Blomqvist
[PATCH] Switch gcc ftp URL's to http
The FTP protocol is getting long in the tooth, and we should emphasize
HTTP where that is available. This patch changes various gcc.gnu.org
URL's to instead use HTTP.
For instance, kernel.org shut down FTP access in 2017, with the
explanation:
- The protocol is inefficient and requires adding awkward kludges to
firewalls and load-balancing daemons
- FTP servers have no support for caching or accelerators, which has
significant performance impacts
- Most software implementations have stagnated and see infrequent
updates
ChangeLog:
2019-11-13 Janne Blomqvist
* configure.ac: Use http for gcc.gnu.org.
* configure: Regenerated.
gcc/ChangeLog:
2019-11-13 Janne Blomqvist
* configure.ac: Use http for gcc.gnu.org
* configure: Regenerated.
* doc/install.texi: Use http for gcc.gnu.org.
* doc/sourcebuild.texi: Likewise.
gcc/testsuite/ChangeLog:
2019-11-13 Janne Blomqvist
* README: Likewise.
libstdc++-v3/ChangeLog:
2019-11-13 Janne Blomqvist
* doc/html/api.html: Likewise.
* doc/xml/api.xml: Likewise.
maintainer-scripts/ChangeLog:
2019-11-13 Janne Blomqvist
* gcc_release: Likewise.
---
configure.ac | 2 +-
gcc/configure.ac | 2 +-
gcc/doc/install.texi | 11 +--
gcc/doc/sourcebuild.texi | 4 ++--
gcc/testsuite/README | 2 +-
libstdc++-v3/doc/html/api.html | 4 ++--
libstdc++-v3/doc/xml/api.xml | 2 +-
maintainer-scripts/gcc_release | 2 +-
8 files changed, 14 insertions(+), 15 deletions(-)
diff --git a/configure.ac b/configure.ac
index d63a8bae940..774e95a989f 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1665,7 +1665,7 @@ if test -d ${srcdir}/gcc && test "x$have_gmp" = xno; then
Try the --with-gmp, --with-mpfr and/or --with-mpc options to specify
their locations. Source code for these libraries can be found at
their respective hosting sites as well as at
-ftp://gcc.gnu.org/pub/gcc/infrastructure/. See also
+http://gcc.gnu.org/pub/gcc/infrastructure/. See also
http://gcc.gnu.org/install/prerequisites.html for additional info. If
you obtained GMP, MPFR and/or MPC from a vendor distribution package,
make sure that you have installed both the libraries and the header
diff --git a/gcc/configure.ac b/gcc/configure.ac
index b9cc2435cdf..7bb77f4e7a0 100644
--- a/gcc/configure.ac
+++ b/gcc/configure.ac
@@ -4748,7 +4748,7 @@ gd:
[ .machine ppc7400])
if test x$gcc_cv_as_machine_directive != xyes; then
echo "*** This target requires an assembler supporting \".machine\""
>&2
- echo you can get it from:
ftp://gcc.gnu.org/pub/gcc/infrastructure/cctools-528.5.dmg >&2
+ echo you can get it from:
http://gcc.gnu.org/pub/gcc/infrastructure/cctools-528.5.dmg >&2
test x$build = x$target && exit 1
fi
;;
diff --git a/gcc/doc/install.texi b/gcc/doc/install.texi
index 215a6fa38ff..c88d73f10cc 100644
--- a/gcc/doc/install.texi
+++ b/gcc/doc/install.texi
@@ -333,7 +333,7 @@ requirements.
@itemx bzip2 version 1.0.2 (or later)
Necessary to uncompress GCC @command{tar} files when source code is
-obtained via FTP mirror sites.
+obtained via HTTP mirror sites.
@item GNU make version 3.80 (or later)
@@ -411,7 +411,7 @@ download_prerequisites installs.
@item isl Library version 0.15 or later.
Necessary to build GCC with the Graphite loop optimizations.
-It can be downloaded from @uref{ftp://gcc.gnu.org/pub/gcc/infrastructure/}.
+It can be downloaded from @uref{http://gcc.gnu.org/pub/gcc/infrastructure/}.
If an isl source distribution is found
in a subdirectory of your GCC sources named @file{isl}, it will be
built together with GCC. Alternatively, the @option{--with-isl} configure
@@ -513,7 +513,7 @@ files in the directories below @file{jit/docs}.
@itemx SSH (any version)
Necessary to access the SVN repository. Public releases and weekly
-snapshots of the development sources are also available via FTP@.
+snapshots of the development sources are also available via HTTP@.
@item GNU diffutils version 2.7 (or later)
@@ -547,9 +547,8 @@ own sources.
@cindex Downloading GCC
@cindex Downloading the Source
-GCC is distributed via @uref{http://gcc.gnu.org/svn.html,,SVN} and FTP
-tarballs compressed with @command{gzip} or
-@command{bzip2}.
+GCC is distributed via @uref{http://gcc.gnu.org/svn.html,,SVN} and via
+HTTP as tarballs compressed with @command{gzip} or @command{bzip2}.
Please refer to the @uref{http://gcc.gnu.org/releases.html,,releases web page}
for information on how to obtain GCC@.
diff --git a/gcc/doc/sourcebuild.texi b/gcc/doc/sourcebuild.texi
index f3bf66c44ee..2650fc078b3 100644
--- a/gcc/doc/sourcebuild.texi
+++ b/gcc/doc/sourcebuild.texi
@@ -566,8 +566,8 @@ and the online manuals should be linked to from
@file{onlinedocs/index.html}.
@item
Any old releases
[PATCH] contrib/download_prerequisites: Use http instead of ftp
Convert the download_prerequisites script to use http instead of
ftp. This works better with firewalls, proxies, and so on. It's also
faster, a quick test on my system before patch:
time contrib/download_prerequisites --directory=/tmp/foo --force
...
real0m17,843s
After patch:
time contrib/download_prerequisites --directory=/tmp/foo --force
...
real0m11,059s
(fastest of three runs)
Question: Should we in fact use https? I haven't used it since
download_prerequisites checks that the sha512/md5 matches the ones in
the GCC tree, but maybe there are reasons? Even https is in fact
faster than ftp:
time contrib/download_prerequisites --directory=/tmp/foo --force
...
real0m12,729s
---
contrib/download_prerequisites | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/contrib/download_prerequisites b/contrib/download_prerequisites
index 72976c46c92..aa0356e6266 100755
--- a/contrib/download_prerequisites
+++ b/contrib/download_prerequisites
@@ -32,7 +32,7 @@ mpfr='mpfr-3.1.4.tar.bz2'
mpc='mpc-1.0.3.tar.gz'
isl='isl-0.18.tar.bz2'
-base_url='ftp://gcc.gnu.org/pub/gcc/infrastructure/'
+base_url='http://gcc.gnu.org/pub/gcc/infrastructure/'
echo_archives() {
echo "${gmp}"
@@ -58,7 +58,7 @@ esac
if type wget > /dev/null ; then
fetch='wget'
else
- fetch='curl -LO -u anonymous:'
+ fetch='curl -LO'
fi
chksum_extension='sha512'
directory='.'
--
2.17.1
Re: [patch, fortran] Load scalar intent-in variables at the beginning of procedures
On Tue, Nov 12, 2019 at 12:53 AM Thomas König wrote: > > Hi Janne, > > > Wouldn't it be even better to pass scalar intent(in) variables by > > value? The obvious objection of course is ABI, but for procedures with > > an explicit interface we're not following any particular ABI anyways? > > The problem with that is that we don't know when we compile a procedure > if it will be called with an explicit interface or not. > > The user can always add an interface block for a stand-alone procedure. Ah, of course. I should have said module procedures. Or even module procedures without bind(C)? That being said, I've seen examples where people have figured out the symbol mangling and are calling module procedures directly from C, so will breaking such code (even if not officially supported) be an issue? -- Janne Blomqvist
Re: [PATCH] Bump minimum MPFR version to 3.1.0
On Tue, Nov 12, 2019 at 6:59 AM Jeff Law wrote: > > On 11/10/19 4:05 AM, Janne Blomqvist wrote: > > On Sun, Nov 10, 2019 at 11:43 AM Thomas Koenig > > wrote: > >> > >> Hi Janne, > >> > >>> Bump the minimum MPFR version to 3.1.0, released 2011-10-03. With this > >>> requirement one can still build GCC with the operating system provided > >>> MPFR on old but still supported operating systems like SLES 12 (MPFR > >>> 3.1.2) or RHEL/CentOS 7.x (MPFR 3.1.1). > >>> > >> > >> OK for trunk. > > > > Thanks, I'll take that as an Ok for the Fortran part. I believe I > > still need an Ok by a global or build machinery reviewer for the > > global and docs parts. > Seems reasonable to me, particularly since 3.1.0 is covered by SLES 12 > and RHEL/CentOS 7. Give others a couple day to object though. Richard Biener already Ok'd it and I committed the patch yesterday. -- Janne Blomqvist
Re: [patch, fortran] Load scalar intent-in variables at the beginning of procedures
On Mon, Nov 11, 2019 at 11:56 PM Thomas König wrote: > > Hello world, > > the attached patch loads scalar INTENT(IN) variables to a local > variable at the start of a procedure, as suggested in PR 67202, in > order to aid optimization. This is controlled by front-end > optimization so it is easier to catch if any bugs should turn up :-) > > This is done to make optimization by the middle-end easier. > > I left in the parts for debugging that I added for this patch. > Seeing the difference between EXEC_INIT_ASSIGN and EXEC_ASSIGN was > particularly instructive. > > Regression-tested. OK for trunk? Wouldn't it be even better to pass scalar intent(in) variables by value? The obvious objection of course is ABI, but for procedures with an explicit interface we're not following any particular ABI anyways? -- Janne Blomqvist
Re: [Patch, RFC] PR81651/Fortran - Enhancement request: have f951 print out fully qualified module file name
On Mon, Nov 11, 2019 at 11:54 PM Harald Anlauf wrote: > > Dear all, > > the attached patch prints the fully qualified path if an error occurs > during module read. E.g., instead of a less helpful error message, > > pr81651.f90:2:6: > > 2 | use netcdf > | 1 > Fatal Error: File 'netcdf.mod' opened at (1) is not a GNU Fortran module > file > > gfortran will print > > pr81651.f90:2:7: > > 2 | use netcdf > | 1 > Fatal Error: File '/opt/pgi/pkg/netcdf/include/netcdf.mod' opened at (1) > is not a GNU Fortran module file > > Regtested on x86_64-pc-linux-gnu. > > I couldn't think of a sensible test for the testsuite, thus no testcase > provided. > > OK for trunk? > > Thanks, > Harald > > 2019-11-11 Harald Anlauf > > PR fortran/81651 > * module.c (gzopen_included_file, gzopen_included_file_1) > (gzopen_intrinsic_module, bad_module, gfc_use_module): Use fully > qualified module path for error reporting. Ok. -- Janne Blomqvist
Re: [PATCH] Bump minimum MPFR version to 3.1.0
On Mon, Nov 11, 2019 at 8:29 PM Joseph Myers wrote: > > On Sat, 9 Nov 2019, Janne Blomqvist wrote: > > > Bump the minimum MPFR version to 3.1.0, released 2011-10-03. With this > > requirement one can still build GCC with the operating system provided > > MPFR on old but still supported operating systems like SLES 12 (MPFR > > 3.1.2) or RHEL/CentOS 7.x (MPFR 3.1.1). > > > > This allows removing some code in the Fortran frontend, as well as > > fixing PR 91828. > > As a remark, beyond the cases where this allows conditional code to be > removed, there are also possible cleanups where GCC is currently using > MPFR constants / types that are documented as obsolescent > <https://gcc.gnu.org/ml/gcc-patches/2016-04/msg01544.html>. > > -- > Joseph S. Myers > jos...@codesourcery.com Yes, good idea. I filed https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92463 to keep track of this. -- Janne Blomqvist
[PATCH, committed] Don't print warning when moving to static with -fno-automatic
As part of PR 91413, GFortran now prints a warning when a variable is
moved from the stack to static storage. However, when the user
explicitly specifies that all local variables should be put in static
storage with the -fno-automatic option, don't print this warning.
Regtested on x86_64-pc-linux-gnu, committed r278027 as obvious.
gcc/fortran/ChangeLog:
2019-11-10 Janne Blomqvist
PR fortran/91413
* trans-decl.c (gfc_finish_var_decl): Don't print warning when
-fno-automatic is enabled.
---
gcc/fortran/trans-decl.c | 19 ++-
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/gcc/fortran/trans-decl.c b/gcc/fortran/trans-decl.c
index ffa6316..76e1c7a8453 100644
--- a/gcc/fortran/trans-decl.c
+++ b/gcc/fortran/trans-decl.c
@@ -746,15 +746,16 @@ gfc_finish_var_decl (tree decl, gfc_symbol * sym)
|| sym->attr.allocatable)
&& !DECL_ARTIFICIAL (decl))
{
- gfc_warning (OPT_Wsurprising,
- "Array %qs at %L is larger than limit set by"
- " %<-fmax-stack-var-size=%>, moved from stack to static"
- " storage. This makes the procedure unsafe when called"
- " recursively, or concurrently from multiple threads."
- " Consider using %<-frecursive%>, or increase the"
- " %<-fmax-stack-var-size=%> limit, or change the code to"
- " use an ALLOCATABLE array.",
- sym->name, >declared_at);
+ if (flag_max_stack_var_size > 0)
+ gfc_warning (OPT_Wsurprising,
+"Array %qs at %L is larger than limit set by"
+" %<-fmax-stack-var-size=%>, moved from stack to static"
+" storage. This makes the procedure unsafe when called"
+" recursively, or concurrently from multiple threads."
+" Consider using %<-frecursive%>, or increase the"
+" %<-fmax-stack-var-size=%> limit, or change the code to"
+" use an ALLOCATABLE array.",
+sym->name, >declared_at);
TREE_STATIC (decl) = 1;
--
2.17.1
Re: [PATCH] Bump minimum MPFR version to 3.1.0
On Sun, Nov 10, 2019 at 11:43 AM Thomas Koenig wrote: > > Hi Janne, > > > Bump the minimum MPFR version to 3.1.0, released 2011-10-03. With this > > requirement one can still build GCC with the operating system provided > > MPFR on old but still supported operating systems like SLES 12 (MPFR > > 3.1.2) or RHEL/CentOS 7.x (MPFR 3.1.1). > > > > OK for trunk. Thanks, I'll take that as an Ok for the Fortran part. I believe I still need an Ok by a global or build machinery reviewer for the global and docs parts. > Can you also make a note in https://gcc.gnu.org/gcc-10/changes.html ? Sure, will do, when the patch is accepted. -- Janne Blomqvist
[PATCH] Bump minimum MPFR version to 3.1.0
Bump the minimum MPFR version to 3.1.0, released 2011-10-03. With this
requirement one can still build GCC with the operating system provided
MPFR on old but still supported operating systems like SLES 12 (MPFR
3.1.2) or RHEL/CentOS 7.x (MPFR 3.1.1).
This allows removing some code in the Fortran frontend, as well as
fixing PR 91828.
ChangeLog:
2019-11-09 Janne Blomqvist
PR fortran/91828
* configure.ac: Bump minimum MPFR to 3.1.0, recommended to 3.1.6+.
* configure: Regenerated.
gcc/ChangeLog:
2019-11-09 Janne Blomqvist
PR fortran/91828
* doc/install.texi: Document that the minimum MPFR version is
3.1.0.
gcc/fortran/ChangeLog:
2019-11-09 Janne Blomqvist
PR fortran/91828
* simplify.c (gfc_simplify_fraction): Remove fallback path for
MPFR < 3.1.0.
---
configure.ac | 6 +++---
gcc/doc/install.texi | 2 +-
gcc/fortran/simplify.c | 37 -
3 files changed, 4 insertions(+), 41 deletions(-)
diff --git a/configure.ac b/configure.ac
index b8ce2ad20b9..d63a8bae940 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1601,12 +1601,12 @@ if test -d ${srcdir}/gcc && test "x$have_gmp" = xno;
then
AC_MSG_CHECKING([for the correct version of mpfr.h])
AC_TRY_COMPILE([#include
#include ],[
-#if MPFR_VERSION < MPFR_VERSION_NUM(2,4,0)
+#if MPFR_VERSION < MPFR_VERSION_NUM(3,1,0)
choke me
#endif
], [AC_TRY_COMPILE([#include
#include ],[
-#if MPFR_VERSION < MPFR_VERSION_NUM(2,4,2)
+#if MPFR_VERSION < MPFR_VERSION_NUM(3,1,6)
choke me
#endif
], [AC_MSG_RESULT([yes])], [AC_MSG_RESULT([buggy but acceptable])])],
@@ -1661,7 +1661,7 @@ if test -d ${srcdir}/gcc && test "x$have_gmp" = xno; then
# The library versions listed in the error message below should match
# the HARD-minimums enforced above.
if test x$have_gmp != xyes; then
-AC_MSG_ERROR([Building GCC requires GMP 4.2+, MPFR 2.4.0+ and MPC 0.8.0+.
+AC_MSG_ERROR([Building GCC requires GMP 4.2+, MPFR 3.1.0+ and MPC 0.8.0+.
Try the --with-gmp, --with-mpfr and/or --with-mpc options to specify
their locations. Source code for these libraries can be found at
their respective hosting sites as well as at
diff --git a/gcc/doc/install.texi b/gcc/doc/install.texi
index 2cb8a342a2c..93b01ff7971 100644
--- a/gcc/doc/install.texi
+++ b/gcc/doc/install.texi
@@ -384,7 +384,7 @@ and @option{--with-gmp-include}.
The in-tree build is only supported with the GMP version that
download_prerequisites installs.
-@item MPFR Library version 2.4.2 (or later)
+@item MPFR Library version 3.1.0 (or later)
Necessary to build GCC@. It can be downloaded from
@uref{https://www.mpfr.org}. If an MPFR source distribution is found
diff --git a/gcc/fortran/simplify.c b/gcc/fortran/simplify.c
index 2eb1943c3ee..0461d31cd88 100644
--- a/gcc/fortran/simplify.c
+++ b/gcc/fortran/simplify.c
@@ -3076,12 +3076,7 @@ gfc_expr *
gfc_simplify_fraction (gfc_expr *x)
{
gfc_expr *result;
-
-#if MPFR_VERSION < MPFR_VERSION_NUM(3,1,0)
- mpfr_t absv, exp, pow2;
-#else
mpfr_exp_t e;
-#endif
if (x->expr_type != EXPR_CONSTANT)
return NULL;
@@ -3095,41 +3090,9 @@ gfc_simplify_fraction (gfc_expr *x)
return result;
}
-#if MPFR_VERSION < MPFR_VERSION_NUM(3,1,0)
-
- /* MPFR versions before 3.1.0 do not include mpfr_frexp.
- TODO: remove the kludge when MPFR 3.1.0 or newer will be required */
-
- if (mpfr_sgn (x->value.real) == 0)
-{
- mpfr_set (result->value.real, x->value.real, GFC_RND_MODE);
- return result;
-}
-
- gfc_set_model_kind (x->ts.kind);
- mpfr_init (exp);
- mpfr_init (absv);
- mpfr_init (pow2);
-
- mpfr_abs (absv, x->value.real, GFC_RND_MODE);
- mpfr_log2 (exp, absv, GFC_RND_MODE);
-
- mpfr_trunc (exp, exp);
- mpfr_add_ui (exp, exp, 1, GFC_RND_MODE);
-
- mpfr_ui_pow (pow2, 2, exp, GFC_RND_MODE);
-
- mpfr_div (result->value.real, x->value.real, pow2, GFC_RND_MODE);
-
- mpfr_clears (exp, absv, pow2, NULL);
-
-#else
-
/* mpfr_frexp() correctly handles zeros and NaNs. */
mpfr_frexp (, result->value.real, x->value.real, GFC_RND_MODE);
-#endif
-
return range_check (result, "FRACTION");
}
--
2.17.1
Re: [Patch,committed] Ensure that gfortran.dg/achar_2.f90 can fail
On Thu, Oct 31, 2019 at 10:04 PM Steve Kargl wrote: > > On Thu, Oct 31, 2019 at 09:30:26PM +0200, Janne Blomqvist wrote: > > > > I'd personally prefer the current behavior. I.e. just let the > > underlying OS/libc handle it as it sees fit. No need to invent our own > > semantics here. Tobias quoted the relevant part of the standard, which > > the current implementation fulfills just fine. > > > > I'm fine with that. I suppose someone should > document how gfortran communicates an exit > status to an invoking shell handle; especially > when the stop codes exceeds 255. In principle yes, but how to do it without bogging down into minutiae of how different targets allow retrieving the process exit status? For POSIX, we can say that the 8 lowest order bits are used. Except if using the POSIX 2008 waitid() function which allows the parent process to retrieve the full 32 bits. And Windows apparently use 32-bit unsigned integers. And then all the weird targets that a handful of people around the world for some reason care about, etc. More info at wikipedia: https://en.wikipedia.org/wiki/Exit_status Perhaps some note that positive integers in the range [0,255] are somewhat portable? -- Janne Blomqvist
Re: [Patch,committed] Ensure that gfortran.dg/achar_2.f90 can fail
On Thu, Oct 31, 2019 at 7:03 PM Steve Kargl wrote: > > On Thu, Oct 31, 2019 at 05:51:38PM +0100, Tobias Burnus wrote: > > On 10/31/19 5:42 PM, Steve Kargl wrote: > > > I suspect the other BSDs also follow posix. I wonder if gfortran > > > should either apply a mask to the stop code or simply map nonzero > > > values to one of EXIT_FAILURE, SIGQUIT, or SIGABRT. Perhaps, > > > > > > - exit (code); > > > + exit (EXIT_FAILURE); > > > > > > Or "exit (code > 255 ? EXIT_FAILURE : code);". I think EXIT_FAILURE is 1 > > on most systems. I recall that windows interpreted exit(3) as abort, > > which can also be surprising. (But is fine for our testsuite purpose.) > > As I replied to Jakub, I'm fine with the ternary expression. > > I did a brief scan of signal.h, and there is a SIGSTOP. > Depending on whether a user installed a signal handler, > SIGSTOP might lead to a zombie process so I did not > mention it as a possibility. I'd personally prefer the current behavior. I.e. just let the underlying OS/libc handle it as it sees fit. No need to invent our own semantics here. Tobias quoted the relevant part of the standard, which the current implementation fulfills just fine. -- Janne Blomqvist
Re: [PATCH] PR fortran/91513 -- Fix poorly worded error message
On Sat, Oct 12, 2019 at 7:58 AM Steve Kargl wrote: > > On Fri, Oct 11, 2019 at 09:56:15PM -0700, Steve Kargl wrote: > > In PR fortran/91513, Damian Rouson points out that the Fortran > > does contain the words "impure variable". > > Geez. That is a messed up sentence. > > In PR fortran/91513, Damian Rouson points out that the Fortran > *standard* does *not* contain the words "impure variable. > > (It's late, and I'm tired!) > > > Damian and I spent > > an afternoon together a few weeks ago where I gave Damian a > > world wind view of how I see gcc/fortran and go about debugging > > problems. Damian and I both have full schedules, but I was > > hoping he would submit the patch to the list. My time of > > fixing gfortran bugs is running out, so ... > > > > OK to commit? > > > > 2019-09-29 Damian Rouson > > > > PR fortran/91513 > > * resolve.c (resolve_ordinary_assign): Improved error message. > > > > 2019-09-29 Damian Rouson > > > > PR fortran/91513 > > * gfortran.dg/impure_assignment_2.f90: Update dg-error regex. > > > > -- > > Steve Ok -- Janne Blomqvist
Re: [PATCH] PR fortran/91714 -- Mangled type-spec
On Sat, Sep 28, 2019 at 10:16 PM Steve Kargl
wrote:
>
> The attached patch issues errors for a few mangled type-specs.
> It has been regression tested on x86_64-*-freebsd. OK to commit?
>
> 2019-09-28 Steven G. Kargl
>
> PR fortran/91714
> * decl.c (gfc_match_decl_type_spec): Issue errors for a few
> mangled types.
>
> 2019-09-28 Steven G. Kargl
>
> PR fortran/91714
> * gfortran.dg/dec_type_print_3.f90: Update dg-error regex.
> * gfortran.dg/pr91714.f90: New test.
>
> A daunting task awaits a brave soul as gfc_match_decl_type_spec
> is a minefield for bugs. This is dues to the fact the the functions
> has grown by adding kludge upon kludge upon kludge. The first
> thing to fix is the broken parsing that follows from the
> matching of 'type('.
>
> --
> Steve
Ok.
--
Janne Blomqvist
Re: [PATCH] PR fortran/91943 -- More BOZ fallout
On Tue, Oct 1, 2019 at 1:23 AM Steve Kargl wrote: > > A BOZ cannot be an actual argument in a subroutine or function > reference except those intrinsic function listed in the Fortran > standard. The attach patch checks for invalid BOZ. Built and > tested on x86_64-*-freebsd. OK to commit? > > 2019-09-30 Steven G. Kargl > > PR fortran/91943 > * match.c (gfc_match_call): BOZ cannot be an actual argument in > a subroutine reference. > * resolve.c (resolve_function): BOZ cannot be an actual argument in > a function reference. > > 2019-09-30 Steven G. Kargl > > PR fortran/91943 > gfortran.dg/pr91943.f90 > > -- > Steve Ok, though I believe your other BOZ patch that I just reviewed applies on top of this one? -- Janne Blomqvist
Re: [PATCH] PR fortran/92018 -- BOZ the gift that keeps giving
On Wed, Oct 9, 2019 at 2:14 AM Steve Kargl wrote: > > Tested on x86_64-*-freebsd. OK to commit? > > A BOZ literal constant can be an actual argument in a > very limited number of intrinsic subprograms. For those > intrinsics subprograms, the BOZ literal constant is converted > either during checking (see check.c) or simplification > (see simplify.c). In resolve.c (resolve_function), I added > code that would walk the actual argument list to check for a > BOZ, but that code was restricted to functions with the EXTERNAL > attribute. > > The new testcase, pr92018.f90, demonstrates a situation > when neither the INTRINSIC and EXTERNAL attribute is set, > and the actual argument list contains BOZ. This led to > an ICE. The patch removes the previous restriction, and > so the actual arguments for all functions are checked. > This works except it pointed to a deficiency in the checking > routines. If something was rejected, (e.g., IAND(Z'12',Z34')), > the BOZ were passed onto resolve_function() and run-on errors > were reported. To avoid these additional error messages, I have > added the reset_boz() function, which converts a rejected > BOZ to a default integer kind 0. > > 2019-10-09 Steven G. Kargl > > PF fortran/92018 > * check.c (reset_boz): New function. > (illegal_boz_arg, boz_args_check, gfc_check_complex, gfc_check_float, > gfc_check_transfer): Use it. > (gfc_check_dshift): Use reset_boz, and re-arrange the checking to > help suppress possible run-on errors. > (gfc_check_and): Restore checks for valid argument types. Use > reset_boz, and re-arrange the checking to help suppress possible > run-on errors. > * resolve.c (resolve_function): Actual arguments cannot be BOZ in > a function reference. > > 2019-10-09 Steven G. Kargl > > PF fortran/92018 > * gfortran.dg/gnu_logical_2.f90: Update dg-error regex. > * gfortran.dg/pr81509_2.f90: Ditto. > * gfortran.dg/pr92018.f90: New test. > > -- > Steve Ok. -- Janne Blomqvist
Re: [PATCH] PR fortran/92019 -- BOZ cannot be an array index
On Wed, Oct 9, 2019 at 11:26 PM Steve Kargl wrote: > > The attach patch fixes an ICE caused by the use of a bOZ as > an array index. Tested on x86_64-*-freebsd. OK to commit? > > 2019-10-09 Steven G. Kargl > > PR fortran/92019 > * array.c (match_subscript): BOZ cannot be an array subscript. > > 2019-10-09 Steven G. Kargl > > PR fortran/92019 > * gfortran.dg/pr92019.f90: New test. > > -- > Steve Ok. -- Janne Blomqvist
Re: [patch][OpenMP,Fortran] Fix several OpenMP use_device_addr/map/update errors found by a length test case
On Thu, Oct 10, 2019 at 7:50 PM Tobias Burnus wrote: > > Hi Jakub, > > On 10/10/19 1:55 PM, Jakub Jelinek wrote: > > What worries me about the test is that the officially only portable way > > to use it in a target region is is_device_ptr. > > How about the attached test cases? The only difference is in "module > target_procs". As previously mentioned, please use "stop N" instead of "call abort()". -- Janne Blomqvist
Re: [patch, libgfortran] Bug 91593 - Implicit enum conversions in libgfortran/io/transfer.c
On Fri, Sep 27, 2019 at 8:14 PM Jerry DeLisle wrote: > > On 9/23/19 8:12 PM, Jerry DeLisle wrote: > > On 9/23/19 8:52 AM, Bernhard Reutner-Fischer wrote: > >> On 22 September 2019 22:51:46 CEST, Jerry DeLisle > >> wrote: > >>> Hi all, > >>> > >>> The attached patch eliminates several warnings by adjusting which > >>> enumerator is > >>> used in the subject offending code. I fixed this by adding an > >>> enumerator at the > >>> end of the file_mode definition. This then triggered a warning in > >>> several other > >>> places for an unhandled case in the switch statements. I cleared those > >>> by > >>> throwing in an assert (false) since it cant happen unless something > >>> really goes > >>> wrong somehow. > >>> > >> I'm curious why you assert (false) instead of the usual gcc_unreachable ()? > >> > >> Thanks, > >> > > > > Because I forgot all about gcc_unreachable. I will give it a try. > > > > Jerry > > gcc_unreachable is only defined in the gfortran frontend and not the runtime. > Therefore, I added a define to io.h which invokes __builtin_unreachable and > does > not use fancy_abort. I don't think we need anything fancy. > > If no objections, I will commit the attached updated patch with a new > ChangeLog. Just a minor nit, why bother with the #define, why not just use __builtin_unreachable directly? (I think for the frontend there's the argument that it might be compiled with a non-GCC compiler which might not support __builtin_unreachable. But libgfortran is always compiled with the corresponding gcc frontend, so this doesn't apply there.) -- Janne Blomqvist
Re: [PATCH, Fortran] Character type names in errors and warnings - new version for review
On Thu, Sep 19, 2019 at 3:59 PM Mark Eggleston wrote: > > Original thread: https://gcc.gnu.org/ml/fortran/2019-09/msg00024.html > > The original patch introduced a new field in gfc_typespec called length > to be used only for character literals. At the time I felt that this was > a bit of kludge. As a results of comments from Janne Blomqvist I > investigated whether the existing mechanism for character length in > gfc_typespec could be used for character literals. This turn out to be > impractical. > > The character length for literals is already held in the gfc_expr > structure for character constants. I've added a new version of > gfc_typename that accepts gfc_expr * instead of gfc_typespec. Where > character types are possible the gfc_expr * version is now used instead > of the gfc_typespec * version. > > I've implemented Janne's suggestions. > > I think this is a better solution. > > Please review. > > Tested on x86_64 (built with bootstrap). Thanks, this is Ok. -- Janne Blomqvist
Re: Proposal for the transition timetable for the move to GIT
On Thu, Sep 19, 2019 at 5:43 PM Damian Rouson
wrote:
>
>
>
> On Thu, Sep 19, 2019 at 5:04 AM Janne Blomqvist
> wrote:
>>
>>
>> One thing that's unclear to me is how should I actually make my stuff
>> appear in the public repo? Say I want to work on some particular
>> thing:
>
>
> This is essentially a git workflow question.
Yes. What I'm saying is we ("we" as in whoever is responsible for the
git conversion, or the gcc development community in general) should
have some kind of workflow documented. Doesn't have to be an academic
paper or a best-seller book written by some self-styled "thought
leader", but there should be some guidance. A page in the wiki or in
wwwdocs is good enough for me.
> A simple and useful workflow to consider is the
> GitHub Flow: https://guides.github.com/introduction/flow/. Others to
> consider are on the
> GitLab Flow page: https://docs.gitlab.com/ee/workflow/gitlab_flow.html and on
> Atlassian's
> Git Flow page: https://docs.gitlab.com/ee/workflow/gitlab_flow.html. Where
> will the GCC
> git repository be hosted?
Yes, I'm aware (though I do think gitflow is a bit too overcomplicated
for its own good, but YMMV).
>> 1. git checkout -b pr1234-foo # A private branch based on latest trunk
>> 2. Then when I'm happy, I send out a patch for review, either manually
>> or with git format-patch + send-email.
>
>
> Will GCC allow workflows other than emailing patches? It could make
> contributing more
> inviting to new developers. A large community of developers has grown up
> around the
> above workflows and are used to using the related tools. I realize emailing
> patches
> probably seems simple to GCC developers, but that practice is one of the main
> reasons I
> haven't contributed code to GCC even though I have supported GCC development
> financially
> and I frequently interact with GCC developers. My problems with email have
> been many.
> I have often forgotten to set my emails to plain text so my emails to GCC
> lists bounce and
> I have to resend them (often hours later if I didn't see the bounce right
> away). When I
> receive patches from GCC developers, I get frustrated with determining what
> -p argument
> to pass when applying the patch. I'm equally daunted with the process of
> searching through
> emails to find related discussions rather than having all the dialogue about
> a pull request
> (which contains the same information as a patch) in one place. And with
> plain-text emails
> as the medium, I really miss the ability to format dialogues with Markdown,
> including inserting
> hyperlinks but also to tie comments to specific lines of code in a browser
> interface to the
> pull request, etc.
I do see the attractiveness of these kinds of tools, however as the
original message in this thread stated, at this point we have enough
to chew on just getting the git transition done. Spending another year
(or more!) bikeshedding various other workflow improvements to tack on
to the git transition would be a mistake. After the git transition is
done and the smoke has settled, we can start thinking whether we want
to move away from the current email-based workflow.
As for the email-based workflow, the nice thing with git is that it
has nice support for it, via the format-patch, send-email, am, and
apply commands. So at least it will be an improvement upon the current
svn-based workflow.
>> 3. Patch goes through a few revisions, and is approved.
>> 4. Now what?
>> 4a) Do I merge my private branch to master (err, trunk?), then commit and
>> push?
>
>
> It's safer to first merge master into your branch and then retest with all
> the new commits
> that have hit master since you branched. If you test right after merging and
> find no
> problems (and no new commits hit master while you're testing), then the head
> of your
> branch will reflect the state master will reach when you merge into master so
> you know
> it's safe to do so.
Ugh. Merging master into your branch and then merging your branch back
into master makes the history somewhat convoluted, IMHO.
> A lot of the testing can be automated. For example, on GitHub, git hooks can
> be set up
> to ensure that if a branch has an open pull request against master (or other
> designated
> branches), tests run for that branch every time a new commit is pushed to it.
Sure. Again, something to look into once the git transition itself is
done, IMHO.
--
Janne Blomqvist
Re: Proposal for the transition timetable for the move to GIT
On Tue, Sep 17, 2019 at 3:02 PM Richard Earnshaw (lists) wrote: > There should be NO CHANGE to the other processes and policies that we > have, eg patch reviews, ChangeLog policies etc at this time. Adding > requirements for this will just slow down the transition by > over-complicating things. A little aside; I fully support the above, lets change one thing at a time. But it would be nice with some short documentation about the git workflow that we'll start with (which, presumably, at least initially shouldn't differ too much from the svn workflow many are familiar with for the reasons you mention above), particularly for those not that familiar with git, or have only used git together with github or such. One thing that's unclear to me is how should I actually make my stuff appear in the public repo? Say I want to work on some particular thing: 1. git checkout -b pr1234-foo # A private branch based on latest trunk 2. Then when I'm happy, I send out a patch for review, either manually or with git format-patch + send-email. 3. Patch goes through a few revisions, and is approved. 4. Now what? 4a) Do I merge my private branch to master (err, trunk?), then commit and push? 4b) Or do I first rebase my branch on top of the latest master, to produce a slightly less branchy history? 4c) Or do I (manually?) apply my patch on master, to create a linear history? 4d) Something else entirely? Thanks, -- Janne Blomqvist
Re: [PATCH] Fix PR fortran/91716
On Fri, Sep 13, 2019 at 1:07 PM Bernd Edlinger wrote: > > Hi, > > this fixes a test case where a short string constant is put in a larger > memory object. > > The consistency check in varasm.c is failed because both types should agree. > > Since the failed assertion is just a gcc_checking_assert I think a back-port > of this fix > to the gcc-9 branch will not be necessary. > > > Bootstrapped and reg-tested on x86_64-pc-linux-gnu. > Is it OK for trunk? > > > Thanks > Bernd. Ok. -- Janne Blomqvist
Re: [PATCH] Fortran - character type names in errors and warning - for review
On Mon, Sep 9, 2019 at 4:52 PM Mark Eggleston wrote: > To work around these problems I added a new length field to gfc_typespec > to used to produce the name of a character type if the character length > structure is not present. > The addition of the length field is a bit of kludge any pointers > regarding a better solution will be appreciated. Thanks for the patch, I agree that we should print character type names better. However, I'm not really happy with this approach. Requiring us to keep track of the character length in two places sounds like a recipe for confusing bugs. I don't really have a good solution thought out for this, but I think this should be solved somehow before committing the patch. Secondly, character lengths can be longer than what fits into int. In gfortran.h you'll find typedef HOST_WIDE_INT gfc_charlen_t; and then you should use gfc_mpz_get_hwi() instead of mpz_get_si(). And for the *printf() format string you should use HOST_WIDE_INT_PRINT_DEC. Thanks, -- Janne Blomqvist
Re: Deprecate -traditional-cpp?
On Thu, Sep 5, 2019 at 2:08 PM Nathan Sidwell wrote: > > Is it time to deprecate traditional preprocessing? It's been 30 years > since C89. Are (non-compiler) tools that use it still things? > > Handling it gets its hooks into a bunch of odd places in libcpp. > > To be specific: deprecate -traditional-cpp for GCC10, remove in GCC11. The Fortran frontend uses traditional mode, for reasons I can't recall at the moment. -- Janne Blomqvist
Re: [PATCH] PR fortran/91660 -- Improve and restore error messages
On Thu, Sep 5, 2019 at 7:51 AM Steve Kargl wrote: > > Built and regression tested on x86_64-*-freebsd. > > The patch restores an improved error message for a malformed > type-spec. See pr91660_1.f90 for code demonstrating problem. > While here, improve the error messages for other malformed > type-specs. Sett pr91660_2.f90. > > OK to commit? Ok. -- Janne Blomqvist
[PATCH,Fortran] Improve PRNG jumping when using threads
Currently, when a new thread needs to use the RANDOM_NUMBER intrinsic,
the per-thread PRNG state is initialized by copying the master state
and then jumping forwards N*2**128 entries in the stream so that the
PRNG streams for different threads don't alias each other, where N is
the number of threads that have so far initialized the PRNG.
With this patch the master state itself is jumped forwards once each
time a new thread initializes the PRNG, thus obviating the need to
jump through all the N-1 previous streams. Effectively turning an O(N)
algorithm into an O(1) one.
Regtested on x86_64-pc-linux-gnu, Ok for trunk?
libgfortran/ChangeLog:
2019-09-04 Janne Blomqvist
* intrinsics/random.c (master_init): Replace with
master_state.init.
(njumps): Remove variable.
(master_state): Make instance of struct prng_state.
(init_rand_state): When jumping, update the master_state once
instead of keeping track of how many jumps need to be done.
(SZU64): Modify to handle new master_state.
(SZ): Likewise.
(random_seed_i4): Likewise.
(random_seed_i8): Likewise.
---
libgfortran/intrinsics/random.c | 46 ++---
1 file changed, 19 insertions(+), 27 deletions(-)
diff --git a/libgfortran/intrinsics/random.c b/libgfortran/intrinsics/random.c
index dd2c46e7ef5..bdad208bd98 100644
--- a/libgfortran/intrinsics/random.c
+++ b/libgfortran/intrinsics/random.c
@@ -194,13 +194,10 @@ typedef struct
prng_state;
-/* master_init, njumps, and master_state are the only variables
- protected by random_lock. */
-static bool master_init;
-static unsigned njumps; /* How many times we have jumped. */
-static uint64_t master_state[] = {
- 0xad63fa1ed3b55f36ULL, 0xd94473e78978b497ULL, 0xbc60592a98172477ULL,
- 0xa3de7c6e81265301ULL
+/* master_state is the only variable protected by random_lock. */
+static prng_state master_state = { .init = false, .s = {
+0xad63fa1ed3b55f36ULL, 0xd94473e78978b497ULL, 0xbc60592a98172477ULL,
+0xa3de7c6e81265301ULL }
};
@@ -353,24 +350,21 @@ init_rand_state (prng_state* rs, const bool locked)
{
if (!locked)
__gthread_mutex_lock (_lock);
- if (!master_init)
+ if (!master_state.init)
{
uint64_t os_seed;
getosrandom (_seed, sizeof (os_seed));
- for (uint64_t i = 0; i < sizeof (master_state) / sizeof (uint64_t); i++)
+ for (uint64_t i = 0; i < sizeof (master_state.s) / sizeof (uint64_t);
i++)
{
os_seed = splitmix64 (os_seed);
- master_state[i] = os_seed;
+ master_state.s[i] = os_seed;
}
- njumps = 0;
- master_init = true;
+ master_state.init = true;
}
- memcpy (>s, master_state, sizeof (master_state));
- unsigned n = njumps++;
+ memcpy (>s, master_state.s, sizeof (master_state.s));
+ jump (_state);
if (!locked)
__gthread_mutex_unlock (_lock);
- for (unsigned i = 0; i < n; i++)
-jump (rs);
rs->init = true;
}
@@ -727,7 +721,7 @@ arandom_r16 (gfc_array_r16 *x)
/* Number of elements in master_state array. */
-#define SZU64 (sizeof (master_state) / sizeof (uint64_t))
+#define SZU64 (sizeof (master_state.s) / sizeof (uint64_t))
/* Keys for scrambling the seed in order to avoid poor seeds. */
@@ -757,7 +751,7 @@ void
random_seed_i4 (GFC_INTEGER_4 *size, gfc_array_i4 *put, gfc_array_i4 *get)
{
uint64_t seed[SZU64];
-#define SZ (sizeof (master_state) / sizeof (GFC_INTEGER_4))
+#define SZ (sizeof (master_state.s) / sizeof (GFC_INTEGER_4))
/* Check that we only have one argument present. */
if ((size ? 1 : 0) + (put ? 1 : 0) + (get ? 1 : 0) > 1)
@@ -800,7 +794,7 @@ random_seed_i4 (GFC_INTEGER_4 *size, gfc_array_i4 *put,
gfc_array_i4 *get)
a processor-dependent value to the seed." */
if (size == NULL && put == NULL && get == NULL)
{
- master_init = false;
+ master_state.init = false;
init_rand_state (rs, true);
}
@@ -822,9 +816,8 @@ random_seed_i4 (GFC_INTEGER_4 *size, gfc_array_i4 *put,
gfc_array_i4 *get)
/* We put it after scrambling the bytes, to paper around users who
provide seeds with quality only in the lower or upper part. */
- scramble_seed (master_state, seed);
- njumps = 0;
- master_init = true;
+ scramble_seed (master_state.s, seed);
+ master_state.init = true;
init_rand_state (rs, true);
}
@@ -844,7 +837,7 @@ random_seed_i8 (GFC_INTEGER_8 *size, gfc_array_i8 *put,
gfc_array_i8 *get)
if ((size ? 1 : 0) + (put ? 1 : 0) + (get ? 1 : 0) > 1)
runtime_error ("RANDOM_SEED should have at most one argument present.");
-#define SZ (sizeof (master_state) / sizeof (GFC_INTEGER_8))
+#define SZ (sizeof (master_state.s) / sizeof (GFC_INTEGER_8))
if (size != NULL)
*size = SZ;
@@ -881,7 +874,7 @@ random_seed_i8 (GFC_INTEGER_8 *size, gfc_array_i8 *put,
gfc_array
Re: [PATCH] PR fortran/91551 -- ALLOCATED has one argument
On Wed, Aug 28, 2019 at 3:43 AM Steve Kargl wrote: > > The attach patch was built and tested on i586-*-freebsd. > It includes a check for ALLOCATED with no arguments. > OK to commit? Ok. -- Janne Blomqvist
Re: [PATCH] PR fortran/91565 -- Extra checks on ORDER
On Wed, Aug 28, 2019 at 1:01 AM Steve Kargl wrote: > > The attached ptch implements additional checks on the > ORDER dummy argument for the RESHAPE intrinsic function. > Built and regression tested on x86_64-*-freebsd. OK to > commit? Ok. -- Janne Blomqvist
Re: [PATCH] PR fortran/91564 -- Additonal checks on STATUS
On Wed, Aug 28, 2019 at 3:37 AM Steve Kargl wrote: > > The attached patch has been built and tested on x86_64-*-freebsd. > It adds additional checks for the status dummy argument, and > therby prevents an ICE. OK to commit? Ok. -- Janne Blomqvist
Re: [patch, fortran] Introduce -fallow-argument-mismatch
On Sun, Aug 25, 2019 at 2:16 PM Thomas Koenig wrote: > > Hello world, > > attached is a patch introducing the -fallow-argument-mismatch > option, to separate this from the general kitchen sink -std=legacy. > As discussed, this can only be turned off by -w. > > Regression-tested on powerpc64le-unknown-linux-gnu. Documentation > tested by "make dvi" and "make pdf". > > OK for trunk? Ok. -- Janne Blomqvist
Re: [PATCH] PR fortran/78719 -- Check for a CLASS
On Sat, Aug 17, 2019 at 4:00 AM Steve Kargl wrote: > > Regression tested on x86_64-*-freebsd. OK to commit? > > When checking to see in attrbutes are being added to > an entity that alrady has an explcit interface, gfortran > failed to consider the case of CLASS. The attach patch > corrects this omission. See the 3 testcases for clarity. > > 2019-08-16 Steven G. Kargl > > PR fortran/78719 > * decl.c (get_proc_name): Check for a CLASS entity when trying to > add attributes to an entity that already has an explicit interface. > > 2019-08-16 Steven G. Kargl > > PR fortran/78719 > * gfortran.dg/pr78719_1.f90: New test. > * gfortran.dg/pr78719_2.f90: Ditto. > * gfortran.dg/pr78719_3.f90: Ditto. Ok, thanks. -- Janne Blomqvist
Re: [PATCH] PR fortran/91471 -- Remove a gfc_internal_error()
On Sat, Aug 17, 2019 at 2:53 AM Steve Kargl wrote: > > The attach patch has been tested on x86_64-*-freebsd. > The code, testcase, and ChangeLog should provide > sufficient information. OK to commit? > > 2019-08-16 Steven G. Kargl > > PR fortran/91471 > * primary.c (gfc_variable_attr): Remove a gfc_internal_error(), > which cannot be reached by conforming Fortran code, but seems to > be reachable from nonconforming Fortran code. Treat the AR_UNKNOWN > case as a no-op. > > 2019-08-16 Steven G. Kargl > > PR fortran/91471 > * gfortran.dg/pr91471.f90: New test. Ok. -- Janne Blomqvist
Re: [PATCH] PR fortran/78739 -- detect statement function name conflict
On Sat, Aug 17, 2019 at 2:47 AM Steve Kargl wrote: > > The attach patch checks that a statement function name > does not conflict the name of the containing function. > Regression tested on x86_64-*-freebsd. OK to commit? > > 2019-08-16 Steven G. Kargl > > PR fortran/78739 > * match.c (gfc_match_st_function): When matching a statement > function, > need to check if the statement function name shadows the function > name. > > 2019-08-16 Steven G. Kargl > > PR fortran/78739 > * fortran.dg/pr78739.f90: New test. Ok, thanks. -- Janne Blomqvist
[PATCH] PR fortran/68401 Improve allocation error message
Improve the error message that is printed when a memory allocation
fails, by including the location, and the size of the allocation that
failed.
Regtested on x86_64-pc-linux-gnu, Ok for trunk?
(libgomp.fortran/appendix-a/a.28.5.f90 fails, but that seems
unrelated)
gcc/fortran/ChangeLog:
2019-08-16 Janne Blomqvist
PR fortran/68401
* trans-decl.c (gfc_build_builtin_function_decls): Replace
os_error with os_error_at decl.
* trans.c (trans_runtime_error_vararg): Modify so the error
function decl is passed directly.
(gfc_trans_runtime_error): Pass correct error function decl.
(gfc_trans_runtime_check): Likewise.
(trans_os_error_at): New function.
(gfc_call_malloc): Use trans_os_error_at.
(gfc_allocate_using_malloc): Likewise.
(gfc_call_realloc): Likewise.
* trans.h (gfor_fndecl_os_error): Replace with gfor_fndecl_os_error_at.
libgfortran/ChangeLog:
2019-08-16 Janne Blomqvist
PR fortran/68401
* gfortran.map: Add GFORTRAN_10 node, add _gfortran_os_error_at
symbol.
* libgfortran.h (os_error_at): New prototype.
* runtime/error.c (os_error_at): New function.
---
gcc/fortran/trans-decl.c| 12 +++
gcc/fortran/trans.c | 68 ++---
gcc/fortran/trans.h | 2 +-
libgfortran/gfortran.map| 5 +++
libgfortran/libgfortran.h | 4 +++
libgfortran/runtime/error.c | 46 -
6 files changed, 102 insertions(+), 35 deletions(-)
diff --git a/gcc/fortran/trans-decl.c b/gcc/fortran/trans-decl.c
index 2a9b852568a..3c6ab60e9b2 100644
--- a/gcc/fortran/trans-decl.c
+++ b/gcc/fortran/trans-decl.c
@@ -102,7 +102,7 @@ tree gfor_fndecl_error_stop_string;
tree gfor_fndecl_runtime_error;
tree gfor_fndecl_runtime_error_at;
tree gfor_fndecl_runtime_warning_at;
-tree gfor_fndecl_os_error;
+tree gfor_fndecl_os_error_at;
tree gfor_fndecl_generate_error;
tree gfor_fndecl_set_args;
tree gfor_fndecl_set_fpe;
@@ -3679,11 +3679,11 @@ gfc_build_builtin_function_decls (void)
void_type_node, 3, pvoid_type_node, integer_type_node,
pchar_type_node);
- gfor_fndecl_os_error = gfc_build_library_function_decl_with_spec (
- get_identifier (PREFIX("os_error")), ".R",
- void_type_node, 1, pchar_type_node);
- /* The runtime_error function does not return. */
- TREE_THIS_VOLATILE (gfor_fndecl_os_error) = 1;
+ gfor_fndecl_os_error_at = gfc_build_library_function_decl_with_spec (
+ get_identifier (PREFIX("os_error_at")), ".RR",
+ void_type_node, -2, pchar_type_node, pchar_type_node);
+ /* The os_error_at function does not return. */
+ TREE_THIS_VOLATILE (gfor_fndecl_os_error_at) = 1;
gfor_fndecl_set_args = gfc_build_library_function_decl (
get_identifier (PREFIX("set_args")),
diff --git a/gcc/fortran/trans.c b/gcc/fortran/trans.c
index 84511477b39..583f6e3b25b 100644
--- a/gcc/fortran/trans.c
+++ b/gcc/fortran/trans.c
@@ -447,7 +447,7 @@ gfc_build_array_ref (tree base, tree offset, tree decl,
tree vptr)
arguments and a locus. */
static tree
-trans_runtime_error_vararg (bool error, locus* where, const char* msgid,
+trans_runtime_error_vararg (tree errorfunc, locus* where, const char* msgid,
va_list ap)
{
stmtblock_t block;
@@ -501,18 +501,13 @@ trans_runtime_error_vararg (bool error, locus* where,
const char* msgid,
/* Build the function call to runtime_(warning,error)_at; because of the
variable number of arguments, we can't use build_call_expr_loc
dinput_location,
irectly. */
- if (error)
-fntype = TREE_TYPE (gfor_fndecl_runtime_error_at);
- else
-fntype = TREE_TYPE (gfor_fndecl_runtime_warning_at);
+ fntype = TREE_TYPE (errorfunc);
loc = where ? where->lb->location : input_location;
tmp = fold_build_call_array_loc (loc, TREE_TYPE (fntype),
fold_build1_loc (loc, ADDR_EXPR,
build_pointer_type (fntype),
-error
-? gfor_fndecl_runtime_error_at
-: gfor_fndecl_runtime_warning_at),
+errorfunc),
nargs + 2, argarray);
gfc_add_expr_to_block (, tmp);
@@ -527,7 +522,10 @@ gfc_trans_runtime_error (bool error, locus* where, const
char* msgid, ...)
tree result;
va_start (ap, msgid);
- result = trans_runtime_error_vararg (error, where, msgid, ap);
+ result = trans_runtime_error_vararg (error
+ ? gfor_fndecl_runtime_error_at
+ : gfor_fndecl_runtime_warning_at,
+ where, msgid, ap);
va_end (ap);
return result;
}
@@ -566,8 +564,10 @@ g
Re: [patch, fortran] Fix PR 91443
On Thu, Aug 15, 2019 at 2:35 PM Thomas Koenig wrote: > > Hello world, > > this patch fixes PR 91443, in which we did not warn about a mismatched > external procedure. The problem was that the module this was called in > was resolved before parsing of the procedure ever started. > > The approach taken here is to move the checking of external procedures > to a stage after normal resolution. And, of course, fix the resulting > fallout from regression-testing :-) > There is also one policy change in the patch. Previously, we only warned > about mismatched declarations. Now, this is a hard error unless the > user specifies -std=legacy. The reason is that we have not yet solved > our single declaration problem, but it cannot be solved unless all > of a procedure's callers match. People who have such broken code > should at least be made aware that they have a problem. However, I would > like to have some sort of agreement on this point before the patch > is committed. This can also be changed (see the code at the bottom > of frontend-passes.c). Personally, I'm fine with making this a hard error. As we've recently seen with the varargs & missing charlengths in LAPACK saga, mismatches can cause extremely subtle issues that can cause silent corruption and take an expert to figure out. > Once this is in, the next step is to issue errors for mismatching > calls where the callee is not in the same file. This can be done > with the infrastructure of this patch. > > So, OK for trunk? The patch itself looks Ok. One worry, are you introducing an O(N**2)(?) algorithm (looping over all symbols for every symbol?), and does this cause performance issues when compiling some gigantic F77 project? If this worry is unfounded, Ok for trunk. -- Janne Blomqvist
Re: [PATCH] PR fortran/91414: Improved PRNG
On Mon, Aug 12, 2019 at 11:23 PM Steve Kargl
wrote:
>
> On Sun, Aug 11, 2019 at 12:37:49PM +0300, Janne Blomqvist wrote:
> > Update the PRNG from xorshift1024* to xoshiro256** by the same
> > author. For details see
> >
> > http://prng.di.unimi.it/
> >
> > and the paper at
> >
> > https://arxiv.org/abs/1805.01407
> >
> > Also the seeding is slightly improved, by reading only 8 bytes from
> > the operating system and using the simple splitmix64 PRNG to fill in
> > the rest of the PRNG state (as recommended by the xoshiro author),
> > instead of reading the entire state from the OS.
> >
> > Regtested on x86_64-pc-linux-gnu, Ok for trunk?
> >
>
> Looks good to me.
Thanks, committed to trunk (with a minor bugfix I noticed). I
backported the initialization changes to the gcc9 and gcc8 branches as
well with the attached patch.
--
Janne Blomqvist
From 16abae420bcff75950868a30327bf7b242e0388e Mon Sep 17 00:00:00 2001
From: Janne Blomqvist
Date: Tue, 13 Aug 2019 10:35:05 +0300
Subject: [PATCH] PR fortran/91414 Improve initialization of PRNG
As part of PR 91414 an improved PRNG was contributed to trunk. This is
a partial backport of some related changes to the PRNG. Namely when
seeding the PRNG, it needs only 8 bytes of randomness from the OS, and
uses a simple splitmix64 PRNG to fill in the rest of the state,
instead of getting all the state from the OS. This can be useful for
operating systems that can run out of entropy.
libgfortran/ChangeLog:
2019-08-13 Janne Blomqvist
Partial backport from trunk
PR fortran/91414
* intrinsics/random.c (lcg_parkmiller): Replace with splitmix64.
(splitmix64): New function.
(getosrandom): Fix return value, simplify.
(init_rand_state): Use getosrandom only to get 8 bytes, splitmix64
to fill rest of state.
---
libgfortran/intrinsics/random.c | 51 ++---
1 file changed, 21 insertions(+), 30 deletions(-)
diff --git a/libgfortran/intrinsics/random.c b/libgfortran/intrinsics/random.c
index 7476439647c..9283477482f 100644
--- a/libgfortran/intrinsics/random.c
+++ b/libgfortran/intrinsics/random.c
@@ -275,30 +275,19 @@ jump (xorshift1024star_state* rs)
}
-/* Super-simple LCG generator used in getosrandom () if /dev/urandom
- doesn't exist. */
+/* Splitmix64 recommended by xorshift author for initializing. After
+ getting one uint64_t value from the OS, this is used to fill in the
+ rest of the state. */
-#define M 2147483647 /* 2^31 - 1 (A large prime number) */
-#define A 16807 /* Prime root of M, passes statistical tests and produces a full cycle */
-#define Q 127773 /* M / A (To avoid overflow on A * seed) */
-#define R 2836 /* M % A (To avoid overflow on A * seed) */
-
-__attribute__((unused)) static uint32_t
-lcg_parkmiller(uint32_t seed)
+static uint64_t
+splitmix64 (uint64_t x)
{
-uint32_t hi = seed / Q;
-uint32_t lo = seed % Q;
-int32_t test = A * lo - R * hi;
-if (test <= 0)
-test += M;
-return test;
+ uint64_t z = (x += 0x9e3779b97f4a7c15);
+ z = (z ^ (z >> 30)) * 0xbf58476d1ce4e5b9;
+ z = (z ^ (z >> 27)) * 0x94d049bb133111eb;
+ return z ^ (z >> 31);
}
-#undef M
-#undef A
-#undef Q
-#undef R
-
/* Get some random bytes from the operating system in order to seed
the PRNG. */
@@ -315,7 +304,7 @@ getosrandom (void *buf, size_t buflen)
#else
#ifdef HAVE_GETENTROPY
if (getentropy (buf, buflen) == 0)
-return 0;
+return buflen;
#endif
int flags = O_RDONLY;
#ifdef O_CLOEXEC
@@ -328,7 +317,7 @@ getosrandom (void *buf, size_t buflen)
close (fd);
return res;
}
- uint32_t seed = 1234567890;
+ uint64_t seed = 0x047f7684e9fc949dULL;
time_t secs;
long usecs;
if (gf_gettime (, ) == 0)
@@ -340,13 +329,9 @@ getosrandom (void *buf, size_t buflen)
pid_t pid = getpid();
seed ^= pid;
#endif
- uint32_t* ub = buf;
- for (size_t i = 0; i < buflen / sizeof (uint32_t); i++)
-{
- ub[i] = seed;
- seed = lcg_parkmiller (seed);
-}
- return buflen;
+ size_t size = buflen < sizeof (uint64_t) ? buflen : sizeof (uint64_t);
+ memcpy (buf, , size);
+ return size;
#endif /* __MINGW64_VERSION_MAJOR */
}
@@ -361,7 +346,13 @@ init_rand_state (xorshift1024star_state* rs, const bool locked)
__gthread_mutex_lock (_lock);
if (!master_init)
{
- getosrandom (master_state, sizeof (master_state));
+ uint64_t os_seed;
+ getosrandom (_seed, sizeof (os_seed));
+ for (uint64_t i = 0; i < sizeof (master_state) / sizeof (uint64_t); i++)
+ {
+ os_seed = splitmix64 (os_seed);
+ master_state[i] = os_seed;
+}
njumps = 0;
master_init = true;
}
--
2.17.1
Re: [PATCH] PR fortran/87993 -- An array can have a kind type inquiry suffix
On Tue, Aug 13, 2019 at 3:35 AM Steve Kargl wrote: > > The attached patch ahs been regression tested on x86_64-*-freebsd. > It probably borders on obvious, but I'll ask away. OK to commit? > > 2019-08-12 Steven G. Kargl > > PR fortran/87993 > * expr.c (gfc_simplify_expr): Simplifcation of an array with a kind > type inquiry suffix yields a constant expression. > > 2019-08-12 Steven G. Kargl > > PR fortran/87993 > * gfortran.dg/pr87993.f90: New test. Ok. -- Janne Blomqvist
Re: [PATCH] PR fortran/89647 -- Allow host associated procedure to be a binding target
On Tue, Aug 13, 2019 at 1:32 AM Steve Kargl wrote: > > The attached patch fixes PR fortran/89647, and has been > regression tested on x86_64-*-freebsd. > > If a procedure is made accessible by host association, the > that procedure can be used as binding target. During the > resolution of a type bound procedure, gfortran needs to > check the parent namespace for a procedure before it fails. > > 2019-08-12 Steven G. Kargl > > PF fortran/89647 > resolve.c (resolve_typebound_procedure): Allow host associated > procedure to be a binding target. While here, wrap long line. > > 2019-08-12 Steven G. Kargl > > PF fortran/89647 > * gfortran.dg/pr89647.f90: New test. > > OK to commit? Ok, thanks. -- Janne Blomqvist
Re: [PATCH,fortran] PR 91413 Generate warning when making array static
On Sat, Aug 10, 2019 at 11:57 PM Steve Kargl wrote: > > On Sat, Aug 10, 2019 at 11:34:15PM +0300, Janne Blomqvist wrote: > > When moving a local variable from the stack to static storage, the > > procedure is no longer safe to be called recursively or concurrently > > from multiple threads. Thus generate a warning when this is done. > > Also double the default limit for switching from stack to static. > > > > Regtested on x86_64-pc-linux-gnu, Ok for trunk? > > > > OK. Thanks for the quick review, committed as r274264. To be clear, this patch does not fix the PR, just makes it slightly less likely to happen, and makes some noise when it does happen. To reignite the discussion from https://gcc.gnu.org/ml/fortran/2017-12/msg00082.html , any opinions how this should properly be fixed? I can see two main options: - Use the heap instead of static storage for local variables going over the size limit set by -fmax-stack-var-size= . This is IMHO a bit ugly, going behind the back of the user like this; If the user wants to use the heap, she can use allocatable arrays. Also, it needs to be done somewhere else than in trans-decl.c (gfc_finish_var_decl) as at that point there is no access to the block, and hence one cannot add new code to malloc() and free() the variable. I'm not sure where the proper place for this would be. - Make -frecursive the default for GFC_STD_F2018 (and thus also for -std=gnu), while keeping the existing behavior for older standards. This would follow the wishes of the user, but would risk crashing applications with stack exhaustion. Any opinions which would be preferable, or any other solution to this problem? -- Janne Blomqvist
[PATCH] PR fortran/91414: Improved PRNG
Update the PRNG from xorshift1024* to xoshiro256** by the same
author. For details see
http://prng.di.unimi.it/
and the paper at
https://arxiv.org/abs/1805.01407
Also the seeding is slightly improved, by reading only 8 bytes from
the operating system and using the simple splitmix64 PRNG to fill in
the rest of the PRNG state (as recommended by the xoshiro author),
instead of reading the entire state from the OS.
Regtested on x86_64-pc-linux-gnu, Ok for trunk?
gcc/fortran/ChangeLog:
2019-08-11 Janne Blomqvist
PR fortran/91414
* check.c (gfc_check_random_seed): Reduce seed_size.
* intrinsic.texi (RANDOM_NUMBER): Update to match new PRNG.
gcc/testsuite/ChangeLog:
2019-08-11 Janne Blomqvist
PR fortran/91414
* gfortran.dg/random_seed_1.f90: Update to match new seed size.
libgfortran/ChangeLog:
2019-08-11 Janne Blomqvist
PR fortran/91414
* intrinsics/random.c (prng_state): Update state struct.
(master_state): Update to match new size.
(get_rand_state): Update to match new PRNG.
(rotl): New function.
(xorshift1024star): Replace with prng_next.
(prng_next): New function.
(jump): Update for new PRNG.
(lcg_parkmiller): Replace with splitmix64.
(splitmix64): New function.
(getosrandom): Fix return value, simplify.
(init_rand_state): Use getosrandom only to get 8 bytes, splitmix64
to fill rest of state.
(random_r4): Update to new function and struct names.
(random_r8): Likewise.
(random_r10): Likewise.
(random_r16): Likewise.
(arandom_r4): Liekwise.
(arandom_r8): Likewise.
(arandom_r10): Likwewise.
(arandom_r16): Likewise.
(xor_keys): Reduce size to match new PRNG.
(random_seed_i4): Update to new function and struct names, remove
special handling of variable p used in previous PRNG.
(random_seed_i8): Likewise.
---
gcc/fortran/check.c | 5 +-
gcc/fortran/intrinsic.texi | 10 +-
gcc/testsuite/gfortran.dg/random_seed_1.f90 | 7 +-
libgfortran/intrinsics/random.c | 213 +---
4 files changed, 110 insertions(+), 125 deletions(-)
diff --git a/gcc/fortran/check.c b/gcc/fortran/check.c
index 370a3c819f9..2bd8bc37556 100644
--- a/gcc/fortran/check.c
+++ b/gcc/fortran/check.c
@@ -6484,9 +6484,8 @@ gfc_check_random_seed (gfc_expr *size, gfc_expr *put,
gfc_expr *get)
mpz_t put_size, get_size;
/* Keep the number of bytes in sync with master_state in
- libgfortran/intrinsics/random.c. +1 due to the integer p which is
- part of the state too. */
- seed_size = 128 / gfc_default_integer_kind + 1;
+ libgfortran/intrinsics/random.c. */
+ seed_size = 32 / gfc_default_integer_kind;
if (size != NULL)
{
diff --git a/gcc/fortran/intrinsic.texi b/gcc/fortran/intrinsic.texi
index f390761dc3d..3aa068dba9d 100644
--- a/gcc/fortran/intrinsic.texi
+++ b/gcc/fortran/intrinsic.texi
@@ -11792,10 +11792,10 @@ end program test_random_seed
Returns a single pseudorandom number or an array of pseudorandom numbers
from the uniform distribution over the range @math{ 0 \leq x < 1}.
-The runtime-library implements the xorshift1024* random number
-generator (RNG). This generator has a period of @math{2^{1024} - 1},
-and when using multiple threads up to @math{2^{512}} threads can each
-generate @math{2^{512}} random numbers before any aliasing occurs.
+The runtime-library implements the xoshiro256** pseudorandom number
+generator (PRNG). This generator has a period of @math{2^{256} - 1},
+and when using multiple threads up to @math{2^{128}} threads can each
+generate @math{2^{128}} random numbers before any aliasing occurs.
Note that in a multi-threaded program (e.g. using OpenMP directives),
each thread will have its own random number state. For details of the
@@ -11852,7 +11852,7 @@ called either without arguments or with the @var{PUT}
argument, the
given seed is copied into a master seed as well as the seed of the
current thread. When a new thread uses @code{RANDOM_NUMBER} for the
first time, the seed is copied from the master seed, and forwarded
-@math{N * 2^{512}} steps to guarantee that the random stream does not
+@math{N * 2^{128}} steps to guarantee that the random stream does not
alias any other stream in the system, where @var{N} is the number of
threads that have used @code{RANDOM_NUMBER} so far during the program
execution.
diff --git a/gcc/testsuite/gfortran.dg/random_seed_1.f90
b/gcc/testsuite/gfortran.dg/random_seed_1.f90
index 39c81ce51b7..a97e53059f8 100644
--- a/gcc/testsuite/gfortran.dg/random_seed_1.f90
+++ b/gcc/testsuite/gfortran.dg/random_seed_1.f90
@@ -10,11 +10,12 @@
PROGRAM random_seed_1
IMPLICIT NONE
- INTEGER, PARAMETER :: nbytes = 128
+ ! Should match sizeof(master_state) in
+ ! libgfortran/intrinsics/random.c
+ INTEGER, PARAMETER :: nby
[PATCH,fortran] PR 91413 Generate warning when making array static
When moving a local variable from the stack to static storage, the
procedure is no longer safe to be called recursively or concurrently
from multiple threads. Thus generate a warning when this is done.
Also double the default limit for switching from stack to static.
Regtested on x86_64-pc-linux-gnu, Ok for trunk?
gcc/fortran/ChangeLog:
2019-08-10 Janne Blomqvist
PR fortran/91413
* invoke.texi (-fmax-stack-var-size): Document increased default.
* options.c (gfc_post_options): Increase default stack var size to
65536 bytes.
* trans-decl.c (gfc_finish_var_decl): Generate warning when local
array moved to static storage.
---
gcc/fortran/invoke.texi | 2 +-
gcc/fortran/options.c| 2 +-
gcc/fortran/trans-decl.c | 10 ++
3 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/gcc/fortran/invoke.texi b/gcc/fortran/invoke.texi
index 3c1b2ac7a26..1039c6084c4 100644
--- a/gcc/fortran/invoke.texi
+++ b/gcc/fortran/invoke.texi
@@ -1720,7 +1720,7 @@ This option currently only affects local arrays declared
with constant
bounds, and may not apply to all character variables.
Future versions of GNU Fortran may improve this behavior.
-The default value for @var{n} is 32768.
+The default value for @var{n} is 65536.
@item -fstack-arrays
@opindex @code{fstack-arrays}
diff --git a/gcc/fortran/options.c b/gcc/fortran/options.c
index ef37cccec97..146be2f1420 100644
--- a/gcc/fortran/options.c
+++ b/gcc/fortran/options.c
@@ -437,7 +437,7 @@ gfc_post_options (const char **pfilename)
/* Set default. */
if (flag_max_stack_var_size == -2)
-flag_max_stack_var_size = 32768;
+flag_max_stack_var_size = 65536;
/* Implement -fno-automatic as -fmax-stack-var-size=0. */
if (!flag_automatic)
diff --git a/gcc/fortran/trans-decl.c b/gcc/fortran/trans-decl.c
index 86c3d3a9796..2a9b852568a 100644
--- a/gcc/fortran/trans-decl.c
+++ b/gcc/fortran/trans-decl.c
@@ -753,6 +753,16 @@ gfc_finish_var_decl (tree decl, gfc_symbol * sym)
|| sym->attr.allocatable)
&& !DECL_ARTIFICIAL (decl))
{
+ gfc_warning (OPT_Wsurprising,
+ "Array %qs at %L is larger than limit set by"
+ " %<-fmax-stack-var-size=%>, moved from stack to static"
+ " storage. This makes the procedure unsafe when called"
+ " recursively, or concurrently from multiple threads."
+ " Consider using %<-frecursive%>, or increase the"
+ " %<-fmax-stack-var-size=%> limit, or change the code to"
+ " use an ALLOCATABLE array.",
+ sym->name, >declared_at);
+
TREE_STATIC (decl) = 1;
/* Because the size of this variable isn't known until now, we may have
--
2.17.1
[PATCH] PR 53796 Make inquire(file=, recl=) conform to F2018
In my original patch to fix PR 53796 I forgot to fix the behavior for unconnected units when inquiring via filename. This patch fixes that. Regtested on x86_64-pc-linux-gnu, committed as obvious. libgfortran/ChangeLog: 2019-08-07 Janne Blomqvist PR fortran/53796 * io/inquire.c (inquire_via_filename): Set recl to -1 for unconnected units. gcc/testsuite/ChangeLog: 2019-08-07 Janne Blomqvist PR fortran/53796 * gfortran.dg/inquire_recl_f2018.f90: Test for unconnected unit with inquire via filename. --- gcc/testsuite/gfortran.dg/inquire_recl_f2018.f90 | 7 +++ libgfortran/io/inquire.c | 4 +++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/gcc/testsuite/gfortran.dg/inquire_recl_f2018.f90 b/gcc/testsuite/gfortran.dg/inquire_recl_f2018.f90 index b744e920f7b..dfb4092a45f 100644 --- a/gcc/testsuite/gfortran.dg/inquire_recl_f2018.f90 +++ b/gcc/testsuite/gfortran.dg/inquire_recl_f2018.f90 @@ -39,4 +39,11 @@ program inqrecl if (r /= -2) then STOP 5 end if + + ! Also inquire by filename for a non-opened unit is considered + ! unconnected similar to the first test. + inquire(file='unconnectedfile.txt', recl=r) + if (r /= -1) then + stop 6 + end if end program inqrecl diff --git a/libgfortran/io/inquire.c b/libgfortran/io/inquire.c index 05cfc14233f..c2174d0a307 100644 --- a/libgfortran/io/inquire.c +++ b/libgfortran/io/inquire.c @@ -706,7 +706,9 @@ inquire_via_filename (st_parameter_inquire *iqp) } if ((cf & IOPARM_INQUIRE_HAS_RECL_OUT) != 0) -*iqp->recl_out = 0; +/* F2018 (N2137) 12.10.2.26: If there is no connection, recl is + assigned the value -1. */ +*iqp->recl_out = -1; if ((cf & IOPARM_INQUIRE_HAS_NEXTREC) != 0) *iqp->nextrec = 0; -- 2.17.1
Re: [patch, libfortran] Adjust block size for libgfortran for unformatted reads
On Sun, Jul 7, 2019 at 11:13 PM Thomas Koenig wrote: > > Hello world, > > the attached patch sets the I/O block size for unformatted files to > 2**17 and makes this, and the block size for formatted files, > adjustable via environment variables. Should the default be affected by the page size (sysconf(_SC_PAGESIZE)) and/or the IO blocksize (we already fstat() a file when we open it, so we could get st_blksize for free)? Though one could of course argue those aren't particularly useful; except for power, everything uses a 4k page size (?), and the IO blocksize varies from too small (4k, ext4) to too large (1 MB, NFS (or whatever rsize/wsize is set to), or 4 MB for Lustre). > 2019-07-07 Thomas König > > PR libfortran/91030 > * gfortran.texi (GFORTRAN_BUFFER_SIZE_FORMATTED): Document > (GFORTRAN_BUFFER_SIZE_FORMATTED): Likewise. One of these should be _UNFORMATTED? -- Janne Blomqvist
Re: [patch, libfortran] Adjust block size for libgfortran for unformatted reads
On Mon, Jul 8, 2019 at 11:18 AM Manfred Schwarb wrote: > > Am 07.07.19 um 22:13 schrieb Thomas Koenig: > > Hello world, > > > > the attached patch sets the I/O block size for unformatted files to > > 2**17 and makes this, and the block size for formatted files, > > adjustable via environment variables. > > > > The main reason is that -fconvert=big-endian was quite slow on > > some HPC systems. A bigger buffer should eliminate that. Also, > > People who use unformatted files are likely to write large amounts > > of data, so this seems like a good fit. Finally, some benchmarking > > showed that 131072 seemed like a good value to use. Thanks to Jerry > > for support. > > > > I didn't change the value for formatted files because, frankly, we are > > using a lot of CPU for converting numbers there, so any gain > > negligible (unless somebody comes up with a benchmark which says > > otherwise). > > formatted write: Did you try writing to an USB stick or similar? I guess > for flash based devices anything below 64k will slow down operation. > Computers like Raspberry Pi and the like often have flash based storage, > and it is not extremely unrealistic to run fortran programs on them. Good point. If you happen to have a USB stick handy, can you try the simple C benchmark program at https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91030#c38 ? (the kernel will coalesce IO's by itself, so the granularity of IO syscalls is not necessarily the same as the actual IO to devices. Network filesystems like NFS/Lustre/GPFS may have less latitude here due to coherency requirements etc.) -- Janne Blomqvist
[PATCH, committed] PR 91048 Open ~/.mklog in string mode
Hi,
another leftover from the py3 conversion. Committed r272921 as obvious.
2019-07-02 Janne Blomqvist
PR other/91048
* mklog (read_user_info): Open ~/.mklog in string mode.
Index: mklog
===
--- mklog(revision 272920)
+++ mklog(working copy)
@@ -100,7 +100,7 @@ EMAIL = ...
mklog_conf = os.path.expanduser('~/.mklog')
if os.path.exists(mklog_conf):
attrs = {}
-f = open(mklog_conf, 'rb')
+f = open(mklog_conf)
for s in f:
if cache.match(r'^\s*([a-zA-Z0-9_]+)\s*=\s*(.*?)\s*$', s):
attrs[cache.group(1)] = cache.group(2)
--
Janne Blomqvist
Re: [PATCH] PR fortran/86587 -- PRIVATE and BIND(C) are allowed for derived type
On Thu, Jun 20, 2019 at 12:10 AM Steve Kargl wrote: > > Revision 126185 introduced ISO C Binding to gfortran. > In that revision, a check for a conflict between a > derived type with the PRIVATE attribute and BIND(C) was > introduced. After checking the F2003, F2008, and F2018 > standards, I cannot find this restriction. Thus, the > check is removed by the attached patch. Regression > checked on x86_64-*-freebsd. OK to commit? > > 2019-06-19 Steven G. Kargl > > PR fortran/86587 > * symbol.c (verify_bind_c_derived_type): Remove erroneous error > checking for BIND(C) and PRIVATE attributes. > > 2019-06-19 Steven G. Kargl > > PR fortran/86587 > * gfortran.dg/pr86587.f90: New test. > > -- > Steve Ok. -- Janne Blomqvist
Re: [PATCH] libgfortran: Use __builtin_mul_overflow in xmallocarray
On Sat, Jun 15, 2019 at 2:23 AM Steve Kargl wrote: > > On Fri, Jun 14, 2019 at 08:07:36AM -0700, Steve Kargl wrote: > > On Fri, Jun 14, 2019 at 01:08:48PM +0300, Janne Blomqvist wrote: > > > As GCC now provides builtins for doing integer overflow checking, lets > > > use it when checking for overflow in xmallocarray. > > > > > > Regtested on x86_64-pc-linux-gnu, Ok for trunk? > > > > > > > OK > > > > Just found > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65921 > > I assume that you close the PR. Oh, I had completely forgotten that I filed that one! (I won't close it yet as the frontend part is still to be done) -- Janne Blomqvist
Re: [PATCH 3/3] Enable full libgfortran library for AMD GCN
On Fri, Jun 7, 2019 at 5:41 PM Andrew Stubbs wrote: > > This patch basically reverts the previous patch to put AMD GCN in > "minimal" mode. This is Ok, provided the maintainers for the areas touched by the other 2 patches in this series Ok those. Thanks for improving Fortran offloading (given the recent Frontier supercomputer announcement, I hope we will receive more work in this area in the future). -- Janne Blomqvist
Re: [PATCH] PR fortran/89646 -- suppress spurious warnings
On Fri, Jun 14, 2019 at 1:16 AM Steve Kargl wrote: > > The attached patch suppresses the spurious warnings that > would otherwise occur for the testcase. Regression > tested cleanly on x86_64-*-freebsd. OK to commit? Ok, thanks for the patch. -- Janne Blomqvist
[PATCH] libgfortran: Use __builtin_mul_overflow in xmallocarray
As GCC now provides builtins for doing integer overflow checking, lets
use it when checking for overflow in xmallocarray.
Regtested on x86_64-pc-linux-gnu, Ok for trunk?
libgfortran/ChangeLog:
2019-06-14 Janne Blomqvist
* runtime/memory.c (SIZE_MAX):Remove macro definition.
(xmallocarray): Use __builtin_mul_overflow.
---
libgfortran/runtime/memory.c | 13 -
1 file changed, 4 insertions(+), 9 deletions(-)
diff --git a/libgfortran/runtime/memory.c b/libgfortran/runtime/memory.c
index 1a3d33b59dd..09a4ff8733f 100644
--- a/libgfortran/runtime/memory.c
+++ b/libgfortran/runtime/memory.c
@@ -26,10 +26,6 @@ see the files COPYING3 and COPYING.RUNTIME respectively. If
not, see
#include "libgfortran.h"
#include
-#ifndef SIZE_MAX
-#define SIZE_MAX ((size_t)-1)
-#endif
-
void *
xmalloc (size_t n)
@@ -52,18 +48,17 @@ void *
xmallocarray (size_t nmemb, size_t size)
{
void *p;
+ size_t prod;
if (!nmemb || !size)
-size = nmemb = 1;
-#define HALF_SIZE_T (((size_t) 1) << (__CHAR_BIT__ * sizeof (size_t) / 2))
- else if (__builtin_expect ((nmemb | size) >= HALF_SIZE_T, 0)
- && nmemb > SIZE_MAX / size)
+prod = 1;
+ else if (__builtin_mul_overflow (nmemb, size, ))
{
errno = ENOMEM;
os_error ("Integer overflow in xmallocarray");
}
- p = malloc (nmemb * size);
+ p = malloc (prod);
if (!p)
os_error ("Memory allocation failed in xmallocarray");
--
2.17.1
Re: [GSoC'19, libgomp work-stealing] Task parallelism runtime
On Wed, Jun 5, 2019 at 10:42 PM 김규래 wrote: > > On Wed, Jun 5, 2019 at 9:25 PM 김규래 wrote: > > > > > > Hi, thanks for the detailed explanation. > > > I think I now get the picture. > > > Judging from my current understanding, the task-parallelism currently > works as follows: > > > 1. Tasks are placed in a global shared queue. > > > 2. Workers consume the tasks by bashing the queue in a while loop, > just as self-scheduling (dynamic scheduling)/ > > > > > > Then the improvements including work-stealing must be done by: > > > 1. Each worker holds a dedicated task queue reducing the resource > contention. > > > 2. The tasks are distributed in a round-robin fashion > > > For nested task submission (does OpenMP support that?) you probably > > want to submit to the local queue rather than round-robin, no? > > > > Hi Janne, > > I believe you're talking about spawning child tasks within an already > running task, which I believe is supported by OpenMP. > > In this case, pushing to the local queue could introduce a deadlock if the > master thread waits on the spawned tasks. > > A short one though since work-stealing can resolve the deadlock. > > A better way to handle this is to round-robin the child tasks to the > queues excluding the queue of the thread consuming the current task. > > Then waiting on the tasks should never cause a deadlock. > > Or did I misunderstand your question? > > Maybe there is a specific reason for avoiding avoid round-robin that I > missed? > Better cache locality. Another option, which I guess starts to go out of scope of your gsoc, is parallel depth first (PDF) search (Blelloch 1999) as an alternative to work stealing. Here's a presentation about some recent work in this area, although for Julia and not OpenMP (no idea if PDF would fit with OpenMP at all): https://www.youtube.com/watch?v=YdiZa0Y3F3c -- Janne Blomqvist
Re: [GSoC'19, libgomp work-stealing] Task parallelism runtime
On Wed, Jun 5, 2019 at 9:25 PM 김규래 wrote: > > Hi, thanks for the detailed explanation. > I think I now get the picture. > Judging from my current understanding, the task-parallelism currently works > as follows: > 1. Tasks are placed in a global shared queue. > 2. Workers consume the tasks by bashing the queue in a while loop, just as > self-scheduling (dynamic scheduling)/ > > Then the improvements including work-stealing must be done by: > 1. Each worker holds a dedicated task queue reducing the resource contention. > 2. The tasks are distributed in a round-robin fashion For nested task submission (does OpenMP support that?) you probably want to submit to the local queue rather than round-robin, no? > 3. work-stealing will resolve the load imbalance. > > If the above statements are correct, I guess the task priority should be > given some special treatment? > > Ray Kim > > -Original Message- > From: "Jakub Jelinek" > To: "김규래"; > Cc: ; > Sent: 2019-06-04 (화) 03:21:01 (GMT+09:00) > Subject: Re: [GSoC'19, libgomp work-stealing] Task parallelism runtime > > On Tue, Jun 04, 2019 at 03:01:13AM +0900, 김규래 wrote: > > Hi, > > I've been studying the libgomp task parallelism system. > > I have a few questions. > > First, Tracing the events shows that only the main thread calls GOMP_task. > > No, any thread can call GOMP_task, in particular the thread that encountered > the #pragma omp task construct. > > The GOMP_task function then decides based on the clauses of the construct > (passed in various ways through the arguments of that function) whether it > will be included (executed by the encountering thread), or queued for > later execution. In the latter case, it will be scheduled during a barrier > (implicit or explicit), see gomp_barrier_handle_tasks called from the > bar.[ch] code, or at other spots, e.g. during taskwait construct > (GOMP_taskwait) or at the end of taskgroup (GOMP_taskgroup_end). > > > How do the other worker threads enter the libgomp runtime? > > If you never encounter a parallel, teams or target construct, then there is > just one thread that does everything (well, the library is written such that > if you by hand pthread_create, each such thread acts as a separate initial > thread from OpenMP POV). > Threads are created e.g. during parallel construct (GOMP_parallel), where > for non-nested parallelism as the standard requires it reuses existing > threads if possible or spawns new ones, see mainly team.c (gomp_team_start) > for the function that spawns new threads or awakes the ones waiting for > work, or gomp_thread_start in the same file for the function actually run by > the libgomp library created threads. > > > I can't find the entry point of the worker threads from the event tracing > > and the assembly dump. > > Second, How is the task priority set? > > By the user, through priority clause, passed to GOMP_task and then taken > into account when handling tasks in the various queues. > > Jakub -- Janne Blomqvist
Re: Fixed: "required ftruncate or chsize support not present" in gfortran testsuite
On Thu, May 23, 2019 at 5:21 AM Hans-Peter Nilsson
wrote:
>
> There was a regression for gfortran.dg/fmt_en.f90 for cris-elf
> that on inspection was due to it having acquired a truncation
> call through the runtime. I updated that and the new tests that
> had "Fortran runtime error: required ftruncate or chsize support
> not present" messages in gfortran.log, ran past cris-elf and
> committed as obvious. See also
> <https://gcc.gnu.org/ml/gcc-patches/2008-05/msg00975.html> from
> which I copy-pasted most of this message. (Yep, 11 years ago.)
>
> gcc/testsuite:
> * gfortran.dg/dec_io_1.f90, gfortran.dg/dtio_1.f90,
> gfortran.dg/dtio_12.f90, gfortran.dg/fmt_en.f90,
> gfortran.dg/namelist_89.f90: Gate test on effective_target
> fd_truncate.
>
> diff --git gcc/testsuite/gfortran.dg/dec_io_1.f90
> gcc/testsuite/gfortran.dg/dec_io_1.f90
> index 2edc427..58daf30 100644
> --- gcc/testsuite/gfortran.dg/dec_io_1.f90
> +++ gcc/testsuite/gfortran.dg/dec_io_1.f90
> @@ -1,4 +1,4 @@
> -! { dg-do run }
> +! { dg-do run { target fd_truncate } }
> ! { dg-options "-fdec" }
> !
> ! Run-time tests for values of DEC I/O parameters (doesn't test
> functionality).
> diff --git gcc/testsuite/gfortran.dg/dtio_1.f90
> gcc/testsuite/gfortran.dg/dtio_1.f90
> index c6f17d9..b168d30 100644
> --- gcc/testsuite/gfortran.dg/dtio_1.f90
> +++ gcc/testsuite/gfortran.dg/dtio_1.f90
> @@ -1,4 +1,4 @@
> -! { dg-do run }
> +! { dg-do run { target fd_truncate } }
> !
> ! Functional test of User Defined Derived Type IO, Formatted WRITE/READ
> !
> diff --git gcc/testsuite/gfortran.dg/dtio_12.f90
> gcc/testsuite/gfortran.dg/dtio_12.f90
> index 54b10cb..ac6d9e7 100644
> --- gcc/testsuite/gfortran.dg/dtio_12.f90
> +++ gcc/testsuite/gfortran.dg/dtio_12.f90
> @@ -1,4 +1,4 @@
> -! { dg-do run }
> +! { dg-do run { target fd_truncate } }
> !
> ! Test the fix for PR77657 in which the DTIO subroutine was not found,
> ! which led to an error in attempting to link to the abstract interface.
> diff --git gcc/testsuite/gfortran.dg/fmt_en.f90
> gcc/testsuite/gfortran.dg/fmt_en.f90
> index b3597e4..89011b3 100644
> --- gcc/testsuite/gfortran.dg/fmt_en.f90
> +++ gcc/testsuite/gfortran.dg/fmt_en.f90
> @@ -1,4 +1,4 @@
> -! { dg-do run }
> +! { dg-do run { target fd_truncate } }
> ! PR60128 Invalid outputs with EN descriptors
> ! Test case provided by Walt Brainerd.
> program pr60128
> diff --git gcc/testsuite/gfortran.dg/namelist_89.f90
> gcc/testsuite/gfortran.dg/namelist_89.f90
> index fbb7143..91f64fe 100644
> --- gcc/testsuite/gfortran.dg/namelist_89.f90
> +++ gcc/testsuite/gfortran.dg/namelist_89.f90
> @@ -1,4 +1,4 @@
> -! { dg-do run }
> +! { dg-do run { target fd_truncate } }
> ! PR69456 Namelist value with trailing sign is ignored without error
> implicit none
> integer :: ios
Do you know which commit caused this? Was it something recent? Based
on a quick look, at least for fmt_en this shouldn't happen..
Since this seems to happen semi-regularly, would it be possible to do
something at the dejagnu level, e.g. to fail a test that uses
ftruncate/chsize even though it's not marked with target fd_truncate.
Hard to see how that could be verified without running testcases under
strace or something like that.
--
Janne Blomqvist
Re: [patch] Fix Fortran size_t parameter passing
On Wed, May 22, 2019 at 3:20 PM Andrew Stubbs wrote: > > On 22/05/2019 12:35, Janne Blomqvist wrote: > > Thanks for the catch. Though for size_t you should use build_zero_cst > > (size_type_node). size_zero_node is a zero constant of type sizetype, > > which is not the same as size_type_node (size_t in C). Ok with that > > change. > > So, integer_zero_node is compatible with integer_type_node, but > size_zero_node is not (necessarily) compatible with size_type_node? > Well, that's just asking for trouble. :-( Indeed it is. IIRC the main difference is that while both are unsigned types, sizetype has undefined behavior on overflow whereas size_type_node wraps around. And sizetype is an internal implementation detail, so should not be used in ABI-visible places. > Just to confirm, is the attached what you mean? Yes, looks good. -- Janne Blomqvist
Re: *ping* Re: [PATCH] PR fortran/89100 Default widths for i, f and g format specifiers in format strings
On Wed, May 22, 2019 at 10:58 AM Mark Eggleston wrote: > > On 13/05/2019 10:45, Mark Eggleston wrote: > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89100 see comment 4 > > > > Please can someone commit the attached patch as I do not have commit > > rights. > > > > Change logs: > > > > For gcc/fortran > > > > Jeff Law > > Mark Eggleston > > > > PR fortran/89100 > > * gfortran.texi: Add Default widths for F, G and I format descriptors > > to Extensions section. > > * invoke.texi: Add -fdec-format-defaults > > * io.c (check_format): Use default widths for i, f and g when > > flag_dec_format_defaults is enabled. > > * lang.opt: Add new option. > > * options.c (set_dec_flags): Add SET_BITFLAG for > > flag_dec_format_defaults. > > > > For gcc/testsuite > > > > Mark Eggleston > > > > PR fortran/89100 > > * gfortran.dg/fmt_f_default_field_width_1.f90: New test. > > * gfortran.dg/fmt_f_default_field_width_2.f90: New test. > > * gfortran.dg/fmt_f_default_field_width_3.f90: New test. > > * gfortran.dg/fmt_g_default_field_width_1.f90: New test. > > * gfortran.dg/fmt_g_default_field_width_2.f90: New test. > > * gfortran.dg/fmt_g_default_field_width_3.f90: New test. > > * gfortran.dg/fmt_i_default_field_width_1.f90: New test. > > * gfortran.dg/fmt_i_default_field_width_2.f90: New test. > > * gfortran.dg/fmt_i_default_field_width_3.f90: New test. > > > > For libgfortran > > > > Jeff Law > > > > PR fortran/89100 > > * io/format.c (parse_format_list): set default width when the > > IOPARM_DT_DEC_EXT flag is set for i, f and g. > > * io/io.h: add default_width_for_integer, default_width_for_float > > and default_precision_for_float. > > * io/write.c (write_boz): extra parameter giving length of data > > corresponding to the type's kind. > > (write_b): pass data length as extra parameter in calls to write_boz. > > (write_o): pass data length as extra parameter in calls to write_boz. > > (write_z): pass data length as extra parameter in calls to write_boz. > > (size_from_kind): also set size is default width is set. > > * io/write_float.def (build_float_string): new paramter inserted > > before > > result parameter. If default width use values passed instead of the > > values in fnode. > > (FORMAT_FLOAT): macro modified to check for default width and > > calls to > > build_float_string to pass in default width. > > (get_float_string): set width and precision to defaults when needed. > > > > > ping? Committed as r271511. -- Janne Blomqvist
Re: [patch] Fix coarray_lock_7.f90 test failure
On Wed, May 22, 2019 at 1:59 PM Andrew Stubbs wrote: > > The coarray_lock_7.f90 test has fixed-width patterns for matching the > variable numbers in the dumps. This has caused recent test failures for > at least amdgcn because changes elsewhere have happened to increase > those numbers too far. > > This patch adds a "\\d+" match to allow variable-width matches. > > OK to commit? Ok. -- Janne Blomqvist
Re: [patch] Fix Fortran size_t parameter passing
On Wed, May 22, 2019 at 1:54 PM Andrew Stubbs wrote: > > This patch fixes a bug observed on amdgcn in which the Fortran frontend > creates function calls using the 32-bit parameters where they ought to > be 64-bit, resulting in UB. > > The issue is caused by the use of "integer_zero_node" where the > definition of the function calls for "size_zero_node". I presume this > works on other architectures because the types are the same size, or > else because parameters are always 64-bit wide. > > OK to commit? > > Andrew Thanks for the catch. Though for size_t you should use build_zero_cst (size_type_node). size_zero_node is a zero constant of type sizetype, which is not the same as size_type_node (size_t in C). Ok with that change. -- Janne Blomqvist
Re: [PATCH] Convert contrib/mklog script to Python 3
On Tue, May 21, 2019 at 10:47 AM Janne Blomqvist
wrote:
>
> On Tue, May 21, 2019 at 10:32 AM Martin Liška wrote:
> >
> > Hi.
> >
> > There's a regression I see after the transition to python3:
> >
> > $ cat /tmp/patch
> > diff --git a/gcc/testsuite/gcc.dg/pr90263.c b/gcc/testsuite/gcc.dg/pr90263.c
> > index acf3db16640..3222a5331c1 100644
> > --- a/gcc/testsuite/gcc.dg/pr90263.c
> > +++ b/gcc/testsuite/gcc.dg/pr90263.c
> > @@ -1,5 +1,6 @@
> > /* PR middle-end/90263 */
> > /* { dg-do compile } */
> > +/* { dg-options "-O2" } */
> > /* { dg-require-effective-target glibc } */
> >
> > int *f (int *p, int *q, long n)
> >
> > $ ~/Programming/gcc/contrib/mklog /tmp/patch
> > Traceback (most recent call last):
> > File "/home/marxin/Programming/gcc/contrib/mklog", line 470, in
> > main()
> > File "/home/marxin/Programming/gcc/contrib/mklog", line 388, in main
> > diffs = parse_patch(contents)
> > File "/home/marxin/Programming/gcc/contrib/mklog", line 273, in
> > parse_patch
> > lines = contents.split('\n')
> > TypeError: a bytes-like object is required, not 'str'
> >
> > Thanks,
> > Martin
>
> Oof, thanks for the report, looking into it!
>
>
> --
> Janne Blomqvist
Committed r271459 as obvious:
diff --git a/contrib/mklog b/contrib/mklog
index 125f52ef11c..be1dc3a27fc 100755
--- a/contrib/mklog
+++ b/contrib/mklog
@@ -380,7 +380,7 @@ def main():
if len(args) == 1 and args[0] == '-':
input = sys.stdin
elif len(args) == 1:
-input = open(args[0], 'rb')
+input = open(args[0])
else:
error("too many arguments; for more details run with -h")
@@ -442,7 +442,7 @@ def main():
shutil.copymode(args[0], tmp)
# Open the temp file, clearing contents.
-out = open(tmp, 'wb')
+out = open(tmp, 'w')
else:
tmp = None
out = sys.stdout
--
Janne Blomqvist
Re: [PATCH] Convert contrib/mklog script to Python 3
On Tue, May 21, 2019 at 10:32 AM Martin Liška wrote:
>
> Hi.
>
> There's a regression I see after the transition to python3:
>
> $ cat /tmp/patch
> diff --git a/gcc/testsuite/gcc.dg/pr90263.c b/gcc/testsuite/gcc.dg/pr90263.c
> index acf3db16640..3222a5331c1 100644
> --- a/gcc/testsuite/gcc.dg/pr90263.c
> +++ b/gcc/testsuite/gcc.dg/pr90263.c
> @@ -1,5 +1,6 @@
> /* PR middle-end/90263 */
> /* { dg-do compile } */
> +/* { dg-options "-O2" } */
> /* { dg-require-effective-target glibc } */
>
> int *f (int *p, int *q, long n)
>
> $ ~/Programming/gcc/contrib/mklog /tmp/patch
> Traceback (most recent call last):
> File "/home/marxin/Programming/gcc/contrib/mklog", line 470, in
> main()
> File "/home/marxin/Programming/gcc/contrib/mklog", line 388, in main
> diffs = parse_patch(contents)
> File "/home/marxin/Programming/gcc/contrib/mklog", line 273, in parse_patch
> lines = contents.split('\n')
> TypeError: a bytes-like object is required, not 'str'
>
> Thanks,
> Martin
Oof, thanks for the report, looking into it!
--
Janne Blomqvist
[PATCH] Convert contrib/mklog script to Python 3
Upstream will drop support for Python 2.x on January 1, 2020. This
patch converts the contrib/mklog script to Python 3. The process for
the conversion was as follows.
- Use the futurize tool (https://python-future.org ) to apply the
print_with_import, except, and dict transformations.
- Remove the "from __future__ import print_function".
- Change the shebang line to search for python3 in the environment.
- Modify the run() function to return a str instead of bytes.
- Update the copyright year.
contrib/ChangeLog:
2019-05-20 Janne Blomqvist
* mklog: Convert to Python 3.
Ok for trunk?
---
contrib/mklog | 24
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/contrib/mklog b/contrib/mklog
index eb765edcbe2..125f52ef11c 100755
--- a/contrib/mklog
+++ b/contrib/mklog
@@ -1,6 +1,6 @@
-#!/usr/bin/python
+#!/usr/bin/env python3
-# Copyright (C) 2017 Free Software Foundation, Inc.
+# Copyright (C) 2017-2019 Free Software Foundation, Inc.
#
# This file is part of GCC.
#
@@ -67,7 +67,7 @@ class RegexCache(object):
cache = RegexCache()
def print_help_and_exit():
-print """\
+print("""\
Usage: %s [-i | --inline] [PATCH]
Generate ChangeLog template for PATCH.
PATCH must be generated using diff(1)'s -up or -cp options
@@ -78,7 +78,7 @@ When PATCH is - or missing, read standard input.
When -i is used, prepends ChangeLog to PATCH.
If PATCH is not stdin, modifies PATCH in-place, otherwise writes
to stdout.
-""" % me
+""" % me)
sys.exit(1)
def run(cmd, die_on_error):
@@ -87,7 +87,7 @@ def run(cmd, die_on_error):
(out, err) = proc.communicate()
if die_on_error and proc.returncode != 0:
error("`%s` failed:\n" % (cmd, proc.stderr))
- return proc.returncode, out, err
+ return proc.returncode, out.decode(), err
def read_user_info():
dot_mklog_format_msg = """\
@@ -153,9 +153,9 @@ class FileDiff:
self.clname, self.relname = get_parent_changelog(filename);
def dump(self):
-print "Diff for %s:\n ChangeLog = %s\n rel name = %s\n" %
(self.filename, self.clname, self.relname)
+print("Diff for %s:\n ChangeLog = %s\n rel name = %s\n" %
(self.filename, self.clname, self.relname))
for i, h in enumerate(self.hunks):
- print "Next hunk %d:" % i
+ print("Next hunk %d:" % i)
h.dump()
class Hunk:
@@ -167,8 +167,8 @@ class Hunk:
self.ctx_diff = is_ctx_hunk_start(hdr)
def dump(self):
-print '%s' % self.hdr
-print '%s' % '\n'.join(self.lines)
+print('%s' % self.hdr)
+print('%s' % '\n'.join(self.lines))
def is_file_addition(self):
"""Does hunk describe addition of file?"""
@@ -358,7 +358,7 @@ def main():
try:
opts, args = getopt.getopt(sys.argv[1:], 'hiv', ['help', 'verbose',
'inline'])
- except getopt.GetoptError, err:
+ except getopt.GetoptError as err:
error(str(err))
inline = False
@@ -388,7 +388,7 @@ def main():
diffs = parse_patch(contents)
if verbose:
-print "Parse results:"
+print("Parse results:")
for d in diffs:
d.dump()
@@ -449,7 +449,7 @@ def main():
# Print log
date = time.strftime('%Y-%m-%d')
- for log_name, msg in sorted(logs.iteritems()):
+ for log_name, msg in sorted(logs.items()):
out.write("""\
%s:
--
2.17.1
Re: [PATCH] libfortran/90038 Reap dead children when wait=.false.
On Sun, May 19, 2019 at 9:26 PM Steve Kargl
wrote:
>
> On Sun, May 19, 2019 at 09:10:57PM +0300, Janne Blomqvist wrote:
> > On Sun, May 19, 2019 at 7:15 PM Steve Kargl
> > wrote:
> > >
> > > On Sun, May 19, 2019 at 01:40:59PM +0300, Janne Blomqvist wrote:
> > > >
> > > > +#if defined(HAVE_SIGACTION) && defined(HAVE_WAITPID)
> > > > + static bool sig_init_saved;
> > > > + bool sig_init = __atomic_load_n (_init_saved,
> > > > __ATOMIC_RELAXED);
> > > > + if (!sig_init)
> > > > + {
> > > > + struct sigaction sa;
> > > > + sa.sa_handler = _handler;
> > > > + sigemptyset(_mask);
> > > > + sa.sa_flags = SA_RESTART | SA_NOCLDSTOP;
> > > > + sigaction(SIGCHLD, , 0);
> > > > + __atomic_store_n (_init_saved, true, __ATOMIC_RELAXED);
> > > > + }
> > > > +#endif
> > >
> > > Where do the prototypes for __atomic_load_n and __atomic_store_n
> > > come from? On FreeBSD, it seems these are in stdatomic.h. Do
> > > we need a HAVE_ATOMIC to include the header?
> >
> > They are GCC atomic builtins, they are always available, no need to
> > include a header:
> > https://gcc.gnu.org/onlinedocs/gcc/_005f_005fatomic-Builtins.html
>
> Thanks for the clarification. OK for trunk.
Thanks, committed as r271384.
> > > On a slightly different note, the nonstandard SYSTEM intrinsic
> > > uses system(3) to execute a command. I believe that it will
> > > leave zombies/orphaned children if a process is interrupted.
> > > Perhap, SYSTEM should be reworked to use your EXECUTE_COMMAND_LINE.
> >
> > I believe any competent implementation of system() would wait for the
> > child process. Since system() is synchronous, it can do the waiting
> > inline without having to use a signal handler.
>
> I'll need to check. I recall that
>
> program foo
>call system("Something_that_takes_along_time_to_execute")
> end program foo
>
> gfortran -o foo foo.f90
>
> % foo
> ^C
>
> Results in termination of the parent foo, and
> Something_that_takes_along_time_to_execute is orphaned and
> continues to run. It would probably be better to deliver
> SIGKILL to child. I suppose that that is for another day.
I guess the expected behavior is that is the parent dies, the child
gets orphaned and thus re-parented to PID 1 (that is, init) which
takes care of wait()'ing for it so it's not left as a zombie when it
dies. Not sure it's possible to change this is a robust and portable
way. But if someone figures it out, we can think about it then.
--
Janne Blomqvist
Re: [PATCH] libfortran/90038 Reap dead children when wait=.false.
On Sun, May 19, 2019 at 7:15 PM Steve Kargl
wrote:
>
> On Sun, May 19, 2019 at 01:40:59PM +0300, Janne Blomqvist wrote:
> >
> > +#if defined(HAVE_SIGACTION) && defined(HAVE_WAITPID)
> > + static bool sig_init_saved;
> > + bool sig_init = __atomic_load_n (_init_saved, __ATOMIC_RELAXED);
> > + if (!sig_init)
> > + {
> > + struct sigaction sa;
> > + sa.sa_handler = _handler;
> > + sigemptyset(_mask);
> > + sa.sa_flags = SA_RESTART | SA_NOCLDSTOP;
> > + sigaction(SIGCHLD, , 0);
> > + __atomic_store_n (_init_saved, true, __ATOMIC_RELAXED);
> > + }
> > +#endif
>
> Where do the prototypes for __atomic_load_n and __atomic_store_n
> come from? On FreeBSD, it seems these are in stdatomic.h. Do
> we need a HAVE_ATOMIC to include the header?
They are GCC atomic builtins, they are always available, no need to
include a header:
https://gcc.gnu.org/onlinedocs/gcc/_005f_005fatomic-Builtins.html
> On a slightly different note, the nonstandard SYSTEM intrinsic
> uses system(3) to execute a command. I believe that it will
> leave zombies/orphaned children if a process is interrupted.
> Perhap, SYSTEM should be reworked to use your EXECUTE_COMMAND_LINE.
I believe any competent implementation of system() would wait for the
child process. Since system() is synchronous, it can do the waiting
inline without having to use a signal handler.
--
Janne Blomqvist
[PATCH] libfortran/90038 Reap dead children when wait=.false.
When using posix_spawn or fork to launch a child process, the parent
needs to wait for the child, otherwise the dead child is left as a
zombie process. For this purpose one can install a signal handler for
SIGCHLD.
2019-05-19 Janne Blomqvist
PR libfortran/90038
* intrinsics/execute_command_line (sigchld_handler): New function.
(execute_command_line): Install handler for SIGCHLD.
* configure.ac: Check for presence of sigaction and waitpid.
* config.h.in: Regenerated.
* configure: Regenerated.
Regtested on x86_64-pc-linux-gnu, Ok for trunk?
---
libgfortran/configure.ac | 2 +-
libgfortran/intrinsics/execute_command_line.c | 25 +++
2 files changed, 26 insertions(+), 1 deletion(-)
diff --git a/libgfortran/configure.ac b/libgfortran/configure.ac
index 8fd5a1a30a9..7cfce28ab69 100644
--- a/libgfortran/configure.ac
+++ b/libgfortran/configure.ac
@@ -314,7 +314,7 @@ if test "${hardwire_newlib:-0}" -eq 1; then
else
AC_CHECK_FUNCS_ONCE(getrusage times mkstemp strtof strtold snprintf \
ftruncate chsize chdir getentropy getlogin gethostname kill link symlink \
- sleep ttyname \
+ sleep ttyname sigaction waitpid \
alarm access fork posix_spawn setmode fcntl writev \
gettimeofday stat fstat lstat getpwuid vsnprintf dup \
getcwd localtime_r gmtime_r getpwuid_r ttyname_r clock_gettime \
diff --git a/libgfortran/intrinsics/execute_command_line.c
b/libgfortran/intrinsics/execute_command_line.c
index 2ef2324b243..1a471632172 100644
--- a/libgfortran/intrinsics/execute_command_line.c
+++ b/libgfortran/intrinsics/execute_command_line.c
@@ -36,6 +36,9 @@ see the files COPYING3 and COPYING.RUNTIME respectively. If
not, see
#include
extern char **environ;
#endif
+#if defined(HAVE_POSIX_SPAWN) || defined(HAVE_FORK)
+#include
+#endif
enum { EXEC_SYNCHRONOUS = -2, EXEC_NOERROR = 0, EXEC_SYSTEMFAILED,
EXEC_CHILDFAILED, EXEC_INVALIDCOMMAND };
@@ -62,6 +65,14 @@ set_cmdstat (int *cmdstat, int value)
}
+#if defined(HAVE_WAITPID) && defined(HAVE_SIGACTION)
+static void
+sigchld_handler (int signum __attribute__((unused)))
+{
+ while (waitpid ((pid_t)(-1), NULL, WNOHANG) > 0) {}
+}
+#endif
+
static void
execute_command_line (const char *command, bool wait, int *exitstat,
int *cmdstat, char *cmdmsg,
@@ -82,6 +93,20 @@ execute_command_line (const char *command, bool wait, int
*exitstat,
set_cmdstat (cmdstat, EXEC_NOERROR);
+#if defined(HAVE_SIGACTION) && defined(HAVE_WAITPID)
+ static bool sig_init_saved;
+ bool sig_init = __atomic_load_n (_init_saved, __ATOMIC_RELAXED);
+ if (!sig_init)
+ {
+ struct sigaction sa;
+ sa.sa_handler = _handler;
+ sigemptyset(_mask);
+ sa.sa_flags = SA_RESTART | SA_NOCLDSTOP;
+ sigaction(SIGCHLD, , 0);
+ __atomic_store_n (_init_saved, true, __ATOMIC_RELAXED);
+ }
+#endif
+
#ifdef HAVE_POSIX_SPAWN
const char * const argv[] = {"sh", "-c", cmd, NULL};
if (posix_spawn (, "/bin/sh", NULL, NULL,
--
2.17.1
Re: [PATCH] Add workaround for broken C/C++ wrappers to LAPACK (PR fortran/90329)
On Fri, May 17, 2019 at 9:57 PM Jerry DeLisle wrote: > > On 5/17/19 10:48 AM, Jeff Law wrote: > > My first, second and third thought has been we shouldn't be catering to > > code that is so clearly broken. Maybe we could do this on the release > > branches which would in turn give folks roughly a year to fix up this mess? > > > > jeff > > > > Not that I have much say, but I have been following this thread and the others > about broken wrappers and screwed up prototypes being used by R for there use > of > LAPACK. I have been holding off saying anything. > > I don't thing we should be doing anything in gfortran at all. I think the R > people need to fix their calls. People get caught by not following the proper > conventions and then want the compiler to fix it. Its not the compiler writers > job to fix users bad code. The Fortran conventions of having the string > lengths > appended to the end has been known for many many years and well documented > everywhere. > > Sorry for ranting and I understand everyone is just trying to do the right > thing. It would have been about an editorial fix on the R side. > > Maybe Jeff as a good compromise here in that at least we would not have to > carry > it forward in time. I don't think it's this simple, unfortunately. If it would be only R, then yes, we could help the R people fix their code and then it would all be done. But seems this is everywhere. It's in CBLAS & LAPACKE (the official BLAS and LAPACK C interfaces), it's in numpy (probably scipy also), R, arma. And BLAS/LAPACK are pretty central, and probably the single biggest reason why non-Fortran programmers use Fortran code. And while the issue has been known, it seems to have been happily ignored for the past 30 years. And yes, while I think one year might be a quite optimistic timeframe to get this fixed, I agree we shouldn't keep the workaround indefinitely either. I think the best way would be if CBLAS & LAPACKE would be fixed, and then we could tell people to use those rather than their own in-house broken interfaces. -- Janne Blomqvist
Re: [PATCH] libfortran/90038: Use posix_spawn instead of fork
On Thu, May 16, 2019 at 11:37 PM Nicolas Koenig wrote: > > Hi everyone, > > a quick heads up, that the native coarray patch I'm working on at the > moment (1200 loc and growing :D) will be based upon forked processes, so > we won't be able to eliminate fork(). I would guess that's a case where fork semantics are useful, as presumably copying the memory of a process is exactly what you want. posix_spawn() is a replacement for some common uses of fork+exec. > P.S.: About the paper: A microsoft guy who doesn't like fork()? Color me > surprised :D Well, it's one author out of four. And it's Microsoft Research, which does serious CS research, not PR department vomit. The paper does make some good points, and it's worth reading. As an interesting aside, the original libgfortran IO library was based upon a paper by one of the authors of this paper. One of my first contributions to GFortran back in the day was ripping out that. > > > On 16/05/2019 22:10, Janne Blomqvist wrote: > > On Thu, May 16, 2019 at 10:59 PM Thomas Koenig > > wrote: > >> > >> Hi Janne, > >> > >>> fork() semantics can be problematic. Most unix style OS'es have > >>> posix_spawn which can be used to replace fork + exec in many cases. > >>> For more information see > >>> e.g. > >>> https://www.microsoft.com/en-us/research/uploads/prod/2019/04/fork-hotos19.pdf > >>> > >>> This replaces the one use of fork in libgfortran with posix_spawn. > >> > >> Do I understand the patch correctly that we would no longer use fork() > >> if posix_spawn is not available? I think we should leave that in as > >> a fallback option. > > > > Yes. But there is already a fallback in case posix_spawn (or > > previously, fork) is not available, namely falling back to synchronous > > behavior. Since this is anyway somewhat of a corner case (namely, with > > wait=.false.), and posix_spawn is supported on all (well, at least > > Linux, macOS, *BSD, cygwin, Solarix, AIX) remotely modern unix type > > systems, a further fallback to fork() is IMHO not warranted. > > > > -- Janne Blomqvist
Re: [PATCH] libfortran/90038: Use posix_spawn instead of fork
On Thu, May 16, 2019 at 11:54 PM Thomas Koenig wrote: > > Hi Janne, > > > I differ there. > > A longer explanation: > > fork() is standard POSIX. So is posix_spawn, as the name implies. >Not all systems have posix_spawn. Do you know of any target we support that has fork but not posix_spawn? > The patch is OK from my side if you add fork() as a fallback option. Sure. I'm quite sure it's dead code that will never be executed, but hey, it's ifdeffed away so whatever. -- Janne Blomqvist
Re: [PATCH] libfortran/90038: Use posix_spawn instead of fork
On Thu, May 16, 2019 at 10:59 PM Thomas Koenig wrote: > > Hi Janne, > > > fork() semantics can be problematic. Most unix style OS'es have > > posix_spawn which can be used to replace fork + exec in many cases. > > For more information see > > e.g. > > https://www.microsoft.com/en-us/research/uploads/prod/2019/04/fork-hotos19.pdf > > > > This replaces the one use of fork in libgfortran with posix_spawn. > > Do I understand the patch correctly that we would no longer use fork() > if posix_spawn is not available? I think we should leave that in as > a fallback option. Yes. But there is already a fallback in case posix_spawn (or previously, fork) is not available, namely falling back to synchronous behavior. Since this is anyway somewhat of a corner case (namely, with wait=.false.), and posix_spawn is supported on all (well, at least Linux, macOS, *BSD, cygwin, Solarix, AIX) remotely modern unix type systems, a further fallback to fork() is IMHO not warranted. -- Janne Blomqvist
[PATCH] libfortran/90038: Use posix_spawn instead of fork
fork() semantics can be problematic. Most unix style OS'es have
posix_spawn which can be used to replace fork + exec in many cases.
For more information see
e.g.
https://www.microsoft.com/en-us/research/uploads/prod/2019/04/fork-hotos19.pdf
This replaces the one use of fork in libgfortran with posix_spawn.
2019-05-16 Janne Blomqvist
PR libfortran/90038
* configure.ac (AC_CHECK_FUNCS_ONCE): Check for posix_spawn
instead of fork.
* intrinsics/execute_command_line (execute_command_line): Use
posix_spawn instead of fork.
* Makefile.in: Regenerated.
* config.h.in: Regenerated.
* configure: Regenerated.
Regtested on x86_64-pc-linux-gnu, Ok for trunk?
---
libgfortran/configure.ac | 2 +-
libgfortran/intrinsics/execute_command_line.c | 19 +--
2 files changed, 10 insertions(+), 11 deletions(-)
diff --git a/libgfortran/configure.ac b/libgfortran/configure.ac
index c06db7b1a78..66af512a292 100644
--- a/libgfortran/configure.ac
+++ b/libgfortran/configure.ac
@@ -315,7 +315,7 @@ else
AC_CHECK_FUNCS_ONCE(getrusage times mkstemp strtof strtold snprintf \
ftruncate chsize chdir getentropy getlogin gethostname kill link symlink \
sleep ttyname \
- alarm access fork setmode fcntl writev \
+ alarm access posix_spawn setmode fcntl writev \
gettimeofday stat fstat lstat getpwuid vsnprintf dup \
getcwd localtime_r gmtime_r getpwuid_r ttyname_r clock_gettime \
getgid getpid getuid geteuid umask getegid \
diff --git a/libgfortran/intrinsics/execute_command_line.c
b/libgfortran/intrinsics/execute_command_line.c
index a234bc328b5..3df99c10678 100644
--- a/libgfortran/intrinsics/execute_command_line.c
+++ b/libgfortran/intrinsics/execute_command_line.c
@@ -32,7 +32,10 @@ see the files COPYING3 and COPYING.RUNTIME respectively. If
not, see
#ifdef HAVE_SYS_WAIT_H
#include
#endif
-
+#ifdef HAVE_POSIX_SPAWN
+#include
+extern char **environ;
+#endif
enum { EXEC_SYNCHRONOUS = -2, EXEC_NOERROR = 0, EXEC_SYSTEMFAILED,
EXEC_CHILDFAILED, EXEC_INVALIDCOMMAND };
@@ -71,7 +74,7 @@ execute_command_line (const char *command, bool wait, int
*exitstat,
/* Flush all I/O units before executing the command. */
flush_all_units();
-#if defined(HAVE_FORK)
+#if defined(HAVE_POSIX_SPAWN)
if (!wait)
{
/* Asynchronous execution. */
@@ -79,14 +82,10 @@ execute_command_line (const char *command, bool wait, int
*exitstat,
set_cmdstat (cmdstat, EXEC_NOERROR);
- if ((pid = fork()) < 0)
+ const char * const argv[] = {"sh", "-c", cmd, NULL};
+ if (posix_spawn (, "/bin/sh", NULL, NULL,
+ (char * const* restrict) argv, environ))
set_cmdstat (cmdstat, EXEC_CHILDFAILED);
- else if (pid == 0)
- {
- /* Child process. */
- int res = system (cmd);
- _exit (WIFEXITED(res) ? WEXITSTATUS(res) : res);
- }
}
else
#endif
@@ -96,7 +95,7 @@ execute_command_line (const char *command, bool wait, int
*exitstat,
if (res == -1)
set_cmdstat (cmdstat, EXEC_SYSTEMFAILED);
-#ifndef HAVE_FORK
+#ifndef HAVE_POSIX_SPAWN
else if (!wait)
set_cmdstat (cmdstat, EXEC_SYNCHRONOUS);
#endif
--
2.17.1
Re: [PATCH] fortran: C++ support for generating C prototypes
On Wed, May 15, 2019 at 4:41 PM Jakub Jelinek wrote:
>
> On Sun, May 12, 2019 at 12:47:04AM +0300, Janne Blomqvist wrote:
> > --- a/gcc/fortran/parse.c
> > +++ b/gcc/fortran/parse.c
> > @@ -6331,6 +6331,24 @@ done:
> >}
> >
> >/* Dump C prototypes. */
> > + if (flag_c_prototypes || flag_c_prototypes_external)
> > +{
> > + fprintf (stdout,
> > +_("#include \n"
> > + "#ifdef __cplusplus\n"
> > + "#include \n"
> > + "#define FLOAT_COMPLEX std::complex\n"
> > + "#define DOUBLE_COMPLEX std::complex\n"
> > + "#define LONG_DOUBLE_COMPLEX std::complex\n"
> > + "extern \"C\" {\n"
> > + "#else\n"
> > + "#define FLOAT_COMPLEX float _Complex\n"
> > + "#define DOUBLE_COMPLEX double _Complex\n"
> > + "#define LONG_DOUBLE_COMPLEX long double _Complex\n"
> > + "#endif\n\n"));
>
> Two more things:
> 1) why the _() around the code snippet? Do you expect translators
>to translate the C snippets to something else or what?
Er, because originally I printed out these definitons as part of the
other fprintf call where a comment was printed, and when I moved it to
another location I forgot to remove the translation markers.
Committed r271261 and r271264 as obvious.
> 2) I don't think float _Complex is
>passed the same as std::complex and similar for others;
>std::complex is in libstdc++ a C++ class with with
>__complex__ float as its sole non-static data member and with non-trivial
>constructors; which means it is passed/returned via a hidden reference;
>when the argument is actually FLOAT_COMPLEX * or FLOAT_COMPLEX &,
>you except for aliasing don't have to care that much, but if
>that complex argument has VALUE attribute in Fortran and so the
>C prototype would be FLOAT_COMPLEX, then std::complex is
>passed in the end as std::complex &, while float _Complex
>the same as __complex__ float; and ditto for functions returning
>complex
Ugh, I guess that's right. Any good way around it? Except print a
warning in the header that passing std::complex<> by value doesn't
work?
--
Janne Blomqvist
Re: [PATCH] Allow opening file on multiple units
On Wed, May 15, 2019 at 9:30 PM Damian Rouson wrote: > > Could someone please update the Fortran 2018 status page on the wiki > to reflect this patch? Thanks for the reminder, done. -- Janne Blomqvist
