Re: MATMUL broken with frontend optimization.
On Fri, Mar 19, 2021 at 07:19:16AM +0100, Thomas Koenig wrote: > > Hi Steve, > > > On my old core2 cpu, a quick test with N=1000 and NxN matrix > > suggest a cross over near N=1000 for REAL(4). This cpu doesn't > > have any AVX* instruction, so YMMV. Program follows .sig > > Looking at your data with AVX (which I think we can mostly count > on now), > > - The library is always faster for matmul(vector,matrix) for any n >=100 > - For matmul(matrix,vector) there is no appreciable difference > > So, putting in the same inline limits for matmul(vector,matrix) > that we have for matmul(matrix,matrix), and leaving > mamul(matrix,vector) alone, seems like a reasonable thing to do. > > I'll work on a patch. > Thanks for working on this in such short notice. I agree the core2 is old, and gfortran should look toward the future so using the same inline threshold seems right. I saw your other email with the concept patch. I'll look through it tomorrow when I'm a little more cogent (it's after midnight here). -- Steve
[Patch] Fortran: Fix intrinsic null() handling [PR99651]
See PR for some analysis. The problem is that during
gfc_intrinsic_func_interface, sym->attr.flavor == FL_PROCEDURE,
hence, attr.intrinsic is not set – but later when parsing
'null()', gfortran calls:
if (sym->attr.proc != PROC_INTRINSIC
&& !(sym->attr.use_assoc && sym->attr.intrinsic)
&& (!gfc_add_procedure(&sym->attr, PROC_INTRINSIC, sym->name, NULL)
|| !gfc_add_function (&sym->attr, sym->name, NULL)))
return MATCH_ERROR;
The gfc_add_procedure call fails as 'sym' is use-associated and
may not be modified.
The obvious solution to also set attr.intrinsic for FL_PROCEDURE fails
in multiple ways, e.g. for gfortran.dg/char_length_16.f90 which has:
CHARACTER (LEN(ITEMVAL)) :: ITEM
INTRINSIC LEN
the error is that INTRINSIC has been speicified twice. It also affects
the error diagnostic in for generic resolution due to (I think):
if (sym->attr.intrinsic)
return gfc_intrinsic_func_interface (expr, 0);
For gfortran.dg/allocatable_scalar_11.f90 the specific
‘array’ argument of ‘allocated’ intrinsic at (1) must be a variable
gets replaced by the generic
Generic function 'allocated' at (1) is not consistent with a specific
intrinsic interface
I have now tried it as shown. I think attr.function was not set, but
am not sure. But setting it again for FL_PROCEDURE looks sensible.
I am far from certain that now everything fits together, but I do hope
that nothing fails which did work before ...
OK for mainline? And after waiting a while for GCC 10?
Tobias
PS: I did see once a fail for pr93792.f90 (additional error as 't' in type(t)
is not known) – but I could not reproduce it; the error is valid but later runs
stopped with 'cannot open module file' and did not reach that follow-up error.
-
Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München
Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank
Thürauf
Fortran: Fix intrinsic null() handling [PR99651]
gcc/fortran/ChangeLog:
PR fortran/99651
* intrinsic.c (gfc_intrinsic_func_interface): Set
attr.proc = PROC_INTRINSIC if FL_PROCEDURE.
gcc/testsuite/ChangeLog:
PR fortran/99651
* gfortran.dg/null_11.f90: New test.
gcc/fortran/intrinsic.c | 5 +
gcc/testsuite/gfortran.dg/null_11.f90 | 16
2 files changed, 21 insertions(+)
diff --git a/gcc/fortran/intrinsic.c b/gcc/fortran/intrinsic.c
index e68eff8bdbb..17fd92eb462 100644
--- a/gcc/fortran/intrinsic.c
+++ b/gcc/fortran/intrinsic.c
@@ -5071,6 +5071,11 @@ got_specific:
sym->attr.intrinsic = 1;
sym->attr.flavor = FL_PROCEDURE;
}
+ if (sym->attr.flavor == FL_PROCEDURE)
+{
+ sym->attr.function = 1;
+ sym->attr.proc = PROC_INTRINSIC;
+}
if (!sym->module)
gfc_intrinsic_symbol (sym);
diff --git a/gcc/testsuite/gfortran.dg/null_11.f90 b/gcc/testsuite/gfortran.dg/null_11.f90
new file mode 100644
index 000..040cc260b5d
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/null_11.f90
@@ -0,0 +1,16 @@
+! { dg-do compile }
+!
+! PR fortran/99651
+!
+module m
+type :: CHAR_STAR
+ character(len=1),dimension(:),pointer :: ptr
+end type
+type(CHAR_STAR), parameter ::CHAR_STAR_NULL = CHAR_STAR(NULL())
+end module m
+
+use m
+type typeNode
+type(typeNode), pointer :: Next => null()
+end type typeNode
+end
Fwd: GCC 10.2.1 Status Report (2021-03-19)
FYI. On my side, I would like to backport the following patch, which is still pending preview: https://gcc.gnu.org/pipermail/gcc-patches/2021-March/566890.html (*PING*: Re: [Patch] Fortran: Fix func decl mismatch [PR93660]) and possible the following patch as well, also pending review: https://gcc.gnu.org/pipermail/gcc-patches/2021-March/566956.html ([Patch] Fortran: Fix intrinsic null() handling [PR99651]) Does anyone else have patches which still have to be reviewed? Tobias Forwarded Message Subject:GCC 10.2.1 Status Report (2021-03-19) Date: Fri, 19 Mar 2021 14:17:45 +0100 From: Richard Biener Reply-To: g...@gcc.gnu.org To: g...@gcc.gnu.org CC: gcc-patc...@gcc.gnu.org Status == The GCC 10 branch is open for regression and documentation fixes. It's time to do the GCC 10.3 release and barring arrival of P1 priority regressions the plan is to do a release candidate in two weeks, around Mar 31th with a release following a week later. Please see to backport regression fixes that went to trunk where appropriate and verify your target is in good health. Report any problems to bugzilla. Quality Data Priority # Change from last report --- --- P1 1 + 1 P2 353 + 134 P3 30 - 27 P4 176 P5 23 + 1 --- --- Total P1-P3 384 + 108 Total 583 + 109 Previous Report === https://gcc.gnu.org/pipermail/gcc/2020-July/233214.html - Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank Thürauf
[patch, fortran] Also use size estimate for vector-matrix matmul
Hell world,
here is the patch I talked about earlier. It passes regression testing.
OK for trunk?
Best regards
Thomas
Add size check to vector-matrix matmul.
It turns out the library version is much faster for vector-matrix
multiplications for large sizes than what inlining can produce.
Use size checks for switching between this and inlining for
that case to.
gcc/fortran/ChangeLog:
* frontend-passes.c (inline_limit_check): Add rank_a
argument. If a is rank 1, set the second dimension to 1.
(inline_matmul_assign): Pass rank_a argument to inline_limit_check.
(call_external_blas): Likewise.
gcc/testsuite/ChangeLog:
* gfortran.dg/inline_matmul_6.f90: Adjust count for _gfortran_matmul.
diff --git a/gcc/fortran/frontend-passes.c b/gcc/fortran/frontend-passes.c
index cfc47471cf1..7d3eae67632 100644
--- a/gcc/fortran/frontend-passes.c
+++ b/gcc/fortran/frontend-passes.c
@@ -3307,7 +3307,7 @@ get_operand (gfc_intrinsic_op op, gfc_expr *e1, gfc_expr *e2)
removed by DCE. Only called for rank-two matrices A and B. */
static gfc_code *
-inline_limit_check (gfc_expr *a, gfc_expr *b, int limit)
+inline_limit_check (gfc_expr *a, gfc_expr *b, int limit, int rank_a)
{
gfc_expr *inline_limit;
gfc_code *if_1, *if_2, *else_2;
@@ -3315,16 +3315,28 @@ inline_limit_check (gfc_expr *a, gfc_expr *b, int limit)
gfc_typespec ts;
gfc_expr *cond;
+ gcc_assert (rank_a == 1 || rank_a == 2);
+
/* Calculation is done in real to avoid integer overflow. */
inline_limit = gfc_get_constant_expr (BT_REAL, gfc_default_real_kind,
&a->where);
mpfr_set_si (inline_limit->value.real, limit, GFC_RND_MODE);
- mpfr_pow_ui (inline_limit->value.real, inline_limit->value.real, 3,
+
+ /* Set the limit according to the rank. */
+ mpfr_pow_ui (inline_limit->value.real, inline_limit->value.real, rank_a + 1,
GFC_RND_MODE);
a1 = get_array_inq_function (GFC_ISYM_SIZE, a, 1);
- a2 = get_array_inq_function (GFC_ISYM_SIZE, a, 2);
+
+ /* For a_rank = 1, must use one as the size of a along the second
+ dimension as to avoid too much code duplication. */
+
+ if (rank_a == 2)
+a2 = get_array_inq_function (GFC_ISYM_SIZE, a, 2);
+ else
+a2 = gfc_get_int_expr (gfc_index_integer_kind, &a->where, 1);
+
b2 = get_array_inq_function (GFC_ISYM_SIZE, b, 2);
gfc_clear_ts (&ts);
@@ -4243,11 +4255,13 @@ inline_matmul_assign (gfc_code **c, int *walk_subtrees,
/* Take care of the inline flag. If the limit check evaluates to a
constant, dead code elimination will eliminate the unneeded branch. */
- if (flag_inline_matmul_limit > 0 && matrix_a->rank == 2
+ if (flag_inline_matmul_limit > 0
+ && (matrix_a->rank == 1 || matrix_a->rank == 2)
&& matrix_b->rank == 2)
{
if_limit = inline_limit_check (matrix_a, matrix_b,
- flag_inline_matmul_limit);
+ flag_inline_matmul_limit,
+ matrix_a->rank);
/* Insert the original statement into the else branch. */
if_limit->block->block->next = co;
@@ -4757,7 +4771,7 @@ call_external_blas (gfc_code **c, int *walk_subtrees ATTRIBUTE_UNUSED,
return 0;
/* Generate the if statement and hang it into the tree. */
- if_limit = inline_limit_check (matrix_a, matrix_b, flag_blas_matmul_limit);
+ if_limit = inline_limit_check (matrix_a, matrix_b, flag_blas_matmul_limit, 2);
co_next = co->next;
(*current_code) = if_limit;
co->next = NULL;
diff --git a/gcc/testsuite/gfortran.dg/inline_matmul_6.f90 b/gcc/testsuite/gfortran.dg/inline_matmul_6.f90
index 491a7215258..da717bda017 100644
--- a/gcc/testsuite/gfortran.dg/inline_matmul_6.f90
+++ b/gcc/testsuite/gfortran.dg/inline_matmul_6.f90
@@ -45,4 +45,4 @@ program main
if (any(abs(c2 - (/39., -61., 75./)) > 1e-3)) STOP 2
end program main
-! { dg-final { scan-tree-dump-times "_gfortran_matmul" 0 "original" } }
+! { dg-final { scan-tree-dump-times "_gfortran_matmul" 1 "original" } }
Re: GCC 10.2.1 Status Report (2021-03-19)
Hi Tobias, I'll do the reviews for you tomorrow morning. Right now my brain has turned to mush after an excess of online meetings :-( I'll take a look through my PRs to see what might need pushing onto 10-branch. Cheers Paul On Fri, 19 Mar 2021 at 14:15, Tobias Burnus wrote: > FYI. > > On my side, I would like to backport the following patch, which is still > pending preview: > https://gcc.gnu.org/pipermail/gcc-patches/2021-March/566890.html > (*PING*: Re: [Patch] Fortran: Fix func decl mismatch [PR93660]) > > and possible the following patch as well, also pending review: > https://gcc.gnu.org/pipermail/gcc-patches/2021-March/566956.html > ([Patch] Fortran: Fix intrinsic null() handling [PR99651]) > > Does anyone else have patches which still have to be reviewed? > > Tobias > > Forwarded Message > Subject:GCC 10.2.1 Status Report (2021-03-19) > Date: Fri, 19 Mar 2021 14:17:45 +0100 > From: Richard Biener > Reply-To: g...@gcc.gnu.org > To: g...@gcc.gnu.org > CC: gcc-patc...@gcc.gnu.org > > > > Status > == > > The GCC 10 branch is open for regression and documentation fixes. > It's time to do the GCC 10.3 release and barring arrival of P1 > priority regressions the plan is to do a release candidate in > two weeks, around Mar 31th with a release following a week later. > > Please see to backport regression fixes that went to trunk where > appropriate and verify your target is in good health. Report any > problems to bugzilla. > > > Quality Data > > > Priority # Change from last report > --- --- > P1 1 + 1 > P2 353 + 134 > P3 30 - 27 > P4 176 > P5 23 + 1 > --- --- > Total P1-P3 384 + 108 > Total 583 + 109 > > > Previous Report > === > > https://gcc.gnu.org/pipermail/gcc/2020-July/233214.html > - > Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München > Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank > Thürauf > -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
Re: MATMUL broken with frontend optimization.
On Fri, Mar 19, 2021 at 07:36:22AM +0100, Thomas Koenig wrote: > > Am 19.03.21 um 07:19 schrieb Thomas Koenig: > > > I'll work on a patch. > > So, here's a concept patch. It still needs a ChangeLog and testsuite > adjustment, but this is what I would propose to use. > I see that you've submitted the patch for review. AFAICT, it looks correct to me. Thanks for q quick fix. -- Steve
Re: [patch, fortran] Also use size estimate for vector-matrix matmul
Yes Ok for trunk. Thanks much! On 3/19/21 10:37 AM, Thomas Koenig via Fortran wrote: Hell world, here is the patch I talked about earlier. It passes regression testing. OK for trunk? Best regards Thomas Add size check to vector-matrix matmul. It turns out the library version is much faster for vector-matrix multiplications for large sizes than what inlining can produce. Use size checks for switching between this and inlining for that case to. gcc/fortran/ChangeLog: * frontend-passes.c (inline_limit_check): Add rank_a argument. If a is rank 1, set the second dimension to 1. (inline_matmul_assign): Pass rank_a argument to inline_limit_check. (call_external_blas): Likewise. gcc/testsuite/ChangeLog: * gfortran.dg/inline_matmul_6.f90: Adjust count for _gfortran_matmul.
[no subject]
Is there a way to make a statically linked binary with fortran in OS X? For much of the past year I have been using: gfortran taxsim.for -static-libgfortran -static-libgcc but since January I only get the error message; ld: library not found for -lm. collect2: error: ld returned 1 exit status? This is OS X 11,2,3 Big Sur and fortran version 6.3.0. I need static linking because my users are not developers and do not have Xcode or gcc installed. This is free software. I have seen postings from 2015 suggesting that I rename libquadmath.0.dylib, which I did try but which did not help. Of course I have no need for lquad precision variables, which I understand is the source of the problem. Daniel Feenberg NBER
gfortran static linking under OS X (was: Re: )
Hi, I am not sure whether it helps, but I want to point out that libm is the math library which is on Linux usually GLIBC and I assume on OS X it is provided by the OS vendor. Additionally, that libm is linked dynamically. This seems to be a OS X issue – and I have no idea about OS X, but I found the following: https://github.com/fxcoudert/gfortran-for-macOS/issues/12 which suggests that there is a mismatch of the XCode / MacOS X SDK version. As work around, it seems to work to set MACOSX_DEPLOYMENT_TARGET and/or to ensure that you have the same xcode version as the one used by gfortran, e.g. by "xcode-select --install". In some other thread, the suggestion was to use -L/usr/lib but I assume that won't help. Looking at https://github.com/xianyi/OpenBLAS/issues/3032 – it seems as if you can use 'gfortran -v taxsim.for' to see the SDK version used when building GCC. At least that example had some |string like: ||-syslibroot /Library/Developer/CommandLineTools/SDKs/MacOSX11.1.sdk/| |If you had installed 11.0 instead, ||MACOSX_DEPLOYMENT_TARGET=11.0 would work.| |I hope it helps. If not, you need to find someone else as I have no idea about OS X.| |Good luck!| |Tobias | On 19.03.21 21:22, Daniel Feenberg via Fortran wrote: Is there a way to make a statically linked binary with fortran in OS X? For much of the past year I have been using: gfortran taxsim.for -static-libgfortran -static-libgcc but since January I only get the error message; ld: library not found for -lm. collect2: error: ld returned 1 exit status? This is OS X 11,2,3 Big Sur and fortran version 6.3.0. I need static linking because my users are not developers and do not have Xcode or gcc installed. This is free software. I have seen postings from 2015 suggesting that I rename libquadmath.0.dylib, which I did try but which did not help. Of course I have no need for lquad precision variables, which I understand is the source of the problem. Daniel Feenberg NBER
Re: gfortran static linking under OS X (was: Re: )
Hi Daniel, Tobias Burnus wrote: I am not sure whether it helps, but I want to point out that libm is the math library which is on Linux usually GLIBC and I assume on OS X it is provided by the OS vendor. actually part of libSystem (but, yes, provided by the vendor) On 19.03.21 21:22, Daniel Feenberg via Fortran wrote: Is there a way to make a statically linked binary with fortran in OS X? For much of the past year I have been using: gfortran taxsim.for -static-libgfortran -static-libgcc OK - that should work - modulo the quadmath issue (if present) - .. but that is probably solvable (with some changes to the link spec). but since January I only get the error message; ld: library not found for -lm. collect2: error: ld returned 1 exit status? lm has not been needed on macOS for a [very] long time (for many releases it was simply a convenience symlink to libSystem.dylib, for the sake of OSS code that appends ‘-lm’). There doesn’t seem to be an issue with gcc-11 (master, development) or 10.2.1 (upcoming 10.3) on macOS 11 - will see if I can fire up a copy of gcc6.5 there ... Will have a think about how to fake the libm too .. not so easy these days. This is OS X 11,2,3 Big Sur and fortran version 6.3.0. This is an old version of Fortran on a very new version of macOS, at present the first supported GCC version for macOS 11 is the upcoming 10.3 release (although homebrew no doubt has a preview courtesy of FX). Is there any way you would be able to update to a newer (and eventually supported) Fortran version ? I need static linking because my users are not developers and do not have Xcode or gcc installed. understood. This is free software. I have seen postings from 2015 suggesting that I rename libquadmath.0.dylib, which I did try but which did not help. Of course I have no need for lquad precision variables, which I understand is the source of the problem. not from what you posted - it’s the absence of “libm.dylib” that results in the message. I realise that this mail contains no solutions - but will try to reproduce the issue over the weekend. cheers Iain
Re: gfortran static linking under OS X (was: Re: )
On Fri, 19 Mar 2021, Tobias Burnus wrote: Hi, I am not sure whether it helps, but I want to point out that libm is the math library which is on Linux usually GLIBC and I assume on OS X it is provided by the OS vendor. Additionally, that libm is linked dynamically. This seems to be a OS X issue ? and I have no idea about OS X, but I found the following: https://github.com/fxcoudert/gfortran-for-macOS/issues/12 It is certainly an OS X issue. There is no problem in Linux, FreeBSD or Windows with the same compiler. As I understand it, OS X doesn't allow true dynamic linking, but it does allow a compiler/linker to produce an executable binary which only requires OS supplied libraries at execution time, and which includes all the compiler specific libraries. So I wouldn't actually expect libm to be statically linked. Apparently the problem arises because the authors of libquadm don't want users to use it in statically linked binaries, and did something to prevent that from happening. Unfortunately, it means that specialized knowledge is required to statically link gfortran programs, even if they don't use quad precision math. Daniel Feenberg which suggests that there is a mismatch of the XCode / MacOS X SDK version. As work around, it seems to work to set MACOSX_DEPLOYMENT_TARGET and/or to ensure that you have the same xcode version as the one used by gfortran, e.g. by "xcode-select --install". In some other thread, the suggestion was to use -L/usr/lib but I assume that won't help. Looking at https://github.com/xianyi/OpenBLAS/issues/3032 ? it seems as if you can use 'gfortran -v taxsim.for' to see the SDK version used when building GCC. At least that example had some |string like: ||-syslibroot /Library/Developer/CommandLineTools/SDKs/MacOSX11.1.sdk/| |If you had installed 11.0 instead, ||MACOSX_DEPLOYMENT_TARGET=11.0 would work.| |I hope it helps. If not, you need to find someone else as I have no idea about OS X.| |Good luck!| |Tobias | On 19.03.21 21:22, Daniel Feenberg via Fortran wrote: Is there a way to make a statically linked binary with fortran in OS X? For much of the past year I have been using: gfortran taxsim.for -static-libgfortran -static-libgcc but since January I only get the error message; ld: library not found for -lm. collect2: error: ld returned 1 exit status? This is OS X 11,2,3 Big Sur and fortran version 6.3.0. I need static linking because my users are not developers and do not have Xcode or gcc installed. This is free software. I have seen postings from 2015 suggesting that I rename libquadmath.0.dylib, which I did try but which did not help. Of course I have no need for lquad precision variables, which I understand is the source of the problem. Daniel Feenberg NBER
Re: gfortran static linking under OS X (was: Re: )
Daniel Feenberg wrote:
On Fri, 19 Mar 2021, Tobias Burnus wrote:
This seems to be a OS X issue ? and I have no idea about OS X, but I
found the following:
https://github.com/fxcoudert/gfortran-for-macOS/issues/12
It is certainly an OS X issue.
Actually, it’s a gfortran issue.
There is no problem in Linux, FreeBSD or Windows with the same compiler.
As I understand it, OS X doesn't allow true dynamic linking, but it does
allow a compiler/linker to produce an executable binary which only
requires OS supplied libraries at execution time, and which includes all
the compiler specific libraries.
OSX doesn’t allow a completely statically-linked user space application.
but it does allow libgfortran and libquadmath to be statically linked - the
issue is that there’s no spec to deal with it.
Apparently the problem arises because the authors of libquadm don't want
users to use it in statically linked binaries,
no such issue, there is a static libquadmath.a installed ...
you should be able to work around this without changing the compiler or
rebuilding it,
find
/path/to/compiler/install/lib/libgfortran.spec
make a copy of that (for backup only)
the file contains something like:
#
# This spec file is read by gfortran when linking.
# It is used to specify the libraries we need to link in, in the right
# order.
#
%rename lib liborig
*lib: -lquadmath -lm %(libgcc) %(liborig)
change the last line line to :
*lib: %{!static-libgfortran: -lquadmath } %{static-libgfortran:
libquadmath.a%s} %(libgcc) %(liborig)
===
and try your link again
look at the linked object with “otool -Lv executable” and you should see
that it only refers to libSystem.dylib.
HTH
Iain
