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 F3BEE3844051; Mon, 26 Apr 2021 19:54:48 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org F3BEE3844051 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=codesourcery.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=Thomas_Schwinge@mentor.com IronPort-SDR: igIKg0Wp0BT5L6XtSTitombq5pKn24mK4faW4he9u+HcOmRIBogwSGM0BjHJxw3VzJfIWUv6pJ 0SF3zRx/TPPl3L+ixomzX/m6G4XIHxn0jpVleCtv4KyDOhZXHf/vVnDW6Io84prE5ns29bxpJO mx6yhqmhbnqwk9Ns7YDKR9EUTvPbeFmun+J8f1DBJflb4Y37GUKKsSPRklDKKZx3FB5OKTt7DQ Q5AQm7tswt5XwXOheQGsl8cRL/KVW4WvnnNcVIOAWRW7S8pk+W0hKCKn7LxWv7lvp4Ps5qxKJH U3Q= X-IronPort-AV: E=Sophos;i="5.82,252,1613462400"; d="scan'208";a="60539003" Received: from orw-gwy-02-in.mentorg.com ([192.94.38.167]) by esa3.mentor.iphmx.com with ESMTP; 26 Apr 2021 11:54:48 -0800 IronPort-SDR: FgNe8M8NbJbR8/2QO7kUFxWIh5XpBIVfq1Cei1jBkYPv2E1ceqKmzscTqb+zFDGGKaILvFJ1k3 tXJi0linNp5OlBkzikY6Ms3QsRotJBQeEniObSD4Ks7hrJSNxVIPGaoIzeEta9pTiYp5b/E4V7 50YtWTeC1I/H9UWMPmBtsyaFIWeA5ZBPZvC0WLsv+E0J9s+CcqfRBUzNBLCKRM11IeclqRwy/N mVPuNb7YZqKt7MzwAFI3Bv49TtZb9AwKIVf35tjEvCUAlgwsCyYZpfbuGKDQsaYat7sh5K5SDd Gq0= From: Thomas Schwinge To: Tobias Burnus CC: Jakub Jelinek , Julian Brown , , Subject: Re: [PATCH] openacc: Warnings for strange partitioning choices for parallel regions In-Reply-To: References: <20210226122154.5209-1-julian@codesourcery.com> <871raxuy7g.fsf@euler.schwinge.homeip.net> User-Agent: Notmuch/0.29.3+94~g74c3f1b (https://notmuchmail.org) Emacs/27.1 (x86_64-pc-linux-gnu) Date: Mon, 26 Apr 2021 21:54:39 +0200 Message-ID: <87pmyg4y9s.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-04.mgc.mentorg.com (139.181.222.4) To svr-ies-mbx-01.mgc.mentorg.com (139.181.222.1) X-Spam-Status: No, score=-6.5 required=5.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS, KAM_DMARC_STATUS, KAM_SHORT, SPF_HELO_PASS, SPF_PASS, TXREP autolearn=no autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) 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, 26 Apr 2021 19:54:52 -0000 Hi! On 2021-04-26T14:32:10+0200, Tobias Burnus wrote: > first, can you add an entry regarding this flag > to https://gcc.gnu.org/gcc-12/changes.html ? Is that a standard thing that all new command-line flags do get highlighted there? (I wouldn't have considered that one important enough?) > Secondly, I now see FAILs like: > > FAIL: gfortran.dg/goacc/classify-serial.f95 -O (test for excess errors= ) > Excess errors: > gfortran.dg/goacc/classify-serial.f95:20:132: Warning: region contains ga= ng partitioned code but is not gang partitioned [-Wopenacc-parallelism] > gfortran.dg/goacc/classify-serial.f95:20:132: Warning: region contains ve= ctor partitioned code but is not vector partitioned [-Wopenacc-parallelism] Eek! I do reproduce that -- but only in a GCC build configuration without offloading enabled! For example, for 'gfortran.dg/goacc/classify-serial.f95', we've got the following diff in diagnostics in a GCC build configuration without vs. with offloading enabled: {+[...]/gcc/testsuite/gfortran.dg/goacc/classify-serial.f95: In functio= n 'MAIN__._omp_fn.0':+} [...]/gcc/testsuite/gfortran.dg/goacc/classify-serial.f95:20:132: [-War= ning:-]{+warning:+} region contains gang partitioned code but is not gang p= artitioned [-Wopenacc-parallelism] [...]/gcc/testsuite/gfortran.dg/goacc/classify-serial.f95:20:132: [-War= ning:-]{+warning:+} region contains vector partitioned code but is not vect= or partitioned [-Wopenacc-parallelism] [...]/gcc/testsuite/gfortran.dg/goacc/classify-serial.f95:20:132: optim= ized: assigned OpenACC gang vector loop parallelism PASS: gfortran.dg/goacc/classify-serial.f95 -O (test for warnings, = line 20) [-XPASS:-]{+XFAIL:+} gfortran.dg/goacc/classify-serial.f95 -O TODO '= serial' (test for bogus messages, line 20) PASS: gfortran.dg/goacc/classify-serial.f95 -O (test for bogus mess= ages, line 20) [-XPASS:-]{+XFAIL:+} gfortran.dg/goacc/classify-serial.f95 -O TODO '= serial' (test for bogus messages, line 20) [-FAIL:-]{+PASS:+} gfortran.dg/goacc/classify-serial.f95 -O (test fo= r excess errors)[-Excess errors:-] [-[...]/gcc/testsuite/gfortran.dg/goacc/classify-serial.f95:20:132: War= ning: region contains gang partitioned code but is not gang partitioned [-W= openacc-parallelism]-] [-[...]/gcc/testsuite/gfortran.dg/goacc/classify-serial.f95:20:132: War= ning: region contains vector partitioned code but is not vector partitioned= [-Wopenacc-parallelism]-] PASS: gfortran.dg/goacc/classify-serial.f95 -O scan-tree-dump-times= ompexp "(?n)__attribute__\\(\\(oacc serial, omp target entrypoint\\)\\)" 1 PASS: gfortran.dg/goacc/classify-serial.f95 -O scan-tree-dump-times= oaccdevlow "(?n)Function is OpenACC serial offload" 1 PASS: gfortran.dg/goacc/classify-serial.f95 -O scan-tree-dump-times= oaccdevlow "(?n)Compute dimensions \\[1, 1, 1\\]" 1 Notice upper-case "Warning" vs. lower-case "warning". That's for Fortran only; for C, C++, it's lower-case "warning" for both build variants. It's of course easy to fix up the regexp, but should we maybe rather figure out how to unify the reporting? I do understand that Fortran has some upper-case diagnostics, but I don't understand the difference/relevance of GCC build configuration without vs. with offloading enabled, how is that coming becoming relevant? Also notice that the preamble "In function 'MAIN__._omp_fn.0':" only appears in the GCC build configuration with offloading. Looking at 'c-c++-common/goacc/classify-serial.c', for C, we've got "In function 'SERIAL._omp_fn.0':" for both GCC build configurations, and for C++, we've got "In function '_Z6SERIALv._omp_fn.0':" without offloading enabled vs. "In function 'SERIAL() [clone ._omp_fn.0]':" with offloading enabled. So there generally seems to be some difference in function outlining setup without vs. with offloading enabled? ..., and for Fortran, with offloading enabled, that causes the outlined function to be "de-Fortranified", thus the lower-case "warning" diagnostic? > FAIL: gfortran.dg/goacc/kernels-decompose-2.f95 -O (test for excess er= rors) > Excess errors: > gfortran.dg/goacc/kernels-decompose-2.f95:124:15: Warning: region contain= s gang partitioned code but is not gang partitioned [-Wopenacc-parallelism] > > FAIL: gfortran.dg/goacc/routine-module-mod-1.f90 (test for excess errors) > Excess errors: > gfortran.dg/goacc/routine-module-mod-1.f90:56:16: Warning: region is work= er partitioned but does not contain worker partitioned code [-Wopenacc-para= llelism] ..., and similarly, several more in 'libgomp.oacc-fortran'. Gr=C3=BC=C3=9Fe Thomas > On 26.04.21 12:39, Thomas Schwinge wrote: > >> Hi! >> >> On 2021-02-26T04:21:54-0800, Julian Brown wrot= e: >>> This patch adds warnings for strange partitioning choices -- specifying >>> either more or fewer partitioning levels on a parallel directive than t= he >>> enclosed offload region actually uses. >> Thanks! >> >>> We've used a version of this patch on the og8/og9/og10 branches for >>> quite a while. Versions have been posted for mainline submission as >>> part of a larger patch several times, e.g. here: >>> >>> https://gcc.gnu.org/pipermail/gcc-patches/2018-October/507938.html >>> >>> One motivation for committing this patch is it removes an ongoing >>> divergence in a large number of test cases between the og* branches and >>> mainline, namely whether the added warnings are expected as a result of >>> compiling those test cases, or not. >>> >>> The warnings themselves are perhaps slightly aggressive, but are intend= ed >>> to help the user write more efficient code. >> As I'd said in >> : >> >> | The general intention is good, but I've seen cases where I considered >> | these diagnostics to be too noisy. See also the several 'dg-bogus' >> | with XFAIL >> >> ... as well as 'dg-additional-options "-w"' being added. >> >> I've spent some more time on this; in particular, 'dg-bogus' with XFAIL >> setup, to indicate individual items that I think need to be addressed. >> (Please let me know if anybody makes a different categorization regardin= g >> these.) Of course, not all of our testcases are representative of >> real-world code (but I've anyway removed '-w' in favor of 'dg-warning' o= r >> 'dg-bogus' with XFAIL, for coverage), but I still came to the conclusion >> that this is too noisy to enable by default (as had been proposed), and >> thus for the time being have hidden it behind '-Wopenacc-parallelism'. >> (Adding an option is good anyway, so that users are able to disable >> unwanted diagnostics; we need to do that for other OpenACC diagnostics, >> too.) The goal is to eventually enable '-Wopenacc-parallelism', with >> '-Wall' and some "special cases" with '-Wextra', I suppose. >> >> I've pushed "Add '-Wopenacc-parallelism'" to master branch in commit >> 22cff118f7526bec195ed6e41452980820fdf3a8, see attached. >> >> >> Gr=C3=BC=C3=9Fe >> Thomas >> >> > ----------------- > Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 M=C3=BCnchen= Registergericht M=C3=BCnchen HRB 106955, Gesch=C3=A4ftsf=C3=BChrer: Thomas= Heurung, Frank Th=C3=BCrauf ----------------- Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 M=C3=BCnchen R= egistergericht M=C3=BCnchen HRB 106955, Gesch=C3=A4ftsf=C3=BChrer: Thomas H= eurung, Frank Th=C3=BCrauf