Re: [PATCH] fortran: Expand ieee_arithmetic module's ieee_value inline [PR106579]
Hi,
>> Why looping over fields? The class type is a simple type with only one
>> member (and it should be an integer, we can assert that).
>
> I wanted to make sure it has exactly one field.
> The ieee_arithmetic.F90 module in libgfortran surely does that, but I've
> been worrying about some user overriding that module with something
> different.
In Fortran world it would be a rare and crazy thing to do, but I see. Could you
add a comment (pointing out to the type definition in libgfortran)?
> The libgfortran version had default: label:
>switch (type) \
>{ \
> case IEEE_SIGNALING_NAN: \
>return __builtin_nans ## SUFFIX (""); \
> case IEEE_QUIET_NAN: \
>return __builtin_nan ## SUFFIX (""); \
> case IEEE_NEGATIVE_INF: \
>return - __builtin_inf ## SUFFIX (); \
> case IEEE_NEGATIVE_NORMAL: \
>return -42; \
> case IEEE_NEGATIVE_DENORMAL: \
>return -(GFC_REAL_ ## TYPE ## _TINY) / 2; \
> case IEEE_NEGATIVE_ZERO: \
>return -(GFC_REAL_ ## TYPE) 0; \
> case IEEE_POSITIVE_ZERO: \
>return 0; \
> case IEEE_POSITIVE_DENORMAL: \
>return (GFC_REAL_ ## TYPE ## _TINY) / 2; \
> case IEEE_POSITIVE_NORMAL: \
>return 42; \
> case IEEE_POSITIVE_INF: \
>return __builtin_inf ## SUFFIX (); \
> default: \
>return 0; \
>} \
> and I've tried to traslate that into what it generates.
OK, that makes sense. But:
> There is at least the IEEE_OTHER_VALUE (aka 0) value
> that isn't covered in the switch, but it is just an integer
> under the hood, so it could have any other value.
I think I originally included the default for IEEE_OTHER_VALUE, but the
standard says IEEE_OTHER_VALUE as an argument to IEE_VALUE is not allowed. If
removing the default would make the generated code shorter, I suggest we do it;
otherwise let’s keep it.
With those two caveats, the patch is OK. We shouldn’t touch the library code
for now, but when the patch is committed we can add the removal of IEEE_VALUE
and IEEE_CLASS from the library to this list:
https://gcc.gnu.org/wiki/LibgfortranAbiCleanup
FX
[Patch] Fortran: OpenMP fix declare simd inside modules and absent linear [PR106566]
This patch fixes two issues – the first was reported to me by email but it
also shows up in the official OpenMP examples (see PR).
Namely: Inside a module, 'gfc_match(" ( %s )")' fails as the symbol is already
host associated. (The symbol is the current procedure name.)
Solution: Match with passing (permit) host_assoc = true to the match function
instead of 'false' as done with '%s'.
Afterwards, it was failing when folding a NULL_TREE. Solution: Init the linear
step with 1 in that case.
OK for mainline?
* * *
I have not checked GCC < mainline. The current example is OpenMP 5.2 only
and only supported since June 7, 2022 in C/C++ and July 4 for Fortran.
However, I assume the same issue also affects GCC < 13 with a tailored
testcase. - If there is the sentiment to fix it also for older GCC,
I can come up with modified testcases and a GCC 12 (and GCC 11?) patch.
Thoughts?
Tobias
-
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634
München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas
Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht
München, HRB 106955
Fortran: OpenMP fix declare simd inside modules and absent linear [PR106566]
gcc/fortran/ChangeLog:
PR fortran/106566
* openmp.cc (gfc_match_omp_declare_simd): Accept module procedures.
* trans-openmp.cc (gfc_trans_omp_clauses): Fix declare simd without
linear-step value.
gcc/testsuite/ChangeLog:
PR fortran/106566
* gfortran.dg/gomp/declare-simd-4.f90: New test.
* gfortran.dg/gomp/declare-simd-5.f90: New test.
gcc/fortran/openmp.cc | 8 +++-
gcc/fortran/trans-openmp.cc | 2 +
gcc/testsuite/gfortran.dg/gomp/declare-simd-4.f90 | 42 +++
gcc/testsuite/gfortran.dg/gomp/declare-simd-5.f90 | 49 +++
4 files changed, 99 insertions(+), 2 deletions(-)
diff --git a/gcc/fortran/openmp.cc b/gcc/fortran/openmp.cc
index a7eb6c3e8f4..e430f4c49dd 100644
--- a/gcc/fortran/openmp.cc
+++ b/gcc/fortran/openmp.cc
@@ -4213,9 +4213,13 @@ gfc_match_omp_declare_simd (void)
gfc_omp_declare_simd *ods;
bool needs_space = false;
- switch (gfc_match (" ( %s ) ", &proc_name))
+ switch (gfc_match (" ( "))
{
-case MATCH_YES: break;
+case MATCH_YES:
+ if (gfc_match_symbol (&proc_name, /* host assoc = */ true) != MATCH_YES
+ || gfc_match (" ) ") != MATCH_YES)
+ return MATCH_ERROR;
+ break;
case MATCH_NO: proc_name = NULL; needs_space = true; break;
case MATCH_ERROR: return MATCH_ERROR;
}
diff --git a/gcc/fortran/trans-openmp.cc b/gcc/fortran/trans-openmp.cc
index de27ed52c02..22e6dd254c7 100644
--- a/gcc/fortran/trans-openmp.cc
+++ b/gcc/fortran/trans-openmp.cc
@@ -2798,6 +2798,8 @@ gfc_trans_omp_clauses (stmtblock_t *block, gfc_omp_clauses *clauses,
}
else
{
+ if (last_step == NULL_TREE)
+ last_step = size_one_node;
if (kind == OMP_CLAUSE_LINEAR_REF)
{
tree type;
diff --git a/gcc/testsuite/gfortran.dg/gomp/declare-simd-4.f90 b/gcc/testsuite/gfortran.dg/gomp/declare-simd-4.f90
new file mode 100644
index 000..44132525963
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/gomp/declare-simd-4.f90
@@ -0,0 +1,42 @@
+! { dg-do compile }
+! { dg-additional-options "-fdump-tree-gimple" }
+!
+! PR fortran/106566
+!
+! { dg-final { scan-tree-dump-times "__attribute__\\(\\(omp declare simd \\(linear\\(0:ref,step\\(4\\)\\) simdlen\\(8\\)\\)\\)\\)" 2 "gimple" } }
+! { dg-final { scan-tree-dump-times "__attribute__\\(\\(omp declare simd \\(linear\\(0:ref,step\\(8\\)\\) simdlen\\(8\\)\\)\\)\\)" 2 "gimple" } }
+
+subroutine add_one2(p)
+ implicit none
+ !$omp declare simd(add_one2) linear(p: ref) simdlen(8)
+ integer :: p
+
+ p = p + 1
+end subroutine
+
+subroutine linear_add_one2(p)
+ implicit none
+ !$omp declare simd(linear_add_one2) linear(p: ref, step(2)) simdlen(8)
+ integer :: p
+
+ p = p + 1
+end subroutine
+
+module m
+ integer, parameter :: NN = 1023
+ integer :: a(NN)
+contains
+ subroutine module_add_one2(q)
+implicit none
+!$omp declare simd(module_add_one2) linear(q: ref) simdlen(8)
+integer :: q
+q = q + 1
+ end subroutine
+
+ subroutine linear_add_one2(q)
+implicit none
+!$omp declare simd(linear_add_one2) linear(q: ref, step(2)) simdlen(8)
+integer :: q
+q = q + 1
+ end subroutine
+end module
diff --git a/gcc/testsuite/gfortran.dg/gomp/declare-simd-5.f90 b/gcc/testsuite/gfortran.dg/gomp/declare-simd-5.f90
new file mode 100644
index 000..f5880f50090
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/gomp/declare-simd-5.f90
@@ -0,0 +1,49 @@
+! { dg-do compile }
+!
+! PR fortran/106566
+!
+
+subroutine add_one2(p)
+ implicit none
+ procedure(add_one2) :: ext1
+ !$omp declare simd(ext1) linear(p: ref) simdlen(8) ! { dg-error "OMP DECLARE SIMD should refer to containing procedure 'add_one2'" }
+ integer :: p
+
+ p = p + 1
+end subroutine
+
+subrou
[PATCH] fortran: Add -static-libquadmath support [PR46539]
Hi!
The following patch is a revival of the
https://gcc.gnu.org/legacy-ml/gcc-patches/2014-10/msg00771.html
patch. While trunk configured against recent glibc and with linker
--as-needed support doesn't really need to link against -lquadmath
anymore, there are still other targets where libquadmath is still in
use.
As has been discussed, making -static-libgfortran imply statically
linking both libgfortran and libquadmath is undesirable because of
the significant licensing differences between the 2 libraries.
Compared to the 2014 patch, this one doesn't handle -lquadmath
addition in the driver, which to me looks incorrect, libgfortran
configure determines where in libgfortran.spec -lquadmath should
be present if at all and with what it should be wrapper, but
analyzes gfortran -### -static-libgfortran stderr and based on
that figures out what gcc/configure.ac determined.
So far slightly tested on x86_64-linux (and will bootstrap/regtest
it there tonight), but I unfortunately don't have a way to test it
e.g. on Darwin.
Thoughts on this?
2022-08-16 Francois-Xavier Coudert
Jakub Jelinek
PR fortran/46539
gcc/
* common.opt (static-libquadmath): New option.
* gcc.c (driver_handle_option): Always accept -static-libquadmath.
* config/darwin.h (LINK_SPEC): Handle -static-libquadmath.
gcc/fortran/
* lang.opt (static-libquadmath): New option.
* invoke.texi (-static-libquadmath): Document it.
* options.c (gfc_handle_option): Error out if -static-libquadmath
is passed but we do not support it.
libgfortran/
* acinclude.m4 (LIBQUADSPEC): From $FC -static-libgfortran -###
output determine -Bstatic/-Bdynamic, -bstatic/-bdynamic,
-aarchive_shared/-adefault linker support or Darwin remapping
of -lgfortran to libgfortran.a%s and use that around or instead
of -lquadmath in LIBQUADSPEC.
* configure: Regenerated.
--- gcc/common.opt.jj 2022-06-27 11:18:02.050066582 +0200
+++ gcc/common.opt 2022-08-16 14:51:04.611673800 +0200
@@ -3601,6 +3601,10 @@ static-libphobos
Driver
; Documented for D, but always accepted by driver.
+static-libquadmath
+Driver
+; Documented for Fortran, but always accepted by driver.
+
static-libstdc++
Driver
--- gcc/gcc.cc.jj 2022-08-11 09:57:24.765334380 +0200
+++ gcc/gcc.cc 2022-08-16 14:57:54.708327024 +0200
@@ -4585,12 +4585,14 @@ driver_handle_option (struct gcc_options
case OPT_static_libgcc:
case OPT_shared_libgcc:
case OPT_static_libgfortran:
+case OPT_static_libquadmath:
case OPT_static_libphobos:
case OPT_static_libstdc__:
/* These are always valid, since gcc.cc itself understands the
first two, gfortranspec.cc understands -static-libgfortran,
-d-spec.cc understands -static-libphobos, and g++spec.cc
-understands -static-libstdc++ */
+d-spec.cc understands -static-libphobos, g++spec.cc
+understands -static-libstdc++ and libgfortran.spec handles
+-static-libquadmath. */
validated = true;
break;
--- gcc/config/darwin.h.jj 2022-08-16 14:51:14.529544492 +0200
+++ gcc/config/darwin.h 2022-08-16 14:53:54.402460097 +0200
@@ -443,6 +443,7 @@ extern GTY(()) int darwin_ms_struct;
%:replace-outfile(-lobjc libobjc-gnu.a%s); \
:%:replace-outfile(-lobjc -lobjc-gnu )}}\
%{static|static-libgcc|static-libgfortran:%:replace-outfile(-lgfortran
libgfortran.a%s)}\
+ %{static|static-libgcc|static-libgfortran:%:replace-outfile(-lquadmath
libquadmath.a%s)}\
%{static|static-libgcc|static-libphobos:%:replace-outfile(-lgphobos
libgphobos.a%s)}\
%{static|static-libgcc|static-libstdc++|static-libgfortran:%:replace-outfile(-lgomp
libgomp.a%s)}\
%{static|static-libgcc|static-libstdc++:%:replace-outfile(-lstdc++
libstdc++.a%s)}\
--- gcc/fortran/lang.opt.jj 2022-02-04 14:36:55.050604670 +0100
+++ gcc/fortran/lang.opt2022-08-16 14:52:52.459267705 +0200
@@ -863,6 +863,10 @@ static-libgfortran
Fortran
Statically link the GNU Fortran helper library (libgfortran).
+static-libquadmath
+Fortran
+Statically link the GCC Quad-Precision Math Library (libquadmath).
+
std=f2003
Fortran
Conform to the ISO Fortran 2003 standard.
--- gcc/fortran/options.cc.jj 2022-01-18 11:58:59.568982256 +0100
+++ gcc/fortran/options.cc 2022-08-16 14:56:22.807525218 +0200
@@ -692,6 +692,13 @@ gfc_handle_option (size_t scode, const c
#endif
break;
+case OPT_static_libquadmath:
+#ifndef HAVE_LD_STATIC_DYNAMIC
+ gfc_fatal_error ("%<-static-libquadmath%> is not supported in this "
+ "configuration");
+#endif
+ break;
+
case OPT_fintrinsic_modules_path:
case OPT_fintrinsic_modules_path_:
--- gcc/fortran/invoke.texi.jj 2022-05-09 09:09:20.312473272 +0200
+++ gcc/fortran/invoke.texi 2022-08-16 16:12:47.638203577 +0200
@@ -170,7 +170,7 @@ and warnings}.
@it
Re: [Patch] Fortran: OpenMP fix declare simd inside modules and absent linear step [PR106566]
Fixed subject line: "absent linear" should be "absent linear step" in the
subject line;
i.e. with "step" added: "Fortran: OpenMP fix declare simd inside modules and absent
linear step [PR106566]"
I have also decided to move the 'step = 1' to openmp.cc, which also set it
before with
the old pre-OpenMP 5.2 syntax.
I also added a pre-OpenMP-5.2-syntax example.
* * *
For GCC 12 (and GCC 11), only the '%s' fix and the third, now added example
apply;
for the 5.1 syntax, 'step' was already set.
OK? And thoughts regarding the backports (none? Only 12? Or 11+12?)?
Tobias
-
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634
München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas
Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht
München, HRB 106955
Fortran: OpenMP fix declare simd inside modules and absent linear step [PR106566]
gcc/fortran/ChangeLog:
PR fortran/106566
* openmp.cc (gfc_match_omp_clauses): Fix setting linear-step value
to 1 when not specified.
(gfc_match_omp_declare_simd): Accept module procedures.
gcc/testsuite/ChangeLog:
PR fortran/106566
* gfortran.dg/gomp/declare-simd-4.f90: New test.
* gfortran.dg/gomp/declare-simd-5.f90: New test.
* gfortran.dg/gomp/declare-simd-6.f90: New test.
gcc/fortran/openmp.cc | 10 +++--
gcc/testsuite/gfortran.dg/gomp/declare-simd-4.f90 | 42 +++
gcc/testsuite/gfortran.dg/gomp/declare-simd-5.f90 | 49 +++
gcc/testsuite/gfortran.dg/gomp/declare-simd-6.f90 | 42 +++
4 files changed, 140 insertions(+), 3 deletions(-)
diff --git a/gcc/fortran/openmp.cc b/gcc/fortran/openmp.cc
index a7eb6c3e8f4..594907714ff 100644
--- a/gcc/fortran/openmp.cc
+++ b/gcc/fortran/openmp.cc
@@ -2480,7 +2480,7 @@ gfc_match_omp_clauses (gfc_omp_clauses **cp, const omp_mask mask,
goto error;
}
}
- else
+ if (step == NULL)
{
step = gfc_get_constant_expr (BT_INTEGER,
gfc_default_integer_kind,
@@ -4213,9 +4213,13 @@ gfc_match_omp_declare_simd (void)
gfc_omp_declare_simd *ods;
bool needs_space = false;
- switch (gfc_match (" ( %s ) ", &proc_name))
+ switch (gfc_match (" ( "))
{
-case MATCH_YES: break;
+case MATCH_YES:
+ if (gfc_match_symbol (&proc_name, /* host assoc = */ true) != MATCH_YES
+ || gfc_match (" ) ") != MATCH_YES)
+ return MATCH_ERROR;
+ break;
case MATCH_NO: proc_name = NULL; needs_space = true; break;
case MATCH_ERROR: return MATCH_ERROR;
}
diff --git a/gcc/testsuite/gfortran.dg/gomp/declare-simd-4.f90 b/gcc/testsuite/gfortran.dg/gomp/declare-simd-4.f90
new file mode 100644
index 000..44132525963
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/gomp/declare-simd-4.f90
@@ -0,0 +1,42 @@
+! { dg-do compile }
+! { dg-additional-options "-fdump-tree-gimple" }
+!
+! PR fortran/106566
+!
+! { dg-final { scan-tree-dump-times "__attribute__\\(\\(omp declare simd \\(linear\\(0:ref,step\\(4\\)\\) simdlen\\(8\\)\\)\\)\\)" 2 "gimple" } }
+! { dg-final { scan-tree-dump-times "__attribute__\\(\\(omp declare simd \\(linear\\(0:ref,step\\(8\\)\\) simdlen\\(8\\)\\)\\)\\)" 2 "gimple" } }
+
+subroutine add_one2(p)
+ implicit none
+ !$omp declare simd(add_one2) linear(p: ref) simdlen(8)
+ integer :: p
+
+ p = p + 1
+end subroutine
+
+subroutine linear_add_one2(p)
+ implicit none
+ !$omp declare simd(linear_add_one2) linear(p: ref, step(2)) simdlen(8)
+ integer :: p
+
+ p = p + 1
+end subroutine
+
+module m
+ integer, parameter :: NN = 1023
+ integer :: a(NN)
+contains
+ subroutine module_add_one2(q)
+implicit none
+!$omp declare simd(module_add_one2) linear(q: ref) simdlen(8)
+integer :: q
+q = q + 1
+ end subroutine
+
+ subroutine linear_add_one2(q)
+implicit none
+!$omp declare simd(linear_add_one2) linear(q: ref, step(2)) simdlen(8)
+integer :: q
+q = q + 1
+ end subroutine
+end module
diff --git a/gcc/testsuite/gfortran.dg/gomp/declare-simd-5.f90 b/gcc/testsuite/gfortran.dg/gomp/declare-simd-5.f90
new file mode 100644
index 000..f5880f50090
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/gomp/declare-simd-5.f90
@@ -0,0 +1,49 @@
+! { dg-do compile }
+!
+! PR fortran/106566
+!
+
+subroutine add_one2(p)
+ implicit none
+ procedure(add_one2) :: ext1
+ !$omp declare simd(ext1) linear(p: ref) simdlen(8) ! { dg-error "OMP DECLARE SIMD should refer to containing procedure 'add_one2'" }
+ integer :: p
+
+ p = p + 1
+end subroutine
+
+subroutine linear_add_one2(p)
+ implicit none
+ procedure(linear_add_one2) :: ext2
+ !$omp declare simd(ext2) linear(p: ref, step(2)) simdlen(8) ! { dg-error "OMP DECLARE SIMD should refer to containing procedure 'linear_add_one2'" }
+ integer :: p
+
+ p = p + 1
+end subroutine
+
+module m
+ integer, parameter :: NN = 1023
+ integer :: a(NN)
+contains
+ subroutine some_proc(r)
+integer :: r
+ end subroutine
+ subroutine mod
