This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [patch, libfortran] [4.7/4.8/4.9 Regression] PR38199 missed optimization: I/O performance
- From: Tobias Burnus <burnus at net-b dot de>
- To: Jerry DeLisle <jvdelisle at charter dot net>, gfortran <fortran at gcc dot gnu dot org>
- Cc: gcc patches <gcc-patches at gcc dot gnu dot org>
- Date: Sat, 08 Mar 2014 11:45:54 +0100
- Subject: Re: [patch, libfortran] [4.7/4.8/4.9 Regression] PR38199 missed optimization: I/O performance
- Authentication-results: sourceware.org; auth=none
- Newsgroups: gmane.comp.gcc.fortran, gmane.comp.gcc.patches
- References: <531ABAF8 dot 2070604 at charter dot net>
Jerry DeLisle wrote:
The attached patch addresses the problem identified in comment #22 of the PR.
For character array internal unit reads, eat_spaces must call next_char to
advance every single character until the end of the string is reached. In the
case sited which is very contrived, this amounts to about 100000 calls to next_char.
Without the patch takes about 25 seconds to run.
With the patch this takes about 2.8 seconds.
Nice!
The speedup is accomplished by simply skipping over spaces without calling
next_read, then backing up one character and letting the existing execution path
proceed, preserving all the end of record code needed in next_char.
I think that code is okay.
I wonder whether it can happen that we read one character too far: i.e.
stell() == end of string; offset++ -> one behind. As one then
immediately steps back (due to "offset < limit" plus offset--), the code
should be handled just fine - except for the out of bounds read - which
might be detected by, e.g., -fsanitizer=address (if libgfortran and the
program use it). However, I am not sure whether eat_spaces would be
called in that case or whether other conditions error out before.
I also remove some unneeded error checks.
I have to admit that I do not really understand why the conditions are
unreachable. Is it because "dtp->u.p.current_unit->bytes_left == 0" is
already checked for "is_array_io (dtp)"? If so, can it happen for scalar
internal I/O? I assume, it is obvious, but I don't quickly see it.
Tobias
2014-03-08 Jerry DeLisle <jvdelisle@gcc.gnu>
PR libfortran/38199
* io/list_read.c (next_char): Delete unuseful error checks.
(eat_spaces): For character array reading, skip ahead over
spaces rather than call next_char multiple times.