Re: [PATCH] [gfortran] Add support for allocate clause (OpenMP 5.0).
Hi Abid!
On 2022-01-11T22:31:54+, Hafiz Abid Qadeer wrote:
> From d1fb55bff497a20e6feefa50bd03890e7a903c0e Mon Sep 17 00:00:00 2001
> From: Hafiz Abid Qadeer
> Date: Fri, 24 Sep 2021 10:04:12 +0100
> Subject: [PATCH] [gfortran] Add support for allocate clause (OpenMP 5.0).
>
> This patch adds support for OpenMP 5.0 allocate clause for fortran. It does
> not
> yet support the allocator-modifier as specified in OpenMP 5.1. The allocate
> clause is already supported in C/C++.
> libgomp/ChangeLog:
>
> * testsuite/libgomp.fortran/allocate-1.c: New test.
> * testsuite/libgomp.fortran/allocate-1.f90: New test.
I'm seeing this test case randomly/non-deterministically FAIL to execute,
differently on different systems and runs, for example:
libgomp:
libgomp:
libgomp: Out of memory allocating 4 bytesOut of memory allocating 4 bytes
libgomp:
libgomp:
libgomp: Out of memory allocating 168 bytes
libgomp: Out of memory allocating 4 bytes
libgomp: Out of memory allocating 4 bytes
libgomp: Out of memory allocating 4 bytes
I'd assume there's some concurrency issue: the problem disappears if I
manually specify a lowerish 'OMP_NUM_THREADS', and conversely, on a
system where I don't normally see the FAILs, I can trigger them with a
largish 'OMP_NUM_THREADS', such as 'OMP_NUM_THREADS=18' and higher.
For example:
Thread 10 "a.out" hit Breakpoint 1, omp_aligned_alloc (alignment=4, size=4,
allocator=6326576) at [...]/source-gcc/libgomp/allocator.c:318
318 if (allocator_data)
(gdb) print *allocator_data
$1 = {memspace = omp_default_mem_space, alignment = 64, pool_size = 8192,
used_pool_size = 8188, fb_data = omp_null_allocator, sync_hint = 3, access = 7,
fallback = 12, pinned = 0, partition = 15}
Given the high 'used_pool_size', is that to be expected, and the test
case shouldn't be requesting "so much" memory? Or might the problem
actually be in 'libgomp/allocator.c' (not touched by your commit)?
All but Thread 10 are in 'gomp_team_barrier_wait_end' -- should memory
have been released at that point?
(gdb) thread apply 10 bt
Thread 10 (Thread 0x732e2700 (LWP 1601318)):
#0 omp_aligned_alloc (alignment=4, size=4, allocator=6326576) at
[...]/source-gcc/libgomp/allocator.c:320
#1 0x7790b4db in GOMP_alloc (alignment=4, size=4,
allocator=6326576) at [...]/source-gcc/libgomp/allocator.c:364
#2 0x00401f3f in foo_._omp_fn.3 () at
source-gcc/libgomp/testsuite/libgomp.fortran/allocate-1.f90:136
#3 0x778f31e6 in gomp_thread_start (xdata=) at
[...]/source-gcc/libgomp/team.c:129
#4 0x7789e609 in start_thread (arg=) at
pthread_create.c:477
#5 0x777c5293 in clone () at
../sysdeps/unix/sysv/linux/x86_64/clone.S:95
(gdb) thread apply 1 bt
Thread 1 (Thread 0x772ec1c0 (LWP 1601309)):
#0 futex_wait (val=96, addr=) at
[...]/source-gcc/libgomp/config/linux/x86/futex.h:97
#1 do_wait (val=96, addr=) at
[...]/source-gcc/libgomp/config/linux/wait.h:67
#2 gomp_team_barrier_wait_end (bar=, state=96) at
[...]/source-gcc/libgomp/config/linux/bar.c:112
#3 0x00401f53 in foo_._omp_fn.3 () at
source-gcc/libgomp/testsuite/libgomp.fortran/allocate-1.f90:136
#4 0x778ea4f2 in GOMP_parallel (fn=0x401e6b ,
data=0x7fffd450, num_threads=18, flags=0) at
[...]/source-gcc/libgomp/parallel.c:178
#5 0x004012ab in foo (x=42, p=..., q=..., px=2, h=6326576, fl=0)
at source-gcc/libgomp/testsuite/libgomp.fortran/allocate-1.f90:122
#6 0x004018e9 in MAIN__ () at
source-gcc/libgomp/testsuite/libgomp.fortran/allocate-1.f90:326
Manually compiling the test case, I see a lot of '-Wtabs' diagnostics
(can be ignored, I suppose), but also:
source-gcc/libgomp/testsuite/libgomp.fortran/allocate-1.f90:11:47:
11 | integer(c_int) function is_64bit_aligned (a) bind(C)
| 1
Warning: Variable ‘a’ at (1) is a dummy argument of the BIND(C) procedure
‘is_64bit_aligned’ but may not be C interoperable [-Wc-binding-type]
Is that something to worry about?
And:
source-gcc/libgomp/testsuite/libgomp.fortran/allocate-1.f90:31:19:
31 | integer :: n, n1, n2, n3, n4
| 1
Warning: Unused variable ‘n1’ declared at (1) [-Wunused-variable]
source-gcc/libgomp/testsuite/libgomp.fortran/allocate-1.f90:18:27:
18 | subroutine foo (x, p, q, px, h, fl)
| 1
Warning: Unused dummy argument ‘px’ at (1) [-Wunused-dummy-argument]
For reference, quoting below the new Fortran test case.
Grüße
Thomas
> --- /dev/null
> +++ b/libgomp/testsuite/libgomp.fortran/allocate-1.c
> @@ -0,0 +1,7 @@
> +#include
> +
> +int
> +is_64bit_aligned_ (uintptr_t a)
> +{
> + return ( (a & 0x3f) == 0);
> +}
> --- /dev/null
> +++ b/libgomp/testsuite/libgomp.fortran/allocate-1.f90
> @@ -0,0 +1,333 @@
> +!
Re: [PATCH] [gfortran] Add support for allocate clause (OpenMP 5.0).
On 21.01.22 18:15, Thomas Schwinge wrote:
source-gcc/libgomp/testsuite/libgomp.fortran/allocate-1.f90:11:47:
11 | integer(c_int) function is_64bit_aligned (a) bind(C)
| 1
Warning: Variable ‘a’ at (1) is a dummy argument of the BIND(C) procedure
‘is_64bit_aligned’ but may not be C interoperable [-Wc-binding-type]
Is that something to worry about?
I think it is not very elegant – but should be okay.
On the Fortran side:
integer(c_int) function is_64bit_aligned (a) bind(C)
import :: c_int
integer :: a
end
that matches 'int is_64bit_aligned (int *a);'
While 'integer' in principle may not be 'int',
the call by reference makes this independent of the
actually used integer kind.
HOWEVER: That interface it not used! While it
defines that interface in 'module m', there is
no 'use m' in 'subroutine foo'.
(or alternatively: 'foo' being after 'contains' inside
the 'module m' - and then 'use m' in the main program)
That means that 'is_64bit_aligned(...)' gets implicitly
types as 'integer' with unknown arguments, which get
passed by value. By gfortran convention, that function
has a tailing underscore.
That matches the C side, which such an underscore:
int
is_64bit_aligned_ (uintptr_t a)
{
return ( (a & 0x3f) == 0);
}
With pass by reference, a pointer is passed, which
should be handled by 'uintptr_t'.
* * *
Side remark: I really recommend 'implicit none'
when writing Fortran code - which disables implicit
typing. I personally have started to use
implicit none (type, external)
which also rejects 'call something()' unless
'something' has been explicitly declared, e.g. by
an interface block.
* * *
Side remark: A Fortran-only variant has been used in
libgomp/testsuite/libgomp.fortran/alloc-11.f90:
if (mod (TRANSFER (p, iptr), 64) /= 0)
As optimization, also 'iand(..., z'3f') == 0' would work ;-)
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
[PATCH] fortran: Extend -fconvert= option for ppc64le r16_ieee and r16_ibm
Hi!
This patch on top of the previously posted option handling changes patch
allows specifying -fconvert=swap,r16_ieee etc. (but will error on it
when not on powerpc64le because in the library such swapping is only
implemented for HAVE_REAL_17).
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2022-01-21 Jakub Jelinek
* lang.opt (fconvert=): Add EnumSet property and mention also
r16_ieee and r16_ibm arguments.
(big-endian, little-endian, native, swap): Add Set(1) property.
(r16_ieee, r16_ibm): New EnumValue entries with Set(2) property.
* trans-types.cc (gfc_init_kinds): Emit gfc_fatal_error for
-fconvert=r16_ieee or -fconvert=r16_ibm when R16_IEEE <=> R16_IBM
conversions aren't supported.
--- gcc/fortran/lang.opt.jj 2022-01-11 23:49:52.167824673 +0100
+++ gcc/fortran/lang.opt2022-01-21 15:15:15.494099716 +0100
@@ -421,23 +421,29 @@ Fortran
Produce a warning at runtime if a array temporary has been created for a
procedure argument.
fconvert=
-Fortran RejectNegative Joined Enum(gfc_convert) Var(flag_convert)
Init(GFC_FLAG_CONVERT_NATIVE)
--fconvert= The endianness used for
unformatted files.
+Fortran RejectNegative Joined Enum(gfc_convert) EnumSet Var(flag_convert)
Init(GFC_FLAG_CONVERT_NATIVE)
+-fconvert= The
endianness used for unformatted files.
Enum
Name(gfc_convert) Type(enum gfc_convert) UnknownError(Unrecognized option to
endianness value: %qs)
EnumValue
-Enum(gfc_convert) String(big-endian) Value(GFC_FLAG_CONVERT_BIG)
+Enum(gfc_convert) String(big-endian) Value(GFC_FLAG_CONVERT_BIG) Set(1)
EnumValue
-Enum(gfc_convert) String(little-endian) Value(GFC_FLAG_CONVERT_LITTLE)
+Enum(gfc_convert) String(little-endian) Value(GFC_FLAG_CONVERT_LITTLE) Set(1)
EnumValue
-Enum(gfc_convert) String(native) Value(GFC_FLAG_CONVERT_NATIVE)
+Enum(gfc_convert) String(native) Value(GFC_FLAG_CONVERT_NATIVE) Set(1)
EnumValue
-Enum(gfc_convert) String(swap) Value(GFC_FLAG_CONVERT_SWAP)
+Enum(gfc_convert) String(swap) Value(GFC_FLAG_CONVERT_SWAP) Set(1)
+
+EnumValue
+Enum(gfc_convert) String(r16_ieee) Value(GFC_FLAG_CONVERT_R16_IEEE) Set(2)
+
+EnumValue
+Enum(gfc_convert) String(r16_ibm) Value(GFC_FLAG_CONVERT_R16_IBM) Set(2)
fcray-pointer
Fortran Var(flag_cray_pointer)
--- gcc/fortran/trans-types.cc.jj 2022-01-18 11:58:59.579982099 +0100
+++ gcc/fortran/trans-types.cc 2022-01-21 20:26:29.438558960 +0100
@@ -527,6 +527,9 @@ gfc_init_kinds (void)
}
}
}
+ else if ((flag_convert & (GFC_CONVERT_R16_IEEE | GFC_CONVERT_R16_IBM)) != 0)
+gfc_fatal_error ("%<-fconvert=r16_ieee%> or %<-fconvert=r16_ibm%> not "
+"supported on this architecture");
/* Choose the default integer kind. We choose 4 unless the user directs us
otherwise. Even if the user specified that the default integer kind is 8,
Jakub
