From patchwork Fri May 13 17:21:02 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Tobias Burnus X-Patchwork-Id: 53969 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id B4D57395BC2B for ; Fri, 13 May 2022 17:21:43 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from esa1.mentor.iphmx.com (esa1.mentor.iphmx.com [68.232.129.153]) by sourceware.org (Postfix) with ESMTPS id A6B7F395B406; Fri, 13 May 2022 17:21:09 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org A6B7F395B406 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=mentor.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=mentor.com X-IronPort-AV: E=Sophos;i="5.91,223,1647331200"; d="diff'?scan'208";a="78511432" Received: from orw-gwy-01-in.mentorg.com ([192.94.38.165]) by esa1.mentor.iphmx.com with ESMTP; 13 May 2022 09:21:08 -0800 IronPort-SDR: MYwol/2dJYpHXaxsX0VYnS9PRYZZm93RVGqLaTRyRX5BPFpR90fKb28ys/llp0QrD19CJbwTkb sr7PtLNWZOAYTgES/KlS3yZ5f5t+chi4MgkYLmsgZoIrq3LKXY8LsCINO0AKPNX1oNR87pIXyZ o+LE7hsrSTJoppuOSuemfRyJ5c0fYdrXc9of5tF+Dmc+7AZhTruDzH201knXyCMNPMc3zmEdZ2 BIfGbfrnG0Gswfkpie+fx8fvI1pX4Wi5q/QiGEH6l52LsLwbyZsEI/8CpXy75F2ojx1+OFHETF hC0= Message-ID: Date: Fri, 13 May 2022 19:21:02 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.9.0 Content-Language: en-US To: gcc-patches , fortran , Jakub Jelinek From: Tobias Burnus Subject: [Patch] OpenMP/Fortran: Use firstprivat not alloc for ptr attach for arrays X-Originating-IP: [137.202.0.90] X-ClientProxiedBy: svr-ies-mbx-13.mgc.mentorg.com (139.181.222.13) To svr-ies-mbx-13.mgc.mentorg.com (139.181.222.13) X-Spam-Status: No, score=-11.6 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_STATUS, SPF_HELO_PASS, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" Based on sollve_vv's tests/4.5/target_teams_distribute/test_target_teams_distribute_nowait.F90 As discussed, for simple pointers – like here with nondescriptor array, instead of alloc:a + pointer assign, a firstprivate + pointer assign makes more sense. It also avoids the race exposed by the sollve_vv testcase in some constellations. (The testcase, both as attached and currently in sollve_vv [→ Issue #532], is invalid if run with 'nowait' as there is a race related to the array - as it only (un)mapped as (disjunct) array sections via interleaved, concurrently running target constructs with nowait clause.) There might be more places where this should/could be done – and in principle, firstprivate could also be useful for an array descriptor (but not its data); this could also be explored. (Including whether it should then not be privatized with shared memory.) OK? Tobias PS: OpenACC is excluded as it does its own firstprivate handling. ----------------- 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 OpenMP/Fortran: Use firstprivat not alloc for ptr attach for arrays For a non-descriptor array, map(A(n:m)) was mapped as map(tofrom:A[n-1] [len: ...]) map(alloc:A [pointer assign, bias: ...]) with this patch, it is changed to map(tofrom:A[n-1] [len: ...]) map(firstprivate:A [pointer assign, bias: ...]) The latter avoids an alloc - and also avoids the race condition with nowait in the enclosed testcase. (Note: predantically, the testcase is invalid since OpenMP 5.1, violating the map clause restriction at [354:10-13]. gcc/fortran/ChangeLog: * trans-openmp.cc (gfc_trans_omp_clauses): When mapping nondescriptor array sections, use GOMP_MAP_FIRSTPRIVATE_POINTER instead of GOMP_MAP_POINTER for the pointer attachment. libgomp/ChangeLog: * testsuite/libgomp.fortran/target-nowait-array-section.f90: New test. gcc/fortran/trans-openmp.cc | 12 +++-- .../target-nowait-array-section.f90 | 56 ++++++++++++++++++++++ 2 files changed, 65 insertions(+), 3 deletions(-) diff --git a/gcc/fortran/trans-openmp.cc b/gcc/fortran/trans-openmp.cc index baa45f78a0e..eb5870c3bc5 100644 --- a/gcc/fortran/trans-openmp.cc +++ b/gcc/fortran/trans-openmp.cc @@ -3312,9 +3312,15 @@ gfc_trans_omp_clauses (stmtblock_t *block, gfc_omp_clauses *clauses, /* An array element or array section which is not part of a derived type, etc. */ bool element = n->expr->ref->u.ar.type == AR_ELEMENT; - gfc_trans_omp_array_section (block, n, decl, element, - GOMP_MAP_POINTER, node, node2, - node3, node4); + tree type = TREE_TYPE (decl); + gomp_map_kind k = GOMP_MAP_POINTER; + if (!openacc + && !GFC_DESCRIPTOR_TYPE_P (type) + && !(POINTER_TYPE_P (type) + && GFC_DESCRIPTOR_TYPE_P (TREE_TYPE (type)))) + k = GOMP_MAP_FIRSTPRIVATE_POINTER; + gfc_trans_omp_array_section (block, n, decl, element, k, + node, node2, node3, node4); } else if (n->expr && n->expr->expr_type == EXPR_VARIABLE diff --git a/libgomp/testsuite/libgomp.fortran/target-nowait-array-section.f90 b/libgomp/testsuite/libgomp.fortran/target-nowait-array-section.f90 new file mode 100644 index 00000000000..7560cff746b --- /dev/null +++ b/libgomp/testsuite/libgomp.fortran/target-nowait-array-section.f90 @@ -0,0 +1,56 @@ +! Runs the the target region asynchrolously and checks for it +! +! Note that map(alloc: work(:, i)) + nowait should be save +! given that a nondescriptor array is used. However, it still +! violates a map clause restriction, added in OpenMP 5.1 [354:10-13]. + +PROGRAM test_target_teams_distribute_nowait + USE ISO_Fortran_env, only: INT64 + implicit none + INTEGER, parameter :: N = 1024, N_TASKS = 16 + INTEGER :: i, j, k, my_ticket + INTEGER :: order(n_tasks) + INTEGER(INT64) :: work(n, n_tasks) + INTEGER :: ticket + logical :: async + + ticket = 0 + + !$omp target enter data map(to: ticket, order) + + !$omp parallel do num_threads(n_tasks) + DO i = 1, n_tasks + !$omp target map(alloc: work(:, i), ticket) private(my_ticket) nowait + !!$omp target teams distribute map(alloc: work(:, i), ticket) private(my_ticket) nowait + DO j = 1, n + ! Waste cyles +! work(j, i) = 0 +! DO k = 1, n*(n_tasks - i) +! work(j, i) = work(j, i) + i*j*k +! END DO + my_ticket = 0 + !$omp atomic capture + ticket = ticket + 1 + my_ticket = ticket + !$omp end atomic + !$omp atomic write + order(i) = my_ticket + END DO + !$omp end target !teams distribute + END DO + !$omp end parallel do + + !$omp target exit data map(from:ticket, order) + + IF (ticket .ne. n_tasks*n) stop 1 + if (maxval(order) /= n_tasks*n) stop 2 + ! order(i) == n*i if synchronous and between n and n*n_tasks if run concurrently + do i = 1, n_tasks + if (order(i) < n .or. order(i) > n*n_tasks) stop 3 + end do + async = .false. + do i = 1, n_tasks + if (order(i) /= n*i) async = .true. + end do + if (.not. async) stop 4 ! Did not run asynchronously +end