From: Hafiz Abid Qadeer <abid_qadeer@mentor.com>
To: Tobias Burnus <tobias@codesourcery.com>,
Thomas Schwinge <thomas@codesourcery.com>,
Jakub Jelinek <jakub@redhat.com>,
Hafiz Abid Qadeer <abidh@codesourcery.com>
Cc: <gcc-patches@gcc.gnu.org>, <fortran@gcc.gnu.org>
Subject: Re: [PATCH] [gfortran] Add support for allocate clause (OpenMP 5.0).
Date: Mon, 31 Jan 2022 19:13:09 +0000 [thread overview]
Message-ID: <343f7669-0e6a-11eb-3580-e351711f02b9@mentor.com> (raw)
In-Reply-To: <f021572d-00f9-60f3-e7e1-928cc135e956@codesourcery.com>
[-- Attachment #1: Type: text/plain, Size: 4581 bytes --]
On 25/01/2022 10:32, Tobias Burnus wrote:
> On 25.01.22 10:19, Thomas Schwinge wrote:
>>> I am trying to figure out if the problem you observed
>>> is a general one or just specific to fortran testcase.
>> So, unless the '-fsanitize=thread' issues are bogus -- unlikely ;-) -- it
>> seems a latent issue generally, now fatal with
>> 'libgomp.fortran/allocate-1.f90'.
>
> There is one known issue with libgomp and TSAN (-fsanitize=thread)
> that I tend to forget about :-(
>
> That's according to Jakub, who wrote a while ago:
>
> "TSAN doesn't understand what libgomp is doing, unless built with --disable-linux-futex"
>
>
>
> However, I now tried to disable futex and still get the following.
> (First result for libgomp.c-c++-common/allocate-1.c).
>
> On the other hand, I have the feeling that the configure option is
> a no op for libgomp. This can also be seen in the configure.ac script,
> which only for libstdc++ uses the result and the others have a no-op
> call to 'true' (alias ':'):
>
> libgomp/configure.ac:GCC_LINUX_FUTEX(:)
> libitm/configure.ac:GCC_LINUX_FUTEX(:)
> libstdc++-v3/configure.ac:GCC_LINUX_FUTEX([AC_DEFINE(HAVE_LINUX_FUTEX, 1, [Define if futex syscall
> is available.])])
>
> (The check is not completely pointless as some checks are still done;
> e.g. 'SYS_gettid and SYS_futex required'.)
>
> (TSAN did find issues in libgomp in the past, however. But those
> habe been fixed.)
>
>
> Thus, there might or might not be an issue when TSAN reports one.
>
> * * *
>
> Glancing at the Fortran testcase, I noted the following,
> which probably does not cause the problems. But still,
> I want to mention it:
>
> !$omp parallel private (y, v) firstprivate (x) allocate (x, y, v)
> if (x /= 42) then
> stop 1
> end if
>
> v(1) = 7
> if ( (and(fl, 2) /= 0) .and. &
> ((is_64bit_aligned(x) == 0) .or. &
> (is_64bit_aligned(y) == 0) .or. &
> (is_64bit_aligned(v(1)) == 0))) then
> stop 2
> end if
>
> If one compares this with the C/C++ testcase, I note that there
> is a barrier before the alignment check in C/C++ but not in
> Fortran. Additionally, 'v(1) = 7' is set twice and the
> alignment check happens earlier than in C/C++. Not that that
> should really matter, but I just saw it.
>
>
> In C/C++:
> int v[x], w[x];
> ...
> v[0] = 7;
> v[41] = 8;
>
> In Fortran:
> integer, dimension(x) :: v
> ...
> v(1) = 7
> v(41) = 8
>
> where 'x == 42'. The Fortran version is not really wrong, but I think
> the idea is to set the first and last array element - and that's here
> v(42) and not v(41).
>
> BTW: Fortran permits to specify a different lower bound. When converting
> C/C++ testcases, it can be useful to use the same lower bound also in
> Fortran: integer :: v(0:x-1) (or: 'integer, dimension(0:x-1) :: v')
> uses then 0 ... 41 for the indices instead of 1 ... 42.
>
> But one has to be careful as Fortran uses the upper bound and C uses the
> number of elements. (Same with OpenMP array sections in Fortran vs. C.)
>
>
> Tobias
>
> PS: The promised data-race warning:
> ==================
> WARNING: ThreadSanitizer: data race (pid=4135381)
> Read of size 8 at 0x7ffc0888bdc0 by thread T10:
> #0 foo._omp_fn.2 libgomp.c-c++-common/allocate-1.c:47 (a.out+0x402c05)
> #1 gomp_thread_start ../../../repos/gcc/libgomp/team.c:129 (libgomp.so.1+0x1e5ed)
>
> Previous write of size 8 at 0x7ffc0888bdc0 by main thread:
> #0 foo._omp_fn.1 libgomp.c-c++-common/allocate-1.c:47 (a.out+0x402aee)
> #1 GOMP_teams_reg ../../../repos/gcc/libgomp/teams.c:51 (libgomp.so.1+0x3638c)
> #2 main libgomp.c-c++-common/allocate-1.c:366 (a.out+0x40273e)
>
> Location is stack of main thread.
>
> Location is global '<null>' at 0x000000000000 ([stack]+0x1ddc0)
>
> Thread T10 (tid=4135398, running) created by main thread at:
> #0 pthread_create ../../../../repos/gcc/libsanitizer/tsan/tsan_interceptors_posix.cpp:1001
> (libtsan.so.2+0x62c76)
> #1 gomp_team_start ../../../repos/gcc/libgomp/team.c:858 (libgomp.so.1+0x1ec18)
> #2 main libgomp.c-c++-common/allocate-1.c:366 (a.out+0x40273e)
>
> SUMMARY: ThreadSanitizer: data race libgomp.c-c++-common/allocate-1.c:47 in foo._omp_fn.2
> ==================
>
Problem was with the pool_size trait. It has limited size which this testcase exceeded. I have
removed it now which seems to fix the problem. Ok to commit the attached patch?
Thanks,
--
Hafiz Abid Qadeer
Mentor, a Siemens Business
[-- Attachment #2: 0001-libgomp-Fix-testcase-to-remove-out-of-memory-error.patch --]
[-- Type: text/x-patch, Size: 1711 bytes --]
From 98b5493bd94520dd78b3963d3a4e67cba6bfb6aa Mon Sep 17 00:00:00 2001
From: Hafiz Abid Qadeer <abidh@codesourcery.com>
Date: Mon, 31 Jan 2022 19:02:14 +0000
Subject: [PATCH] [libgomp] Fix testcase to remove out of memory error.
Thomas reported in
https://gcc.gnu.org/pipermail/gcc-patches/2022-January/589039.html
that this testcase is randomly failing. The problem was fixed pool
size which was exhausted when there were a lot of threads. Fixed it
by removing pool_size trait which causes default pool size to be used
which should be big enough.
libgomp/ChangeLog:
* testsuite/libgomp.fortran/allocate-1.f90: Remove pool_size trait.
---
libgomp/testsuite/libgomp.fortran/allocate-1.f90 | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/libgomp/testsuite/libgomp.fortran/allocate-1.f90 b/libgomp/testsuite/libgomp.fortran/allocate-1.f90
index 35d1750b878..04bf2307462 100644
--- a/libgomp/testsuite/libgomp.fortran/allocate-1.f90
+++ b/libgomp/testsuite/libgomp.fortran/allocate-1.f90
@@ -313,13 +313,12 @@ program main
integer, dimension(4) :: p
integer, dimension(4) :: q
- type (omp_alloctrait) :: traits(3)
+ type (omp_alloctrait) :: traits(2)
integer (omp_allocator_handle_kind) :: a
traits = [omp_alloctrait (omp_atk_alignment, 64), &
- omp_alloctrait (omp_atk_fallback, omp_atv_null_fb), &
- omp_alloctrait (omp_atk_pool_size, 8192)]
- a = omp_init_allocator (omp_default_mem_space, 3, traits)
+ omp_alloctrait (omp_atk_fallback, omp_atv_null_fb)]
+ a = omp_init_allocator (omp_default_mem_space, 2, traits)
if (a == omp_null_allocator) stop 1
call omp_set_default_allocator (omp_default_mem_alloc);
--
2.25.1
next prev parent reply other threads:[~2022-01-31 19:13 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20211022130502.2211568-1-abidh@codesourcery.com>
2021-10-22 13:28 ` Tobias Burnus
[not found] ` <20211102162714.GF304296@tucnak>
[not found] ` <e3c9083c-0906-699a-b437-84a49ece33b8@mentor.com>
[not found] ` <20211220200650.GN2646553@tucnak>
[not found] ` <fddcdfcf-3fab-1674-722e-2756a1d6aef8@mentor.com>
2022-01-14 9:10 ` Thomas Schwinge
2022-01-14 11:45 ` Tobias Burnus
2022-01-14 11:55 ` Jakub Jelinek
2022-01-14 12:20 ` Tobias Burnus
2022-01-17 14:01 ` Hafiz Abid Qadeer
2022-01-21 17:15 ` Thomas Schwinge
2022-01-21 17:43 ` Tobias Burnus
2022-01-24 8:45 ` Tobias Burnus
2022-01-24 12:54 ` Hafiz Abid Qadeer
2022-01-25 9:19 ` Thomas Schwinge
2022-01-25 10:32 ` Tobias Burnus
2022-01-31 19:13 ` Hafiz Abid Qadeer [this message]
2022-02-04 9:46 ` Thomas Schwinge
2022-02-04 11:25 ` Hafiz Abid Qadeer
2022-02-05 19:09 ` Hafiz Abid Qadeer
2022-02-16 10:29 ` Hafiz Abid Qadeer
2022-02-04 9:37 ` Thomas Schwinge
2022-02-04 13:57 ` [committed] libgomp.fortran/allocate-1.f90: Minor cleanup (was: Re: [PATCH] [gfortran] Add support for allocate clause (OpenMP 5.0).) Tobias Burnus
2022-02-04 15:33 ` Thomas Schwinge
2022-02-04 16:34 ` Tobias Burnus
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=343f7669-0e6a-11eb-3580-e351711f02b9@mentor.com \
--to=abid_qadeer@mentor.com \
--cc=abidh@codesourcery.com \
--cc=fortran@gcc.gnu.org \
--cc=gcc-patches@gcc.gnu.org \
--cc=jakub@redhat.com \
--cc=thomas@codesourcery.com \
--cc=tobias@codesourcery.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).