From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id 7080438133C0 for ; Fri, 27 May 2022 13:42:18 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 7080438133C0 Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-76-UdNSq5-fN2qyLL8WcGvvYg-1; Fri, 27 May 2022 09:42:14 -0400 X-MC-Unique: UdNSq5-fN2qyLL8WcGvvYg-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 3695C2949BBA; Fri, 27 May 2022 13:42:14 +0000 (UTC) Received: from tucnak.zalov.cz (unknown [10.33.36.77]) by smtp.corp.redhat.com (Postfix) with ESMTPS id EF0F72026D64; Fri, 27 May 2022 13:42:13 +0000 (UTC) Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.17.1/8.17.1) with ESMTPS id 24RDgBol1984221 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NOT); Fri, 27 May 2022 15:42:11 +0200 Received: (from jakub@localhost) by tucnak.zalov.cz (8.17.1/8.17.1/Submit) id 24RDgAlM1984220; Fri, 27 May 2022 15:42:10 +0200 Date: Fri, 27 May 2022 15:42:09 +0200 From: Jakub Jelinek To: Kwok Cheung Yeung Cc: gcc-patches Subject: Re: [PATCH 7/7] openmp: Add testcases for metadirectives Message-ID: Reply-To: Jakub Jelinek References: MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.78 on 10.11.54.4 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=us-ascii Content-Disposition: inline X-Spam-Status: No, score=-4.4 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_NONE, 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: , X-List-Received-Date: Fri, 27 May 2022 13:42:22 -0000 On Fri, Dec 10, 2021 at 05:40:36PM +0000, Kwok Cheung Yeung wrote: > This adds testcases for metadirectives. Let me start with the last patch. > +++ b/gcc/testsuite/c-c++-common/gomp/metadirective-1.c > @@ -0,0 +1,29 @@ > +/* { dg-do compile } */ > + > +#define N 100 > + > +void f (int a[], int b[], int c[]) > +{ > + #pragma omp metadirective \ > + default (teams loop) \ > + default (parallel loop) /* { dg-error "there can only be one default clause in a metadirective before '\\(' token" } */ I'd prefer consistency, check_no_duplicate_clause prints for similar bugs too many %qs clauses so it would be nice if this emitted the same (and the before '\\(' token part would be nice to avoid as well (i.e. use error rather than parse error). > --- /dev/null > +++ b/gcc/testsuite/c-c++-common/gomp/metadirective-2.c > @@ -0,0 +1,74 @@ > +/* { dg-do compile } */ > + > +#define N 100 > + > +int main (void) > +{ > + int x = 0; > + int y = 0; > + > + /* Test implicit default (nothing). */ > + #pragma omp metadirective \ > + when (device={arch("nvptx")}: barrier) > + x = 1; I'm not really sure if device={arch("nvptx")} in main is the best idea for most of such tests. Because we should be able to decide that right away, main isn't declare target to (and better shouldn't be) and so when we know host isn't nvptx and that it won't be offloaded, it is clear it can't be that arch. Of course, we need to test such a case too in a few spots, but it would be nice to have more diversity in the tests. One possibility is non-main function with declare target to after the function definition (but one can't then use teams in the metadirectives). But would be nice to use more variety in selectors, user, implementation, device with isa or kind, etc. instead of using always the same thing in most of the tests. Also it would be nice to cover say a directive which needs loop, a directive which needs a normal body and say 2 directives which are standalone. > +++ b/gcc/testsuite/c-c++-common/gomp/metadirective-3.c > @@ -0,0 +1,31 @@ > +/* { dg-do compile } */ > +/* { dg-additional-options "-fdump-tree-original" } */ > +/* { dg-additional-options "-fdump-tree-gimple" } */ > +/* { dg-additional-options "-fdump-tree-optimized" } */ > + > +#define N 100 > + > +void f (int x[], int y[], int z[]) > +{ > + int i; > + > + #pragma omp target map(to: x, y) map(from: z) > + #pragma omp metadirective \ > + when (device={arch("nvptx")}: teams loop) \ > + default (parallel loop) It would be nice to have many of the tests where all the metadirective variants are actually possible. Here the nvptx variant is quite unlikely, nvptx is rarely tested as host arch, f is not declare target to and even if it was, teams is not allowed inside of target regions like that. > --- /dev/null > +++ b/gcc/testsuite/c-c++-common/gomp/metadirective-4.c > + > + /* TODO: This does not execute a version of f with the default clause > + active as might be expected. */ Might be nice to mention that it is correct 5.0 behavior, 5.1 is just broken in this regard and 5.2 changed the behavior so that parallel loop is actually invoked. > + f (a, 2.71828); > +++ b/gcc/testsuite/c-c++-common/gomp/metadirective-5.c > @@ -0,0 +1,24 @@ > +/* { dg-do compile } */ > +/* { dg-additional-options "-fdump-tree-original" } */ > + > +#define N 100 > + > +void f (int a[], int flag) > +{ > + int i; > + #pragma omp metadirective \ > + when (user={condition(flag)}: \ > + target teams distribute parallel for map(from: a[0:N])) \ > + default (parallel for) > + for (i = 0; i < N; i++) > + a[i] = i; > +} > + > +/* The metadirective should be resolved at parse time. */ ??? How can it? The above is invalid in OpenMP 5.0 (condition should be constant expression), it is valid in OpenMP 5.1, but is then resolved at runtime, certainly not at parse time. Would be nice to also test user={condition(1)} etc. where it would be resolved at parse time. And, please add some tests even with user scores. > +++ b/gcc/testsuite/c-c++-common/gomp/metadirective-6.c > @@ -0,0 +1,31 @@ > +/* { dg-do compile } */ > +/* { dg-additional-options "-fdump-tree-original" } */ > +/* { dg-additional-options "-fdump-tree-gimple" } */ > + > +#define N 100 > + > +void bar (int a[], int run_parallel, int run_guided) > +{ > + #pragma omp metadirective \ > + when (user={condition(run_parallel)}: parallel) > + { > + int i; > + #pragma omp metadirective \ > + when (construct={parallel}, user={condition(run_guided)}: \ > + for schedule(guided)) \ > + when (construct={parallel}: for schedule(static)) > + for (i = 0; i < N; i++) > + a[i] = i; > + } > + } > + > +/* The outer metadirective should be resolved at parse time. */ > +/* The inner metadirective should be resolved during Gimplificiation. */ Again, dynamic condition, so I don't see how this holds, both should be resolved at runtime. > +++ b/libgomp/testsuite/libgomp.c-c++-common/metadirective-1.c > @@ -0,0 +1,35 @@ > +/* { dg-do run } */ > + > +#define N 100 > + > +void f (int x[], int y[], int z[]) > +{ > + int i; > + > + #pragma omp target map(to: x[0:N], y[0:N]) map(from: z[0:N]) > + #pragma omp metadirective \ > + when (device={arch("nvptx")}: teams loop) \ > + default (parallel loop) > + for (i = 0; i < N; i++) This doesn't really test which of them was selected and one of them is extremely unlikely. Doesn't hurt to have some such tests, but would be nice if there were tests that it could vary (e.g. same test triggering both or all variants in different code paths, with selector chosen such that it is possible) where it would return different results and at runtime it would be possible to decide which one is which. That can be done either through declare variant in the body, or say nested metadirective which will say through data sharing or mapping result in different values (say task shared(var) in one case and task firstprivate(var) in another etc.). Jakub