From patchwork Mon Jan 11 11:55:11 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Thomas Schwinge X-Patchwork-Id: 565772 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id A8F7B1402ED for ; Mon, 11 Jan 2016 22:55:56 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=FpJzFGiI; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:cc:subject:in-reply-to:references:date:message-id :mime-version:content-type; q=dns; s=default; b=pOBL5Wx9itWPMiA3 UVhmDcQ7MhJkxWC1p8CGo/SfK3TOUvpOhaI7yqLnv3nptzBe5t2yJPgg1x0byDJQ gZcrxQiWXYgSAU4Bzoy0kksK/U64d/aq2AEWn3HTA2NX1+llwvk4EUWMlxfjBSOo qAG4YlsKrIVkmgkQAW3BzIURBYc= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:cc:subject:in-reply-to:references:date:message-id :mime-version:content-type; s=default; bh=BXbucXm6yZ8RhWpXkjOH7m hkwDE=; b=FpJzFGiIL4TlxIIoviapl8TleQq1TX80l7VYJJl8mqOcIiBCeqnueA xIXok2YcMllGqC2FW0p3VhYIb8ihqAO3KPHgVUrgOm8vF+e/LUEvyLH21xmrZcwJ Ujh1Cthfuhdh1rOeRk+XBuB429AVRgE/qDvYbyZcb2WBUBQlmmynE= Received: (qmail 26399 invoked by alias); 11 Jan 2016 11:55:33 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 26084 invoked by uid 89); 11 Jan 2016 11:55:31 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.4 required=5.0 tests=AWL, BAYES_50, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.2 spammy=356, Additional, 47, UD:c-typeck.c X-HELO: relay1.mentorg.com Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 11 Jan 2016 11:55:25 +0000 Received: from svr-orw-fem-02x.mgc.mentorg.com ([147.34.96.206] helo=SVR-ORW-FEM-02.mgc.mentorg.com) by relay1.mentorg.com with esmtp id 1aIb4D-0002KC-7s from Thomas_Schwinge@mentor.com ; Mon, 11 Jan 2016 03:55:21 -0800 Received: from tftp-cs (147.34.91.1) by svr-orw-fem-02.mgc.mentorg.com (147.34.96.168) with Microsoft SMTP Server id 14.3.224.2; Mon, 11 Jan 2016 03:55:20 -0800 Received: by tftp-cs (Postfix, from userid 49978) id 47D2AC2328; Mon, 11 Jan 2016 03:55:20 -0800 (PST) From: Thomas Schwinge To: James Norris , GCC Patches CC: Jakub Jelinek Subject: Re: [gomp4] Fix use of declare'd vars by routine procedures. In-Reply-To: <568EB51C.9010405@codesourcery.com> References: <87mvtapdp0.fsf@kepler.schwinge.homeip.net> <568EB51C.9010405@codesourcery.com> User-Agent: Notmuch/0.9-125-g4686d11 (http://notmuchmail.org) Emacs/24.5.1 (i586-pc-linux-gnu) Date: Mon, 11 Jan 2016 12:55:11 +0100 Message-ID: <874mek72lc.fsf@kepler.schwinge.homeip.net> MIME-Version: 1.0 Hi! On Thu, 7 Jan 2016 12:57:32 -0600, James Norris wrote: > The checking of variables declared by OpenACC declare directives > used within an OpenACC routine procedure was not being done correctly. > This fix adds the checking required and also adds to the testing. > > This fix resolves the issue pointed out by Cesar with declare-4.c > (https://gcc.gnu.org/ml/gcc-patches/2016-01/msg00339.html). > > This patch has been applied to gomp-4_0-branch. Such a patch needs to go into trunk; see my report in . > --- gcc/c/c-parser.c (revision 232138) > +++ gcc/c/c-parser.c (working copy) > @@ -14115,6 +14115,10 @@ > /* Also add an "omp declare target" attribute, with clauses. */ > DECL_ATTRIBUTES (fndecl) = tree_cons (get_identifier ("omp declare target"), > clauses, DECL_ATTRIBUTES (fndecl)); > + > + DECL_ATTRIBUTES (fndecl) > + = tree_cons (get_identifier ("oacc routine"), > + clauses, DECL_ATTRIBUTES (fndecl)); > } Yuck, another attribute... (..., and it's not listed/documented in gcc/c-family/c-common.c:c_common_attribute_table.) You store clauses in the "oacc routine" here, but it's unused as far as I can tell. Given that we have the clauses available from the "omp declare target" attribute (subject to change to switching to a generic "omp clauses" attribute as suggested in ), could we maybe just look up some specific clause instead of detecting the presence of this extra attribute? (Jakub, any preference?) Anyway, have we verified that the desired behavior: > --- gcc/c/c-typeck.c (revision 232138) > +++ gcc/c/c-typeck.c (working copy) > @@ -2664,6 +2664,26 @@ > tree ref; > tree decl = lookup_name (id); > > + if (decl > + && decl != error_mark_node > + && current_function_decl > + && TREE_CODE (decl) == VAR_DECL > + && is_global_var (decl) > + && lookup_attribute ("oacc routine", > + DECL_ATTRIBUTES (current_function_decl))) > + { > + if (lookup_attribute ("omp declare target link", > + DECL_ATTRIBUTES (decl)) > + || ((!lookup_attribute ("omp declare target", > + DECL_ATTRIBUTES (decl)) > + && ((TREE_STATIC (decl) && !DECL_EXTERNAL (decl)) > + || (!TREE_STATIC (decl) && DECL_EXTERNAL (decl)))))) > + { > + error_at (loc, "invalid use in % function"); > + return error_mark_node; > + } > + } ..., isn't applicable to OpenMP as well (thus, no "oacc routine" attribute conditional is needed here)? > --- gcc/cp/parser.c (revision 232138) > +++ gcc/cp/parser.c (working copy) > @@ -36732,6 +36732,10 @@ > DECL_ATTRIBUTES (fndecl) > = tree_cons (get_identifier ("omp declare target"), > clauses, DECL_ATTRIBUTES (fndecl)); > + > + DECL_ATTRIBUTES (fndecl) > + = tree_cons (get_identifier ("oacc routine"), > + NULL_TREE, DECL_ATTRIBUTES (fndecl)); > } You don't store clauses in the "oacc routine" here. > --- gcc/cp/semantics.c (revision 232138) > +++ gcc/cp/semantics.c (working copy) > @@ -3700,6 +3700,25 @@ > > decl = convert_from_reference (decl); > } > + > + if (decl != error_mark_node > + && current_function_decl > + && TREE_CODE (decl) == VAR_DECL > + && is_global_var (decl) > + && lookup_attribute ("oacc routine", > + DECL_ATTRIBUTES (current_function_decl))) > + { > + if (lookup_attribute ("omp declare target link", > + DECL_ATTRIBUTES (decl)) > + || ((!lookup_attribute ("omp declare target", > + DECL_ATTRIBUTES (decl)) > + && ((TREE_STATIC (decl) && !DECL_EXTERNAL (decl)) > + || (!TREE_STATIC (decl) && DECL_EXTERNAL (decl)))))) > + { > + *error_msg = "invalid use in % function"; > + return error_mark_node; > + } > + } Likewise. No equivalent change is needed for Fortran? > --- libgomp/testsuite/libgomp.oacc-c-c++-common/declare-4.c (revision 232138) > +++ libgomp/testsuite/libgomp.oacc-c-c++-common/declare-4.c (working copy) > @@ -4,7 +4,7 @@ > #include > > float b; > -#pragma acc declare link (b) > +#pragma acc declare create (b) > > #pragma acc routine > int I have not verified the details, but a very similar fix was required to get rid of a number of regressions: @@ -2637,18 +2637,18 @@ PASS: c-c++-common/goacc-gomp/nesting-fail-1.c (test for errors, line 350) PASS: c-c++-common/goacc-gomp/nesting-fail-1.c (test for errors, line 356) PASS: c-c++-common/goacc-gomp/nesting-fail-1.c (test for errors, line 358) PASS: c-c++-common/goacc-gomp/nesting-fail-1.c (test for errors, line 360) [-PASS:-]{+FAIL:+} c-c++-common/goacc-gomp/nesting-fail-1.c (test for errors, line 371) [-PASS:-]{+FAIL:+} c-c++-common/goacc-gomp/nesting-fail-1.c (test for errors, line 378) [-PASS:-]{+FAIL:+} c-c++-common/goacc-gomp/nesting-fail-1.c (test for errors, line 386) [-PASS:-]{+FAIL:+} c-c++-common/goacc-gomp/nesting-fail-1.c (test for errors, line 395) [-PASS:-]{+FAIL:+} c-c++-common/goacc-gomp/nesting-fail-1.c (test for errors, line 402) [-PASS:-]{+FAIL:+} c-c++-common/goacc-gomp/nesting-fail-1.c (test for errors, line 409) [-PASS:-]{+FAIL:+} c-c++-common/goacc-gomp/nesting-fail-1.c (test for errors, line 416) PASS: c-c++-common/goacc-gomp/nesting-fail-1.c (test for errors, line 42) [-PASS:-]{+FAIL:+} c-c++-common/goacc-gomp/nesting-fail-1.c (test for errors, line 423) [-PASS:-]{+FAIL:+} c-c++-common/goacc-gomp/nesting-fail-1.c (test for errors, line 430) [-PASS:-]{+FAIL:+} c-c++-common/goacc-gomp/nesting-fail-1.c (test for errors, line 432) [-PASS:-]{+FAIL:+} c-c++-common/goacc-gomp/nesting-fail-1.c (test for errors, line 434) PASS: c-c++-common/goacc-gomp/nesting-fail-1.c (test for errors, line 47) PASS: c-c++-common/goacc-gomp/nesting-fail-1.c (test for errors, line 52) PASS: c-c++-common/goacc-gomp/nesting-fail-1.c (test for errors, line 57) @@ -2667,7 +2667,7 @@ PASS: c-c++-common/goacc-gomp/nesting-fail-1.c (test for errors, line 93) PASS: c-c++-common/goacc-gomp/nesting-fail-1.c (test for errors, line 95) PASS: c-c++-common/goacc-gomp/nesting-fail-1.c (test for errors, line 97) PASS: c-c++-common/goacc-gomp/nesting-fail-1.c (test for errors, line 99) [-PASS:-]{+FAIL:+} c-c++-common/goacc-gomp/nesting-fail-1.c (test for excess errors) Same for C++. Committed to gomp-4_0-branch in r232219: commit 1ac87f2b59dd03cb305ec94a7c6b5657dbb54e66 Author: tschwinge Date: Mon Jan 11 11:51:57 2016 +0000 Resolve c-c++-common/goacc-gomp/nesting-fail-1.c regressions gcc/testsuite/ * c-c++-common/goacc-gomp/nesting-fail-1.c: Add OpenACC declare directive for "i". git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/branches/gomp-4_0-branch@232219 138bc75d-0d04-0410-961f-82ee72b054a4 --- gcc/testsuite/ChangeLog.gomp | 5 +++++ gcc/testsuite/c-c++-common/goacc-gomp/nesting-fail-1.c | 1 + 2 files changed, 6 insertions(+) Grüße Thomas diff --git gcc/testsuite/ChangeLog.gomp gcc/testsuite/ChangeLog.gomp index 607ca8e..2db11df 100644 --- gcc/testsuite/ChangeLog.gomp +++ gcc/testsuite/ChangeLog.gomp @@ -1,3 +1,8 @@ +2016-01-11 Thomas Schwinge + + * c-c++-common/goacc-gomp/nesting-fail-1.c: Add OpenACC declare + directive for "i". + 2016-01-07 James Norris * c-c++-common/goacc/routine-5.c: Additional tests. diff --git gcc/testsuite/c-c++-common/goacc-gomp/nesting-fail-1.c gcc/testsuite/c-c++-common/goacc-gomp/nesting-fail-1.c index 8e0f217..9011fcf 100644 --- gcc/testsuite/c-c++-common/goacc-gomp/nesting-fail-1.c +++ gcc/testsuite/c-c++-common/goacc-gomp/nesting-fail-1.c @@ -1,4 +1,5 @@ extern int i; +#pragma acc declare create(i) void f_omp (void)