public inbox for fortran@gcc.gnu.org
 help / color / mirror / Atom feed
From: Tobias Burnus <tobias@codesourcery.com>
To: 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: Tue, 25 Jan 2022 11:32:17 +0100	[thread overview]
Message-ID: <f021572d-00f9-60f3-e7e1-928cc135e956@codesourcery.com> (raw)
In-Reply-To: <87r18wtbol.fsf@euler.schwinge.homeip.net>

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
==================

-----------------
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

  reply	other threads:[~2022-01-25 10:32 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 [this message]
2022-01-31 19:13                     ` Hafiz Abid Qadeer
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=f021572d-00f9-60f3-e7e1-928cc135e956@codesourcery.com \
    --to=tobias@codesourcery.com \
    --cc=abidh@codesourcery.com \
    --cc=fortran@gcc.gnu.org \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jakub@redhat.com \
    --cc=thomas@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).