headers
Timothy C Prince | 1 Dec 2006 02:02
Picon

Re: Re: Polyhedron speed comparison (on AMD64)


In case my original reply has disappeared permanently:

> If anyone has an idea what gfortran or GCC should do better ...
>
Vectorize that loop you identified in gas_dyn.

> -- and why we are that slow in fatigue.

My copy of gfortran makes separate scalar calls to sin and cos, where ifort makes a vector sincos call. 2
seconds to be gained there, maybe 1 second with a scalar sincos.  gfortran allocates and deallocates
arrays all over, including around each call to generalized_hookes_law.  There's another 2 seconds.  Is it
because of the whole array syntax somearray(:,:) ?
Another half second can be gained by recognizing the matmul() in generalized_hookes_law, using
-fexternal-blas, linking with MKL 9.0, and setting 2 threads.

Tim
Tim Prince
Permalink | Reply | 0 comments |
headers
Jerry DeLisle | 1 Dec 2006 04:38
Picon

Re: PR libfortran/29568 (subrecord patch)

Thomas Koenig wrote:
> :ADDPATCH fortran:
> 
> Hello world,
> 
> after quite some time, here it is:  The gfortran subrecord patch
> (aka implementing the Intel format for unformatted files).  This was
> regression-tested on i686-pc-linux-gnu.  A slightly earlier version
> (which was missing some casts) was extensively tested on 64-bit
> Linux and FreeBSD by Jerry DeLisle (thanks a lot, Jerry!)
> 
> I introduced a new flag with this patch to specify a smaller
> maximum length of subrecords.  This enables automatic
> regression-testing of the subrecod feature, which I think is
> important.  It is very easy to break something in this
> area of the library, and not having a regression-test at all
> would be bad.
> 
> With this patch, we're having a flag day:  Unformatted sequential
> files written with default options in eariler versions of gfortran
> (including 4.1) will not be readable with default options.
> The minutes from the IRC planning meeting showed that this was
> preferred way over being incompatible with just about anybody
> else (and I agree, or I wouldn't have written the patch :-)  We
> should update the wiki accordingly, and make an announcement on
> comp.lang.fortran.
> 
> Still missing:  a description of the subrecord format.
> 
> I propose to apply this to 4.3, wait for some time (more than
(Continue reading)

Permalink | Reply | 0 comments |
headers
Brooks Moses | 1 Dec 2006 07:17

Re: PR libfortran/29568 (subrecord patch)

Thomas Koenig wrote:
> 2006-11-30  Thomas Koenig  <Thomas.Koenig <at> online.de>
[...]
> 	* invoke.texi:  Document the new default for
> 	-frecord-marker=<n>.

I can't seem to find this change in the patch file.  Am I missing 
something, or was it left out?

- Brooks
Permalink | Reply | 1 comment |
headers
Jerry DeLisle | 1 Dec 2006 07:41
Picon

Re: PR libfortran/29568 (subrecord patch)

Brooks Moses wrote:
> Thomas Koenig wrote:
>> 2006-11-30  Thomas Koenig  <Thomas.Koenig <at> online.de>
> [...]
>>     * invoke.texi:  Document the new default for
>>     -frecord-marker=<n>.
> 
> I can't seem to find this change in the patch file.  Am I missing 
> something, or was it left out?
> 
> - Brooks
> 
> 
No, it was left out.  Good catch.
Permalink | Reply | 0 comments |
headers
Brooks Moses | 1 Dec 2006 07:52

Re: PR libfortran/29568 (subrecord patch)

Thomas Koenig wrote:
> after quite some time, here it is:  The gfortran subrecord patch
> (aka implementing the Intel format for unformatted files).  This was
> regression-tested on i686-pc-linux-gnu.  A slightly earlier version
> (which was missing some casts) was extensively tested on 64-bit
> Linux and FreeBSD by Jerry DeLisle (thanks a lot, Jerry!)

A few minor things:

> Index: gcc/fortran/options.c
> ===================================================================
> --- gcc/fortran/options.c	(revision 119261)
> +++ gcc/fortran/options.c	(working copy)
>  <at>  <at>  -636,6 +636,12  <at>  <at>  gfc_handle_option (size_t scode, const c
>      case OPT_frecord_marker_8:
>        gfc_option.record_marker = 8;
>        break;
> +
> +    case OPT_fmax_subrecord_length_:
> +      if (value > MAX_SUBRECORD_LENGTH || value < 1)
> +	gfc_fatal_error ("Maximum subrecord length is %d", MAX_SUBRECORD_LENGTH);
> +
> +      gfc_option.max_subrecord_length = value;

I think this error message is confusing, because you've got two 
competing definitions for "maximum subrecord length" -- the option is 
(according to the lang.opt documentation) defining what the maximum 
subrecord length is, but this is giving an error that (1) doesn't state 
clearly what's wrong, and (2) gives a value for the "maximum subrecord 
length" that isn't what the user specified in the option that defines it.
(Continue reading)

Permalink | Reply | 6 comments |
headers
Uros Bizjak | 1 Dec 2006 08:30
Picon

Re: Polyhedron speed comparison (on AMD64)

On 11/30/06, Paul Thomas <paulthomas2 <at> wanadoo.fr> wrote:

> > First, you remember our big regression for ae on AMD64? This has now
> > been partially fixed by Uros (using x87 for fmod):

> > Thus we are not yet back at our original speed, maybe one should find
> > the reason for the extra 2s step at Nov 13/14.
> >
> Nothing stands out in fortran or libgfortran, does it?

I'd propose a bisection search for the offending change. However, it
takes a couple of rebuilds to find it. I've done it a couple of times
by hand in the past, but a script could come really handy there.

Basically, you start from the current HEAD and create a svn diff -r
xxx, where xxx represents starting revision. Apply this diff as a
reversed patch and rebuild gfortran (use --enable-languages=c,fortran
--disable-bootstrap here). Benchmark it.

Following this, you create a diff between current revision and some
later revision. You can check ChangeLog diffs, if there was some
activity in areas you are interested in (script can't do this...) and
repeat above... If you know approximate time frame, it could be better
to do a linear search instead of bisection, but it takes around 3-5
rebuilds to find the offender.

Looking at [1], there is at least one more point of interest,
performance drop in protein somewhere in the same time frame as
performance drop in ac.
(Continue reading)

Permalink | Reply | 0 comments |
headers
Tobias Burnus | 1 Dec 2006 10:26
Picon

Re: PR libfortran/29568 (subrecord patch)

Hello Thomas, hi all,

Thomas Koenig wrote:
> after quite some time, here it is:  The gfortran subrecord patch
> (aka implementing the Intel format for unformatted files).
Great! And also thanks to Jerry.

> We should update the wiki accordingly, and make an announcement on
> comp.lang.fortran.
>   
And shouldn't forget the 4.3 (and 4.2) changelog:
http://gcc.gnu.org/gcc-4.3/changes.html

> Still missing:  a description of the subrecord format.
>   

> 	* invoke.texi:  Document the new default for
> 	-frecord-marker=<n>.
>   
I don't see the patch for this item.

Tobias
Permalink | Reply | 0 comments |
headers
Tobias Burnus | 1 Dec 2006 12:25
Picon

Re: Polyhedron speed comparison (on AMD64)

Hi,

Paul Thomas wrote:
>> Thus we are not yet back at our original speed, maybe one should find
>> the reason for the extra 2s step at Nov 13/14.  
> Nothing stands out in fortran or libgfortran, does it?

According to
http://www.suse.de/~gcctest/c++bench/polyhedron/polyhedron-summary.txt
the performance regressions occur for ac between Nov 13 and Nov 14 (~31
to ~33.5s) -> 8% slower and for protein between Nov 14 (~70s) and Nov 15
(~73s) -> 4% slower. Thanks for Uros for pointing out the latter.

I didn't do performance a detailed regression tests, but with my nightly
builds
using "gfortran -march=opteron -ffast-math -funroll-loops
-ftree-vectorize -msse3 -O3"

First result: I cannot find the ac regression ?!?
for protein, it must be between r118802 and r118918 (4% slower, same as
Richi's results.)

2006-11-11-r118703
ac:
22.20user 0.00system 0:22.21elapsed
22.10user 0.00system 0:22.10elapsed
22.12user 0.00system 0:22.12elapsed
protein:
51.94user 12.79system 1:04.73elapsed
51.51user 12.64system 1:04.16elapsed
(Continue reading)

Permalink | Reply | 2 comments |
headers
Thomas Koenig | 1 Dec 2006 22:13
Picon

Re: PR libfortran/29568 (subrecord patch)

Hello world,

based on Jerry's approval, and after some minor changes suggested
mainly by Brooks, I have committed the patch to trunk as revision
119412.

Thanks!

I have attached the version I committed (including the Changelog
entries) for reference.

Brooks wrote:

> I'd suggest making "maximum subrecord length" unambiguously refer to the 
> user-supplied value in the UI, never to MAX_SUBRECORD_LENGTH, and 
> writing this error as "Maximum subrecord length cannot be greater than %d".

Done.

> That, of course, is inappropriate for the "value < 1" case, but I think 
> it makes sense either to put in a separate error message for that, or to 
> note that the only possible value that's less than 1 is 0, since 
> UInteger options never have negative values, and that it might make just 
> as much sense to use -fmax-subrecord-length=0 as a flag that means "as 
> unlimited as possible" in the same way that it means that for line 
> lengths and such, and have that set the value to MAX_SUBRECORD_LENGTH.

I have removed the test for 0.  Now, -fmax-subrecord-length=0 will
silently be treated as a do-nothing option.
(Continue reading)

Permalink | Reply | 4 comments |
headers
Tobias Burnus | 2 Dec 2006 11:17
Picon

[patch,wwwdata] Update Fortran section of gcc-4.2/changes.html

:ADDPATCH wwwdocs:

The following patch documents the record-marker change and adds also
some Fortran 2003 changes to the list of changes.

According to Gerd Pfeiffer: "Note that for Y-specific changes [...] it's
perfectly fine for Y-specific maintainers to approve/commit them
directly. No need for the wwwdocs maintainer to micro-manage."
http://gcc.gnu.org/ml/fortran/2006-11/msg00205.html

Thus I only need an approval by a gfortran maintainer.

Tobias

2006-12-02  Tobias Burnus  <burnus <at> net-b.de>

    * gcc-4.3/changes.html: Document record-marker change and new
Fortran 2003 support.
Attachment (gcc-4.3.diff): text/x-patch, 1353 bytes
Permalink | Reply | 11 comments |

Gmane