From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from esa3.mentor.iphmx.com (esa3.mentor.iphmx.com [68.232.137.180]) by sourceware.org (Postfix) with ESMTPS id 76ABB3858C1F; Mon, 31 Jan 2022 19:13:16 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 76ABB3858C1F Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=mentor.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=mentor.com IronPort-SDR: Y2zPW4H+TvyJkDqwnL1cZADoz1Q6pVaiKrcNu06Lv1X1gSziBJgXl8QJsszls7zMjTrv6ZO9f/ lzeywdb5YKgqtu1n3gjFunD/KaZJZIwZqPEFyLBY9UDDojMj/4p9CXdoRaN7PXv8IZDDyhkOAb FO02R2F0AgrAKWFVdPXKMcRj6ZzwkOdfWDJjrrssS4qfUxsHx7z5iS+MzX43X4l5SeKHPtgUWI U9+FkBAGpYhU3zdhUerG/TzxoU9UE8zSjkpbiM5H5OVZGDWd7AEcDr0S0xtaX7gdy/XZRxbFZR tk8SZ+sKWGibTLfTlQcupLFk X-IronPort-AV: E=Sophos;i="5.88,331,1635235200"; d="scan'208,223";a="71263170" Received: from orw-gwy-02-in.mentorg.com ([192.94.38.167]) by esa3.mentor.iphmx.com with ESMTP; 31 Jan 2022 11:13:15 -0800 IronPort-SDR: ZyOzdS7vl7Ku7UznKiCows+nJJgGDdXual6ewjiPueCio/0So2DQHLzRPKnMvnNd+MuVVBPGA8 nX6+He13Ajy6AFl+duabQnplvj0MSHWTgZjJBbf7z76VEoO273Gi8YTHyCeNFnToaR7daERzh0 JcTqOX+fxqKeSBs+XbWXsVr3O4N4e3X6VkUCungj5GuHfxV11yDK6OOruv4SkqdPDWP2WZ/nsx g7+yqWFoewiRGGPyTgmUKoNbbtFmws7MLqltk1QMY1tZ2YofPVE9WMLv3mgfVy+Kq4eSnVzn1+ T2s= Content-Type: multipart/mixed; boundary="------------flj7rL4DJHgnaLFZ6ocoowbz" Message-ID: <343f7669-0e6a-11eb-3580-e351711f02b9@mentor.com> Date: Mon, 31 Jan 2022 19:13:09 +0000 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.5.0 Subject: Re: [PATCH] [gfortran] Add support for allocate clause (OpenMP 5.0). Content-Language: en-US To: Tobias Burnus , Thomas Schwinge , Jakub Jelinek , Hafiz Abid Qadeer CC: , References: <20211022130502.2211568-1-abidh@codesourcery.com> <20211102162714.GF304296@tucnak> <20211220200650.GN2646553@tucnak> <8735lh6mcx.fsf@euler.schwinge.homeip.net> <48d8c123-fa4f-d4a3-17de-b082de32f0bf@codesourcery.com> <87r18wtbol.fsf@euler.schwinge.homeip.net> From: Hafiz Abid Qadeer In-Reply-To: X-Originating-IP: [137.202.0.90] X-ClientProxiedBy: SVR-IES-MBX-08.mgc.mentorg.com (139.181.222.8) To SVR-IES-MBX-03.mgc.mentorg.com (139.181.222.3) X-Spam-Status: No, score=-10.2 required=5.0 tests=BAYES_00, BODY_8BITS, GIT_PATCH_0, KAM_DMARC_STATUS, KAM_SHORT, NICE_REPLY_A, SPF_HELO_PASS, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: fortran@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Fortran mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 31 Jan 2022 19:13:20 -0000 --------------flj7rL4DJHgnaLFZ6ocoowbz Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 8bit 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 '' 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 --------------flj7rL4DJHgnaLFZ6ocoowbz Content-Type: text/x-patch; charset="UTF-8"; name="0001-libgomp-Fix-testcase-to-remove-out-of-memory-error.patch" Content-Disposition: attachment; filename*0="0001-libgomp-Fix-testcase-to-remove-out-of-memory-error.patc"; filename*1="h" Content-Transfer-Encoding: base64 RnJvbSA5OGI1NDkzYmQ5NDUyMGRkNzhiMzk2M2QzYTRlNjdjYmE2YmZiNmFhIE1vbiBTZXAg MTcgMDA6MDA6MDAgMjAwMQpGcm9tOiBIYWZpeiBBYmlkIFFhZGVlciA8YWJpZGhAY29kZXNv dXJjZXJ5LmNvbT4KRGF0ZTogTW9uLCAzMSBKYW4gMjAyMiAxOTowMjoxNCArMDAwMApTdWJq ZWN0OiBbUEFUQ0hdIFtsaWJnb21wXSBGaXggdGVzdGNhc2UgdG8gcmVtb3ZlIG91dCBvZiBt ZW1vcnkgZXJyb3IuCgpUaG9tYXMgcmVwb3J0ZWQgaW4KaHR0cHM6Ly9nY2MuZ251Lm9yZy9w aXBlcm1haWwvZ2NjLXBhdGNoZXMvMjAyMi1KYW51YXJ5LzU4OTAzOS5odG1sCnRoYXQgdGhp cyB0ZXN0Y2FzZSBpcyByYW5kb21seSBmYWlsaW5nLiBUaGUgcHJvYmxlbSB3YXMgZml4ZWQg cG9vbApzaXplIHdoaWNoIHdhcyBleGhhdXN0ZWQgd2hlbiB0aGVyZSB3ZXJlIGEgbG90IG9m IHRocmVhZHMuIEZpeGVkIGl0CmJ5IHJlbW92aW5nIHBvb2xfc2l6ZSB0cmFpdCB3aGljaCBj YXVzZXMgZGVmYXVsdCBwb29sIHNpemUgdG8gYmUgdXNlZAp3aGljaCBzaG91bGQgYmUgYmln IGVub3VnaC4KCmxpYmdvbXAvQ2hhbmdlTG9nOgoKCSogdGVzdHN1aXRlL2xpYmdvbXAuZm9y dHJhbi9hbGxvY2F0ZS0xLmY5MDogUmVtb3ZlIHBvb2xfc2l6ZSB0cmFpdC4KLS0tCiBsaWJn b21wL3Rlc3RzdWl0ZS9saWJnb21wLmZvcnRyYW4vYWxsb2NhdGUtMS5mOTAgfCA3ICsrKy0t LS0KIDEgZmlsZSBjaGFuZ2VkLCAzIGluc2VydGlvbnMoKyksIDQgZGVsZXRpb25zKC0pCgpk aWZmIC0tZ2l0IGEvbGliZ29tcC90ZXN0c3VpdGUvbGliZ29tcC5mb3J0cmFuL2FsbG9jYXRl LTEuZjkwIGIvbGliZ29tcC90ZXN0c3VpdGUvbGliZ29tcC5mb3J0cmFuL2FsbG9jYXRlLTEu ZjkwCmluZGV4IDM1ZDE3NTBiODc4Li4wNGJmMjMwNzQ2MiAxMDA2NDQKLS0tIGEvbGliZ29t cC90ZXN0c3VpdGUvbGliZ29tcC5mb3J0cmFuL2FsbG9jYXRlLTEuZjkwCisrKyBiL2xpYmdv bXAvdGVzdHN1aXRlL2xpYmdvbXAuZm9ydHJhbi9hbGxvY2F0ZS0xLmY5MApAQCAtMzEzLDEz ICszMTMsMTIgQEAgcHJvZ3JhbSBtYWluCiAgIGludGVnZXIsIGRpbWVuc2lvbig0KSA6OiBw CiAgIGludGVnZXIsIGRpbWVuc2lvbig0KSA6OiBxCiAKLSAgdHlwZSAob21wX2FsbG9jdHJh aXQpIDo6IHRyYWl0cygzKQorICB0eXBlIChvbXBfYWxsb2N0cmFpdCkgOjogdHJhaXRzKDIp CiAgIGludGVnZXIgKG9tcF9hbGxvY2F0b3JfaGFuZGxlX2tpbmQpIDo6IGEKIAogICB0cmFp dHMgPSBbb21wX2FsbG9jdHJhaXQgKG9tcF9hdGtfYWxpZ25tZW50LCA2NCksICYKLSAgICAg ICAgICAgIG9tcF9hbGxvY3RyYWl0IChvbXBfYXRrX2ZhbGxiYWNrLCBvbXBfYXR2X251bGxf ZmIpLCAmCi0gICAgICAgICAgICBvbXBfYWxsb2N0cmFpdCAob21wX2F0a19wb29sX3NpemUs IDgxOTIpXQotICBhID0gb21wX2luaXRfYWxsb2NhdG9yIChvbXBfZGVmYXVsdF9tZW1fc3Bh Y2UsIDMsIHRyYWl0cykKKyAgICAgICAgICAgIG9tcF9hbGxvY3RyYWl0IChvbXBfYXRrX2Zh bGxiYWNrLCBvbXBfYXR2X251bGxfZmIpXQorICBhID0gb21wX2luaXRfYWxsb2NhdG9yIChv bXBfZGVmYXVsdF9tZW1fc3BhY2UsIDIsIHRyYWl0cykKICAgaWYgKGEgPT0gb21wX251bGxf YWxsb2NhdG9yKSBzdG9wIDEKIAogICBjYWxsIG9tcF9zZXRfZGVmYXVsdF9hbGxvY2F0b3Ig KG9tcF9kZWZhdWx0X21lbV9hbGxvYyk7Ci0tIAoyLjI1LjEKCg== --------------flj7rL4DJHgnaLFZ6ocoowbz--