From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 21472 invoked by alias); 27 Nov 2017 15:17:20 -0000 Mailing-List: contact fortran-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Post: List-Help: , Sender: fortran-owner@gcc.gnu.org Received: (qmail 20806 invoked by uid 89); 27 Nov 2017 15:17:19 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-24.3 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KB_WAM_FROM_NAME_SINGLEWORD,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=flags.cc, INQUIRE, flagscc, 4517 X-Spam-User: qpsmtpd, 2 recipients X-HELO: mail-yw0-f180.google.com Received: from mail-yw0-f180.google.com (HELO mail-yw0-f180.google.com) (209.85.161.180) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 27 Nov 2017 15:17:12 +0000 Received: by mail-yw0-f180.google.com with SMTP id v195so8214642ywg.5; Mon, 27 Nov 2017 07:17:11 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to; bh=F7f/iQFnu1EUhOPph3mtVfKjIn0acPZ42jczoqbd82Q=; b=UrsQZnAecw5lbLJKsro4wpavxCmDH/tXysxcMszvvWK+nmdgVFX6eazyMbZdBejBa2 /7GpTXbEUlDQEw4yWTRhKPI2ujMkBQmXoCyTkIxazX9WMALW5GUm6W1K86efqov09Epf DMgAtcpnEqNWBbqF25HZJynvEoDM4LmsA8jP5goVrJ7HIAhtba+NEtNFPY0sWuSdewLx eBiJDl14lS/ippj6aOesqBU6wf3CXvl1GIKPA3jgijWkP/8P0iTLyMZU2z3Cu24sI8qo qKJWJ87KEgQJZ9OcBINRVnlGQtw+N7KnNcHTBNyO4MjPiXNFIj/r5xzJIEhOEGt6Mte6 64Fg== X-Gm-Message-State: AJaThX7h14u9FC3tjB3g6ldbqt6MP5BIR/TPQTftd6+vnQqmFlGbFK6+ KGrs4PkVaWTwys7v0YKe4NbGpBd90Celg9wWTuY= X-Google-Smtp-Source: AGs4zMYcYjak/ZZkDfrojm4Qncl9ez4mNgnahkJY/oFs7HjZu6ZXdMChPjl6DqMZgxcUQ8uJDr5q8GjFvcI5dUZJx0s= X-Received: by 10.129.93.195 with SMTP id r186mr4792063ywb.90.1511795829849; Mon, 27 Nov 2017 07:17:09 -0800 (PST) MIME-Version: 1.0 Received: by 10.129.163.146 with HTTP; Mon, 27 Nov 2017 07:17:09 -0800 (PST) In-Reply-To: <1511208375-18252-1-git-send-email-blomqvist.janne@gmail.com> References: <1511208375-18252-1-git-send-email-blomqvist.janne@gmail.com> From: Janne Blomqvist Date: Mon, 27 Nov 2017 15:17:00 -0000 Message-ID: Subject: Re: [PATCH] PR 53796 Improve INQUIRE(RECL=...) handling To: Fortran List , GCC Patches Content-Type: text/plain; charset="UTF-8" X-IsSubscribed: yes X-SW-Source: 2017-11/txt/msg00167.txt.bz2 PING On Mon, Nov 20, 2017 at 10:06 PM, Janne Blomqvist wrote: > The current F2018 draft (N2137) specifies behavior of the RECL= > specifier in the INQUIRE statement, where it previously was left as > undefined. Namely: > > - If the unit is not connected, RECL= should be given the value -1. > - If the unit is connected with stream access, RECL= should be given > the value -2. > > Further, as PR 53796 describes, the handling of RECL= is poor in other > ways as well. When the recl is set to the maximum possible > (GFC_INTEGER_8_HUGE / LLONG_MAX), which it does by default except for > preconnected units, and when INQUIRE(RECL=) is used with a 4 byte > integer, the value is truncated and the 4 byte value is thus > -1. Fixing this to generate an error is a lot of work, as currently > the truncation is done by the frontend, the library sees only an 8 > byte value with no indication that the frontend is going to copy it to > a 4 byte one. Instead, this patch does a bit twiddling trick such that > the truncated 4 byte value is GFC_INTEGER_4_HUGE while still being > 0.99999999 * GFC_INTEGER_8_HUGE which is large enough for all > practical purposes. > > Finally, the patch removes GFORTRAN_DEFAULT_RECL which was used only > for preconnected units, and instead uses the same approach as describe > above. > > Regtested on x86_64-pc-linux-gnu, Ok for trunk? > > gcc/fortran/ChangeLog: > > 2017-11-20 Janne Blomqvist > > PR fortran/53796 > * gfortran.texi: Remove mentions of GFORTRAN_DEFAULT_RECL. > > libgfortran/ChangeLog: > > 2017-11-20 Janne Blomqvist > > PR fortran/53796 > * io/inquire.c (inquire_via_unit): Set recl to -1 for unconnected > units. > * io/io.h (default_recl): New variable. > * io/open.c (new_unit): Set recl to default_recl for sequential, > -2 for stream access. > * io/transfer.c (read_block_form): Test against default_recl > instead of DEFAULT_RECL. > (write_block): Likewise. > * io/unit.c (init_units): Calculate max_offset, default_recl. > * libgfortran.h (DEFAULT_RECL): Remove. > * runtime/environ.c: Remove GFORTRAN_DEFAULT_RECL. > > gcc/testsuite/ChangeLog: > > 2017-11-20 Janne Blomqvist > > PR fortran/53796 > * gfortran.dg/inquire_recl_f2018.f90: New test. > --- > gcc/fortran/gfortran.texi | 9 ----- > gcc/testsuite/gfortran.dg/inquire_recl_f2018.f90 | 42 ++++++++++++++++++++++++ > libgfortran/io/inquire.c | 4 ++- > libgfortran/io/io.h | 5 +++ > libgfortran/io/open.c | 6 ++-- > libgfortran/io/transfer.c | 4 +-- > libgfortran/io/unit.c | 33 ++++++++++++------- > libgfortran/libgfortran.h | 8 +---- > libgfortran/runtime/environ.c | 4 --- > 9 files changed, 79 insertions(+), 36 deletions(-) > create mode 100644 gcc/testsuite/gfortran.dg/inquire_recl_f2018.f90 > > diff --git a/gcc/fortran/gfortran.texi b/gcc/fortran/gfortran.texi > index 4b4688c..36c7b94 100644 > --- a/gcc/fortran/gfortran.texi > +++ b/gcc/fortran/gfortran.texi > @@ -600,7 +600,6 @@ Malformed environment variables are silently ignored. > * GFORTRAN_UNBUFFERED_PRECONNECTED:: Do not buffer I/O for preconnected units. > * GFORTRAN_SHOW_LOCUS:: Show location for runtime errors > * GFORTRAN_OPTIONAL_PLUS:: Print leading + where permitted > -* GFORTRAN_DEFAULT_RECL:: Default record length for new files > * GFORTRAN_LIST_SEPARATOR:: Separator for list output > * GFORTRAN_CONVERT_UNIT:: Set endianness for unformatted I/O > * GFORTRAN_ERROR_BACKTRACE:: Show backtrace on run-time errors > @@ -683,14 +682,6 @@ where permitted by the Fortran standard. If the first letter > is @samp{n}, @samp{N} or @samp{0}, a plus sign is not printed > in most cases. Default is not to print plus signs. > > -@node GFORTRAN_DEFAULT_RECL > -@section @env{GFORTRAN_DEFAULT_RECL}---Default record length for new files > - > -This environment variable specifies the default record length, in > -bytes, for files which are opened without a @code{RECL} tag in the > -@code{OPEN} statement. This must be a positive integer. The > -default value is 1073741824 bytes (1 GB). > - > @node GFORTRAN_LIST_SEPARATOR > @section @env{GFORTRAN_LIST_SEPARATOR}---Separator for list output > > diff --git a/gcc/testsuite/gfortran.dg/inquire_recl_f2018.f90 b/gcc/testsuite/gfortran.dg/inquire_recl_f2018.f90 > new file mode 100644 > index 0000000..8a13340 > --- /dev/null > +++ b/gcc/testsuite/gfortran.dg/inquire_recl_f2018.f90 > @@ -0,0 +1,42 @@ > +! { dg-do run } > +! PR 53796 INQUIRE(RECL=...) > +program inqrecl > + implicit none > + integer(8) :: r > + integer :: r4 > + ! F2018 (N2137) 12.10.2.26: recl for unconnected should be -1 > + inquire(10, recl=r) > + if (r /= -1) then > + call abort() > + end if > + > + ! Formatted sequential > + open(10, status="scratch") > + inquire(10, recl=r) > + inquire(10, recl=r4) > + close(10) > + if (r /= huge(0_8) - huge(0_4) - 1) then > + call abort() > + end if > + if (r4 /= huge(0)) then > + call abort() > + end if > + > + ! Formatted sequential with recl= specifier > + open(10, status="scratch", recl=100) > + inquire(10, recl=r) > + close(10) > + if (r /= 100) then > + call abort() > + end if > + > + ! Formatted stream > + ! F2018 (N2137) 12.10.2.26: If unit is connected > + ! for stream access, recl should be assigned the value -2. > + open(10, status="scratch", access="stream") > + inquire(10, recl=r) > + close(10) > + if (r /= -2) then > + call abort() > + end if > +end program inqrecl > diff --git a/libgfortran/io/inquire.c b/libgfortran/io/inquire.c > index 4cf87d3..f6c6078 100644 > --- a/libgfortran/io/inquire.c > +++ b/libgfortran/io/inquire.c > @@ -218,7 +218,9 @@ inquire_via_unit (st_parameter_inquire *iqp, gfc_unit *u) > } > > if ((cf & IOPARM_INQUIRE_HAS_RECL_OUT) != 0) > - *iqp->recl_out = (u != NULL) ? u->recl : 0; > + /* F2018 (N2137) 12.10.2.26: If there is no connection, recl is > + assigned the value -1. */ > + *iqp->recl_out = (u != NULL) ? u->recl : -1; > > if ((cf & IOPARM_INQUIRE_HAS_STRM_POS_OUT) != 0) > *iqp->strm_pos_out = (u != NULL) ? u->strm_pos : 0; > diff --git a/libgfortran/io/io.h b/libgfortran/io/io.h > index d29b112..41611c4 100644 > --- a/libgfortran/io/io.h > +++ b/libgfortran/io/io.h > @@ -735,6 +735,11 @@ gfc_saved_unit; > extern gfc_offset max_offset; > internal_proto(max_offset); > > +/* Default RECL for sequential access if not given in OPEN statement, > + computed at library initialization time. */ > +extern gfc_offset default_recl; > +internal_proto(default_recl); > + > /* Unit tree root. */ > extern gfc_unit *unit_root; > internal_proto(unit_root); > diff --git a/libgfortran/io/open.c b/libgfortran/io/open.c > index 9d3988a..4d292ef 100644 > --- a/libgfortran/io/open.c > +++ b/libgfortran/io/open.c > @@ -586,7 +586,7 @@ new_unit (st_parameter_open *opp, gfc_unit *u, unit_flags *flags) > else > { > u->flags.has_recl = 0; > - u->recl = max_offset; > + u->recl = default_recl; > if (compile_options.max_subrecord_length) > { > u->recl_subrecord = compile_options.max_subrecord_length; > @@ -622,7 +622,9 @@ new_unit (st_parameter_open *opp, gfc_unit *u, unit_flags *flags) > if (flags->access == ACCESS_STREAM) > { > u->maxrec = max_offset; > - u->recl = 1; > + /* F2018 (N2137) 12.10.2.26: If the connection is for stream > + access recl is assigned the value -2. */ > + u->recl = -2; > u->bytes_left = 1; > u->strm_pos = stell (u->s) + 1; > } > diff --git a/libgfortran/io/transfer.c b/libgfortran/io/transfer.c > index c173447..7f881c7 100644 > --- a/libgfortran/io/transfer.c > +++ b/libgfortran/io/transfer.c > @@ -451,7 +451,7 @@ read_block_form (st_parameter_dt *dtp, int *nbytes) > /* For preconnected units with default record length, set bytes left > to unit record length and proceed, otherwise error. */ > if (dtp->u.p.current_unit->unit_number == options.stdin_unit > - && dtp->u.p.current_unit->recl == DEFAULT_RECL) > + && dtp->u.p.current_unit->recl == default_recl) > dtp->u.p.current_unit->bytes_left = dtp->u.p.current_unit->recl; > else > { > @@ -757,7 +757,7 @@ write_block (st_parameter_dt *dtp, int length) > == options.stdout_unit > || dtp->u.p.current_unit->unit_number > == options.stderr_unit) > - && dtp->u.p.current_unit->recl == DEFAULT_RECL)) > + && dtp->u.p.current_unit->recl == default_recl)) > dtp->u.p.current_unit->bytes_left = dtp->u.p.current_unit->recl; > else > { > diff --git a/libgfortran/io/unit.c b/libgfortran/io/unit.c > index e06867a..0f8bbbb 100644 > --- a/libgfortran/io/unit.c > +++ b/libgfortran/io/unit.c > @@ -96,7 +96,10 @@ static void newunit_free (int); > > #define CACHE_SIZE 3 > static gfc_unit *unit_cache[CACHE_SIZE]; > + > gfc_offset max_offset; > +gfc_offset default_recl; > + > gfc_unit *unit_root; > #ifdef __GTHREAD_MUTEX_INIT > __gthread_mutex_t unit_lock = __GTHREAD_MUTEX_INIT; > @@ -576,7 +579,6 @@ void > init_units (void) > { > gfc_unit *u; > - unsigned int i; > > #ifdef HAVE_NEWLOCALE > c_locale = newlocale (0, "C", 0); > @@ -590,6 +592,22 @@ init_units (void) > __GTHREAD_MUTEX_INIT_FUNCTION (&unit_lock); > #endif > > + if (sizeof (max_offset) == 8) > + { > + max_offset = GFC_INTEGER_8_HUGE; > + /* Why this weird value? Because if the recl specifier in the > + inquire statement is a 4 byte value, u->recl is truncated, > + and this trick ensures it becomes HUGE(0) rather than -1. > + The full 8 byte value of default_recl is still 0.99999999 * > + max_offset which is large enough for all practical > + purposes. */ > + default_recl = max_offset & ~(1LL<<31); > + } > + else if (sizeof (max_offset) == 4) > + max_offset = default_recl = GFC_INTEGER_4_HUGE; > + else > + internal_error (NULL, "sizeof (max_offset) must be 4 or 8"); > + > if (options.stdin_unit >= 0) > { /* STDIN */ > u = insert_unit (options.stdin_unit); > @@ -612,7 +630,7 @@ init_units (void) > u->flags.share = SHARE_UNSPECIFIED; > u->flags.cc = CC_LIST; > > - u->recl = options.default_recl; > + u->recl = default_recl; > u->endfile = NO_ENDFILE; > > u->filename = strdup (stdin_name); > @@ -643,7 +661,7 @@ init_units (void) > u->flags.share = SHARE_UNSPECIFIED; > u->flags.cc = CC_LIST; > > - u->recl = options.default_recl; > + u->recl = default_recl; > u->endfile = AT_ENDFILE; > > u->filename = strdup (stdout_name); > @@ -673,7 +691,7 @@ init_units (void) > u->flags.share = SHARE_UNSPECIFIED; > u->flags.cc = CC_LIST; > > - u->recl = options.default_recl; > + u->recl = default_recl; > u->endfile = AT_ENDFILE; > > u->filename = strdup (stderr_name); > @@ -683,13 +701,6 @@ init_units (void) > > __gthread_mutex_unlock (&u->lock); > } > - > - /* Calculate the maximum file offset in a portable manner. > - max will be the largest signed number for the type gfc_offset. > - set a 1 in the LSB and keep a running sum, stopping at MSB-1 bit. */ > - max_offset = 0; > - for (i = 0; i < sizeof (max_offset) * 8 - 1; i++) > - max_offset = max_offset + ((gfc_offset) 1 << i); > } > > > diff --git a/libgfortran/libgfortran.h b/libgfortran/libgfortran.h > index cdbdd951..e9648b5 100644 > --- a/libgfortran/libgfortran.h > +++ b/libgfortran/libgfortran.h > @@ -514,7 +514,7 @@ typedef struct > int separator_len; > const char *separator; > > - int all_unbuffered, unbuffered_preconnected, default_recl; > + int all_unbuffered, unbuffered_preconnected; > int fpe, backtrace; > } > options_t; > @@ -580,12 +580,6 @@ extern char *filename; > iexport_data_proto(filename); > > > -/* The default value of record length for preconnected units is defined > - here. This value can be overriden by an environment variable. > - Default value is 1 Gb. */ > -#define DEFAULT_RECL 1073741824 > - > - > #define CHARACTER2(name) \ > gfc_charlen_type name ## _len; \ > char * name > diff --git a/libgfortran/runtime/environ.c b/libgfortran/runtime/environ.c > index f0a593e..fb9a3c1 100644 > --- a/libgfortran/runtime/environ.c > +++ b/libgfortran/runtime/environ.c > @@ -208,10 +208,6 @@ static variable variable_table[] = { > /* Print optional plus signs in numbers where permitted */ > { "GFORTRAN_OPTIONAL_PLUS", 0, &options.optional_plus, init_boolean }, > > - /* Default maximum record length for sequential files */ > - { "GFORTRAN_DEFAULT_RECL", DEFAULT_RECL, &options.default_recl, > - init_unsigned_integer }, > - > /* Separator to use when writing list output */ > { "GFORTRAN_LIST_SEPARATOR", 0, NULL, init_sep }, > > -- > 2.7.4 > -- Janne Blomqvist