From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from esa1.mentor.iphmx.com (esa1.mentor.iphmx.com [68.232.129.153]) by sourceware.org (Postfix) with ESMTPS id 215D23858C83; Tue, 18 Oct 2022 14:46:17 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 215D23858C83 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=codesourcery.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=mentor.com X-IronPort-AV: E=Sophos;i="5.95,193,1661846400"; d="scan'208";a="87777679" Received: from orw-gwy-02-in.mentorg.com ([192.94.38.167]) by esa1.mentor.iphmx.com with ESMTP; 18 Oct 2022 06:46:15 -0800 IronPort-SDR: 0zAukiuwJno7iEVVbtlvrCeW9o9f6/s7yyDRBnsIV7v4DGjuXNZvnlJPS/EjMtzyawdPSDO8Bu dr4Uhq5X3Yb/scWIgASxm9y22Dc3eTwlQfqB7dxLu6/lvW66E6zKWWfazDY8YgAsR/ieDFMHVt tYbDvUe9sAh2E+zqL2wTcy1EZ10M+XOx1cNtEql0ZcKAomNPIT0+Cv58u5NjFWfL2JXFTkJ2QB J6eSCrbW/PXItFRk2yicNUoxdklBTxebNxvh0Dz68oUVMJSmOFiipnBf8jpf8W07LhDAYbUZWA new= From: Thomas Schwinge To: Julian Brown CC: , Andrew Stubbs , Subject: Re: [PATCH] [og12] OpenACC: Don't gang-privatize artificial variables In-Reply-To: <20221014133856.3388109-2-julian@codesourcery.com> References: <20221014133856.3388109-1-julian@codesourcery.com> <20221014133856.3388109-2-julian@codesourcery.com> User-Agent: Notmuch/0.29.3+94~g74c3f1b (https://notmuchmail.org) Emacs/27.1 (x86_64-pc-linux-gnu) Date: Tue, 18 Oct 2022 16:46:07 +0200 Message-ID: <878rldur4g.fsf@euler.schwinge.homeip.net> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable X-Originating-IP: [137.202.0.90] X-ClientProxiedBy: svr-ies-mbx-14.mgc.mentorg.com (139.181.222.14) To svr-ies-mbx-10.mgc.mentorg.com (139.181.222.10) X-Spam-Status: No, score=-5.9 required=5.0 tests=BAYES_00,HEADER_FROM_DIFFERENT_DOMAINS,KAM_DMARC_STATUS,SPF_HELO_PASS,SPF_PASS,TXREP autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Hi Julian! On 2022-10-14T13:38:56+0000, Julian Brown wrote: > This patch prevents compiler-generated artificial variables from being > treated as privatization candidates for OpenACC. > > The rationale is that e.g. "gang-private" variables actually must be > shared by each worker and vector spawned within a particular gang, but > that sharing is not necessary for any compiler-generated variable (at > least at present, but no such need is anticipated either). Variables on > the stack (and machine registers) are already private per-"thread" > (gang, worker and/or vector), and that's fine for artificial variables. OK, that seems fine rationale for this change in behavior. No contradicting test case jumped onto me, either. > Several tests need their scan output patterns adjusted to compensate. ACK -- surprisingly few. (Some minor fine-tuning necessary for GCC master branch, as had to be expected; I'm working on that.) > --- a/gcc/omp-low.cc > +++ b/gcc/omp-low.cc > @@ -11400,6 +11400,28 @@ oacc_privatization_candidate_p (const location_t= loc, const tree c, > } > } > > + /* If an artificial variable has been added to a bind, e.g. > + a compiler-generated temporary structure used by the Fortran front-= end, do > + not consider it as a privatization candidate. Note that variables = on > + the stack are private per-thread by default: making them "gang-priv= ate" > + for OpenACC actually means to share a single instance of a variable > + amongst all workers and threads spawned within each gang. > + At present, no compiler-generated artificial variables require such > + sharing semantics, so this is safe. */ > + > + if (res && DECL_ARTIFICIAL (decl)) > + { > + res =3D false; > + > + if (dump_enabled_p ()) > + { > + oacc_privatization_begin_diagnose_var (l_dump_flags, loc, c, decl= ); > + dump_printf (l_dump_flags, > + "isn%'t candidate for adjusting OpenACC privatizatio= n " > + "level: %s\n", "artificial"); > + } > + } In the source code comment, you say "added to a bind", and that's indeed what I was expecting, too, and thus put in: if (res && DECL_ARTIFICIAL (decl)) { + gcc_checking_assert (block); + res =3D false; ..., but to my surprised, that did fire in one occasion: > --- a/libgomp/testsuite/libgomp.oacc-fortran/privatized-ref-2.f90 > +++ b/libgomp/testsuite/libgomp.oacc-fortran/privatized-ref-2.f90 > @@ -94,9 +94,7 @@ contains > !$acc parallel copy(array) > !$acc loop gang private(array) ! { dg-line l_loop[incr c_loop] } > ! { dg-note {variable 'i' in 'private' clause isn't candidate for ad= justing OpenACC privatization level: not addressable} "" { target *-*-* } l= _loop$c_loop } > - ! { dg-note {variable 'array\.[0-9]+' in 'private' clause is candida= te for adjusting OpenACC privatization level} "" { target *-*-* } l_loop$c_= loop } > - ! { dg-note {variable 'array\.[0-9]+' ought to be adjusted for OpenA= CC privatization level: 'gang'} "" { target *-*-* } l_loop$c_loop } > - ! { dg-note {variable 'array\.[0-9]+' adjusted for OpenACC privatiza= tion level: 'gang'} "" { target { ! { openacc_host_selected || { openacc_nv= idia_accel_selected && __OPTIMIZE__ } } } } l_loop$c_loop } > + ! { dg-note {variable 'array\.[0-9]+' in 'private' clause isn't cand= idate for adjusting OpenACC privatization level: artificial} "" { target *-= *-* } l_loop$c_loop } > ! { dg-message {sorry, unimplemented: target cannot support alloca} = PR65181 { target openacc_nvidia_accel_selected } l_loop$c_loop } > do i =3D 1, 10 > array(i) =3D 9*i ... here. Note "variable 'array\.[0-9]+' in 'private' clause"; everywhere else we have "declared in block". As part of your verification, have you already looked into whether the new behavior is correct here, or does this one need to continue to be "adjusted for OpenACC privatization level: 'gang'"? If the latter, should we check 'if (res && block && DECL_ARTIFICIAL (decl))' instead of 'if (res && DECL_ARTIFICIAL (decl))', or is there some wrong setting of 'DECL_ARTIFICIAL' -- or are we maybe looking at an inappropriate 'decl'? (Thinking of commit r12-7580-g7a5e036b61aa088e6b8564bc9383d37dfbb4801e "[OpenACC privatization] Analyze 'lookup_decl'-translated DECL [PR90115, PR= 102330, PR104774]", for example.) Gr=C3=BC=C3=9Fe Thomas > @@ -122,9 +120,7 @@ contains > ! { dg-note {variable 'str' in 'private' clause is candidate for adj= usting OpenACC privatization level} "" { target *-*-* } l_loop$c_loop } > ! { dg-note {variable 'str' ought to be adjusted for OpenACC privati= zation level: 'gang'} "" { target *-*-* } l_loop$c_loop } > ! { dg-note {variable 'str' adjusted for OpenACC privatization level= : 'gang'} "" { target { ! { openacc_host_selected || { openacc_nvidia_accel= _selected && __OPTIMIZE__ } } } } l_loop$c_loop } > - ! { dg-note {variable 'char\.[0-9]+' declared in block is candidate = for adjusting OpenACC privatization level} "" { target *-*-* } l_loop$c_loo= p } > - ! { dg-note {variable 'char\.[0-9]+' ought to be adjusted for OpenAC= C privatization level: 'gang'} "" { target *-*-* } l_loop$c_loop } > - ! { dg-note {variable 'char\.[0-9]+' adjusted for OpenACC privatizat= ion level: 'gang'} "" { target { ! { openacc_host_selected || { openacc_nvi= dia_accel_selected && __OPTIMIZE__ } } } } l_loop$c_loop } > + ! { dg-note {variable 'char\.[0-9]+' declared in block isn't candida= te for adjusting OpenACC privatization level: artificial} "" { target *-*-*= } l_loop$c_loop } > ! { dg-message {sorry, unimplemented: target cannot support alloca} = PR65181 { target openacc_nvidia_accel_selected } l_loop$c_loop } > do i =3D 1, 10 > str(i:i) =3D achar(ichar('A') + i) > @@ -167,9 +163,7 @@ contains > ! { dg-note {variable 'scalar' in 'private' clause is candidate for = adjusting OpenACC privatization level} "" { target *-*-* } l_loop$c_loop } > ! { dg-note {variable 'scalar' ought to be adjusted for OpenACC priv= atization level: 'gang'} "" { target *-*-* } l_loop$c_loop } > ! { dg-note {variable 'scalar' adjusted for OpenACC privatization le= vel: 'gang'} "" { target { ! { openacc_host_selected || { openacc_nvidia_ac= cel_selected && __OPTIMIZE__ } } } } l_loop$c_loop } > - ! { dg-note {variable 'char\.[0-9]+' declared in block is candidate = for adjusting OpenACC privatization level} "" { target *-*-* } l_loop$c_loo= p } > - ! { dg-note {variable 'char\.[0-9]+' ought to be adjusted for OpenAC= C privatization level: 'gang'} "" { target *-*-* } l_loop$c_loop } > - ! { dg-note {variable 'char\.[0-9]+' adjusted for OpenACC privatizat= ion level: 'gang'} "" { target { ! { openacc_host_selected || { openacc_nvi= dia_accel_selected && __OPTIMIZE__ } } } } l_loop$c_loop } > + ! { dg-note {variable 'char\.[0-9]+' declared in block isn't candida= te for adjusting OpenACC privatization level: artificial} "" { target *-*-*= } l_loop$c_loop } > do i =3D 1, 15 > scalar(i:i) =3D achar(ichar('A') + i) > end do ----------------- Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstra=C3=9Fe 201= , 80634 M=C3=BCnchen; Gesellschaft mit beschr=C3=A4nkter Haftung; Gesch=C3= =A4ftsf=C3=BChrer: Thomas Heurung, Frank Th=C3=BCrauf; Sitz der Gesellschaf= t: M=C3=BCnchen; Registergericht M=C3=BCnchen, HRB 106955