Re: [Patch, libgfortran] PR 25561 Part 1/2 Get rid of alloc stream

[Jerry DeLisle <jvdelisle at verizon dot net>]

Janne Blomqvist wrote:
> Jerry DeLisle wrote:
> > Janne Blomqvist wrote:
> > > Hello,
> > >
> > > compared to the previous submission
> > >
> > > http://gcc.gnu.org/ml/gcc-patches/2008-05/msg00268.html
> > >
> > > this patch now regtests cleanly due to the introduction of a format
> > > buffer to correctly handle T and TL edit descriptors on non-seekable
> > > files. Currently the default size of the format buffer is 4 KB. This is
> > > perhaps a bit on the large side, my thinking was that it should be large
> > > enough to cover everything but the most pathological testcases without
> > > having to do a realloc. Perhaps a more modest, say, 512 bytes would be
> > > more than enough to cover the common cases. The format buffer is flushed
> > > after every write statement, relying on the underlying buffering to
> > > avoid too much syscall overhead.
> > >
> > > There is still a single use of salloc left in the library; however this
> > > is only used for internal files, thus paving the way for using stdio for
> > > external files as explained in the message linked above.
> > >
> > > Performance-wise, at the moment it is roughly the same as without the
> > > patch. For stream I/O there should be a performance increase, as the
> > > patch reduces unnecessary seeks. Comparing the testio.f90 benchmark
> > > Jerry provided with ifort 9.1, there is still lots of room for
> > > improvement.
> > >
> > > For the NIST tests, it still fails FM111, FM406 and FM903 if I'm
> > > interpreting the results correctly, but it seems 4.2 and 4.3 have the
> > > same problems (406 passes on 4.2 and fails on 4.3) so I don't think it's
> > > a regression?
> > The NIST tests are dependent on visual inspections.  The reference
> > output files from the site given on the wiki may not be current, so I
> > will check on those here. Diff against the reference files is used to
> > determine pass or fail and there can be more than one right answer for
> > some tests. FM406 may be one of those that needs to be updated. 
> > Regardless, I will try testing the patch and see if I can narrow down on
> > any NIST regressions.  I would like those fixed before we commit if
> > possible.  Since I am later in the time zones look for further reply to
> > this submittal in the next few days.
>
> Hi,
>
> I just downloaded the entire NIST suite from the link on the wiki, and
> with the correct compile options provided by the script I'm happy to
> report it ran without errors. It seems I previously didn't realize that
> the programs read input from stdin rather than opening the .DAT file
> directly, so please discard my report of NIST failures above.

I was finally able to build and test the patch.  I have found no regressions 
after exercising largio, NIST, and several nasty little corner cases I have.

I did find two compile warnings:

../../../gcc44/libgfortran/io/transfer.c: In function 'skip_record':
../../../gcc44/libgfortran/io/transfer.c:2293: warning: signed and unsigned type 
in conditional expression
../../../gcc44/libgfortran/io/fbuf.c: In function 'fbuf_seek':
../../../gcc44/libgfortran/io/fbuf.c:129: warning: comparison between signed and 
unsigned

I also did some performance testing:

cat write-many.f
      program main
      open(10,status='SCRATCH')
      a = 0.3858204
      do i=1,50000
         a = a + 0.4761748164
         write(10, '(G12.5)') a
      end do
      end

g77 write-many.f
time ./a.out > wm-g77.out

real    0m0.098s
user    0m0.095s
sys     0m0.003s

gfc -static write-many.f      <<This is 4.4.0 with Janne's patch
time ./a.out > wm-gfc.out

real    0m0.244s
user    0m0.240s
sys     0m0.005s

gfortran -static write-many.f   <<This is 4.1.2
time ./a.out > wm-gfortran.out

real    0m0.236s
user    0m0.228s
sys     0m0.008s

Yes, compared to g77 we have a long way to go.  However, this patch does not 
regress performance relative to trunk without the patch.

Good news and OK to commit after warnings cleaned up.

Thanks for the great effort.

Jerry