| Message ID | 32d1a016-d097-9eb2-b5ea-95433f6f50ec@charter.net |
|---|---|
| State | New |
| Headers | show |
On Mon, Oct 17, 2016 at 06:02:52PM -0700, Jerry DeLisle wrote: > Hi all, > > The attached patch enables the size= specifier in a READ statement to work with > child DTIO procedures. This is accomplished by moving the size_used variable > from the dtp structure to the gfc_unit structure so that the accumulation of > bytes during READ is carried across the procedures via the UNIT. > > As far as I know, this is the last DTIO patch needed for full implementation and > will close the PR. > > After this patch is committed I plan to prepare a clean up patch to reorganize > the dtp structure and clear at least one TODO related to stream IO. The > follow-on patch will bump the major version number of libgfortran to 4. > > Regression tested on x86-64-linux. New test case attached. > > OK for trunk? Lookd good to me. > * transfer.c (read_sf_internal): Likewise. (read_sf): Likewise. > (read_block_form): Likewise. (read_block_form4): Likewise. You can simplify this by * transfer.c (read_sf_internal, read_sf, read_block_form, read_block_form4): Likewise. -- Steve
Hi, On 18 October 2016 at 04:45, Steve Kargl <sgk@troutmask.apl.washington.edu> wrote: > On Mon, Oct 17, 2016 at 06:02:52PM -0700, Jerry DeLisle wrote: >> Hi all, >> >> The attached patch enables the size= specifier in a READ statement to work with >> child DTIO procedures. This is accomplished by moving the size_used variable >> from the dtp structure to the gfc_unit structure so that the accumulation of >> bytes during READ is carried across the procedures via the UNIT. >> >> As far as I know, this is the last DTIO patch needed for full implementation and >> will close the PR. >> >> After this patch is committed I plan to prepare a clean up patch to reorganize >> the dtp structure and clear at least one TODO related to stream IO. The >> follow-on patch will bump the major version number of libgfortran to 4. >> >> Regression tested on x86-64-linux. New test case attached. >> >> OK for trunk? > > Lookd good to me. > >> * transfer.c (read_sf_internal): Likewise. (read_sf): Likewise. >> (read_block_form): Likewise. (read_block_form4): Likewise. > > Since this was committed (r241294), I'm seeing regressions on arm and aarch64 targets. The list is a bit long, so it's probably simpler you look at: http://people.linaro.org/~christophe.lyon/cross-validation/gcc/trunk/241306/report-build-info.html Then click on the red "REGRESSED" in the gfortran column. Thanks, Christophe > You can simplify this by > > * transfer.c (read_sf_internal, read_sf, read_block_form, > read_block_form4): Likewise. > > -- > Steve
At line 74 of file /opt/gcc/gcc-20161019/gcc/testsuite/gfortran.dg/dtio_17.f90 (unit = 28, file = '/tmp/gfortrantmpQF10b7') Fortran runtime error: Bad value during integer read Error termination. Backtrace: FAIL: gfortran.dg/dtio_17.f90 -O1 execution test Andreas. -- Andreas Schwab, SUSE Labs, schwab@suse.de GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7 "And now for something completely different."
On 10/19/2016 01:59 AM, Christophe Lyon wrote: > Hi, > > > On 18 October 2016 at 04:45, Steve Kargl > <sgk@troutmask.apl.washington.edu> wrote: >> On Mon, Oct 17, 2016 at 06:02:52PM -0700, Jerry DeLisle wrote: >>> Hi all, >>> >>> The attached patch enables the size= specifier in a READ statement to work with >>> child DTIO procedures. This is accomplished by moving the size_used variable >>> from the dtp structure to the gfc_unit structure so that the accumulation of >>> bytes during READ is carried across the procedures via the UNIT. --- snip --- > > Since this was committed (r241294), I'm seeing regressions on > arm and aarch64 targets. > > The list is a bit long, so it's probably simpler you look at: > http://people.linaro.org/~christophe.lyon/cross-validation/gcc/trunk/241306/report-build-info.html > Then click on the red "REGRESSED" in the gfortran column. > --- snip --- The patch changes nothing related to any particular target. If I were to hazard a guess I would think the tests are linking against a wrong version of the library, the follow on patch (not yet committed) is bumping the libgfortran major version number. I do not have access to a target machine unless there is a way to remote in. Is there one in the gcc compile farm? Jerry
On 10/19/2016 04:04 AM, Andreas Schwab wrote: > At line 74 of file /opt/gcc/gcc-20161019/gcc/testsuite/gfortran.dg/dtio_17.f90 (unit = 28, file = '/tmp/gfortrantmpQF10b7') > Fortran runtime error: Bad value during integer read > > Error termination. Backtrace: > FAIL: gfortran.dg/dtio_17.f90 -O1 execution test > > Andreas. > See my reply to Christophe post on arm aarch64. Could you try compiling manually the failing test with gfortran -static and then run it to see what happens. Also what target is this failing on. Jerry
On Okt 19 2016, Jerry DeLisle <jvdelisle@charter.net> wrote: > The patch changes nothing related to any particular target. If I were to > hazard a guess I would think the tests are linking against a wrong version > of the library, the follow on patch (not yet committed) is bumping the > libgfortran major version number. There is only one version of the library, the one just built. Andreas. -- Andreas Schwab, SUSE Labs, schwab@suse.de GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7 "And now for something completely different."
On Okt 19 2016, Jerry DeLisle <jvdelisle@charter.net> wrote: > Could you try compiling manually the failing test with gfortran -static > and then run it to see what happens. That does not change anything. > Also what target is this failing on. aarch64 and m68k. Andreas. -- Andreas Schwab, SUSE Labs, schwab@suse.de GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7 "And now for something completely different."
On 10/19/2016 08:20 AM, Andreas Schwab wrote: > On Okt 19 2016, Jerry DeLisle <jvdelisle@charter.net> wrote: > >> Could you try compiling manually the failing test with gfortran -static >> and then run it to see what happens. > > That does not change anything. > >> Also what target is this failing on. > > aarch64 and m68k. > > Andreas. > Dominique has discovered my flaw, the integers are being used not initialized and can happen to be large negative values that need 11 bytes in the format to hold the values. I will commit a change to the test case shortly. Jerry
diff --git a/libgfortran/ChangeLog b/libgfortran/ChangeLog index bfda86df..f20c5106 100644 --- a/libgfortran/ChangeLog +++ b/libgfortran/ChangeLog @@ -1,3 +1,15 @@ +2016-10-17 Jerry DeLisle <jvdelisle@gcc.gnu.org> + + PR fortran/48298 + * io/io.h: Move size_used from dtp to unit structure. Add bool + has_size to unit structure. + * read.c (read_x): Use has_size and size_used. + * transfer.c (read_sf_internal): Likewise. (read_sf): Likewise. + (read_block_form): Likewise. (read_block_form4): Likewise. + (data_transfer_init): If parent, initialize the size variables. + (finalize_transfer): Set the size variable using size_used in + gfc_unit. (write_block): Delete bogus/dead code. + 2016-10-16 Janne Blomqvist <jb@gcc.gnu.org> PR libfortran/48587 diff --git a/libgfortran/io/io.h b/libgfortran/io/io.h index aaacc089..edc520a9 100644 --- a/libgfortran/io/io.h +++ b/libgfortran/io/io.h @@ -514,7 +514,6 @@ typedef struct st_parameter_dt large enough to hold a complex value (two reals) of the largest kind. */ char value[32]; - GFC_IO_INT size_used; formatted_dtio fdtio_ptr; unformatted_dtio ufdtio_ptr; } p; @@ -650,6 +649,8 @@ typedef struct gfc_unit /* DTIO Parent/Child procedure, 0 = parent, >0 = child level. */ int child_dtio; int last_char; + bool has_size; + GFC_IO_INT size_used; } gfc_unit; diff --git a/libgfortran/io/read.c b/libgfortran/io/read.c index f8d5b72e..d72cdb37 100644 --- a/libgfortran/io/read.c +++ b/libgfortran/io/read.c @@ -1282,8 +1282,9 @@ read_x (st_parameter_dt *dtp, int n) } done: - if ((dtp->common.flags & IOPARM_DT_HAS_SIZE) != 0) - dtp->u.p.size_used += (GFC_IO_INT) n; + if (((dtp->common.flags & IOPARM_DT_HAS_SIZE) != 0) || + dtp->u.p.current_unit->has_size) + dtp->u.p.current_unit->size_used += (GFC_IO_INT) n; dtp->u.p.current_unit->bytes_left -= n; dtp->u.p.current_unit->strm_pos += (gfc_offset) n; } diff --git a/libgfortran/io/transfer.c b/libgfortran/io/transfer.c index 2232417a..e5805772 100644 --- a/libgfortran/io/transfer.c +++ b/libgfortran/io/transfer.c @@ -267,8 +267,9 @@ read_sf_internal (st_parameter_dt *dtp, int * length) dtp->u.p.current_unit->bytes_left -= *length; - if ((dtp->common.flags & IOPARM_DT_HAS_SIZE) != 0) - dtp->u.p.size_used += (GFC_IO_INT) *length; + if (((dtp->common.flags & IOPARM_DT_HAS_SIZE) != 0) || + dtp->u.p.current_unit->has_size) + dtp->u.p.current_unit->size_used += (GFC_IO_INT) *length; return base; @@ -397,8 +398,9 @@ read_sf (st_parameter_dt *dtp, int * length) dtp->u.p.current_unit->bytes_left -= n; - if ((dtp->common.flags & IOPARM_DT_HAS_SIZE) != 0) - dtp->u.p.size_used += (GFC_IO_INT) n; + if (((dtp->common.flags & IOPARM_DT_HAS_SIZE) != 0) || + dtp->u.p.current_unit->has_size) + dtp->u.p.current_unit->size_used += (GFC_IO_INT) n; /* We can't call fbuf_getptr before the loop doing fbuf_getc, because fbuf_getc might reallocate the buffer. So return current pointer @@ -478,8 +480,9 @@ read_block_form (st_parameter_dt *dtp, int * nbytes) source = fbuf_read (dtp->u.p.current_unit, nbytes); fbuf_seek (dtp->u.p.current_unit, *nbytes, SEEK_CUR); - if ((dtp->common.flags & IOPARM_DT_HAS_SIZE) != 0) - dtp->u.p.size_used += (GFC_IO_INT) *nbytes; + if (((dtp->common.flags & IOPARM_DT_HAS_SIZE) != 0) || + dtp->u.p.current_unit->has_size) + dtp->u.p.current_unit->size_used += (GFC_IO_INT) *nbytes; if (norig != *nbytes) { @@ -536,8 +539,9 @@ read_block_form4 (st_parameter_dt *dtp, int * nbytes) dtp->u.p.current_unit->bytes_left -= *nbytes; - if ((dtp->common.flags & IOPARM_DT_HAS_SIZE) != 0) - dtp->u.p.size_used += (GFC_IO_INT) *nbytes; + if (((dtp->common.flags & IOPARM_DT_HAS_SIZE) != 0) || + dtp->u.p.current_unit->has_size) + dtp->u.p.current_unit->size_used += (GFC_IO_INT) *nbytes; return source; } @@ -770,9 +774,6 @@ write_block (st_parameter_dt *dtp, int length) } } - if ((dtp->common.flags & IOPARM_DT_HAS_SIZE) != 0) - dtp->u.p.size_used += (GFC_IO_INT) length; - dtp->u.p.current_unit->strm_pos += (gfc_offset) length; return dest; @@ -2596,9 +2597,6 @@ data_transfer_init (st_parameter_dt *dtp, int read_flag) if ((dtp->common.flags & IOPARM_LIBRETURN_MASK) != IOPARM_LIBRETURN_OK) return; - if ((cf & IOPARM_DT_HAS_SIZE) != 0) - dtp->u.p.size_used = 0; /* Initialize the count. */ - dtp->u.p.current_unit = get_unit (dtp, 1); if (dtp->u.p.current_unit == NULL) @@ -2674,6 +2672,18 @@ data_transfer_init (st_parameter_dt *dtp, int read_flag) return; } + if (dtp->u.p.current_unit->child_dtio == 0) + { + if ((cf & IOPARM_DT_HAS_SIZE) != 0) + { + dtp->u.p.current_unit->has_size = true; + /* Initialize the count. */ + dtp->u.p.current_unit->size_used = 0; + } + else + dtp->u.p.current_unit->has_size = false; + } + /* Check the action. */ if (read_flag && dtp->u.p.current_unit->flags.action == ACTION_WRITE) @@ -3772,7 +3782,7 @@ finalize_transfer (st_parameter_dt *dtp) return; if ((dtp->common.flags & IOPARM_DT_HAS_SIZE) != 0) - *dtp->size = dtp->u.p.size_used; + *dtp->size = dtp->u.p.current_unit->size_used; if (dtp->u.p.eor_condition) {