From patchwork Thu May 7 18:32:26 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Julian Brown X-Patchwork-Id: 469768 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 BA9C3140218 for ; Fri, 8 May 2015 04:32:55 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=GP5VuQPe; 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:date :from:to:cc:subject:message-id:in-reply-to:references :mime-version:content-type; q=dns; s=default; b=jvYHJRvnPC5y0kLW DWwL9882HLpfRHfqqP4j26HZpncabap6xoKIP2WmiCPGWRLHx2JUOj6H98lcnp38 iwdBMNqVHnwCUR0HnKiQNk1zo8RkeIZnVMR3OpYYoUAZCuhqbns+8wj9PZyCGBUW 9HYIp988kkSLpB8N3EvIjZHymyc= 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:date :from:to:cc:subject:message-id:in-reply-to:references :mime-version:content-type; s=default; bh=y4Uf+AXXAlBcyfeZHpymXm 8l+4U=; b=GP5VuQPekVvZi+IDzMBDfO7/KONQCGgeEn9KHC7f7Vor9bVMoHhDu5 8pPN7h2WtQqOSYHNVZPGJMkctxDjuPiQvxxHVWtWJRv8MAik5EvqXGXK5MubddaJ LR2424M+z6etGgyMuSelu0CPzlprt/lARQIJvouoN09ld5MQvEh+Y= Received: (qmail 105491 invoked by alias); 7 May 2015 18:32:47 -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 105482 invoked by uid 89); 7 May 2015 18:32:46 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.8 required=5.0 tests=AWL, BAYES_50, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 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; Thu, 07 May 2015 18:32:44 +0000 Received: from nat-ies.mentorg.com ([192.94.31.2] helo=SVR-IES-FEM-01.mgc.mentorg.com) by relay1.mentorg.com with esmtp id 1YqQb8-0007gQ-E2 from Julian_Brown@mentor.com ; Thu, 07 May 2015 11:32:38 -0700 Received: from octopus (137.202.0.76) by SVR-IES-FEM-01.mgc.mentorg.com (137.202.0.104) with Microsoft SMTP Server id 14.3.224.2; Thu, 7 May 2015 19:32:36 +0100 Date: Thu, 7 May 2015 19:32:26 +0100 From: Julian Brown To: Jakub Jelinek CC: Thomas Schwinge , , Kirill Yukhin , Ilya Verbin Subject: Re: acc_on_device for device_type_host_nonshm (was: libgomp nvptx plugin: rework initialisation and support the proposed load/unload hooks) (PR65742) Message-ID: <20150507193226.6aee40c5@octopus> In-Reply-To: <20150417131619.GP1725@tucnak.redhat.com> References: <20150331235328.GC623@msticlxl57.ims.intel.com> <20150401052147.GG19273@tucnak.redhat.com> <20150401131405.GD623@msticlxl57.ims.intel.com> <20150401132025.GM19273@tucnak.redhat.com> <20150406124557.GA5541@msticlxl57.ims.intel.com> <20150407152645.GJ19273@tucnak.redhat.com> <20150408153142.128b97b9@octopus> <20150408145856.GA19410@msticlxl57.ims.intel.com> <20150414151502.5ca882a4@octopus> <87pp76zpep.fsf@schwinge.name> <20150417131619.GP1725@tucnak.redhat.com> MIME-Version: 1.0 X-IsSubscribed: yes On Fri, 17 Apr 2015 15:16:19 +0200 Jakub Jelinek wrote: > On Tue, Apr 14, 2015 at 05:43:26PM +0200, Thomas Schwinge wrote: > > On Tue, 14 Apr 2015 15:15:02 +0100, Julian Brown > > wrote: > > > On Wed, 8 Apr 2015 17:58:56 +0300 > > > Ilya Verbin wrote: > > > > I see several regressions: > > > > FAIL: > > > > libgomp.oacc-c/../libgomp.oacc-c-c++-common/acc_on_device-1.c > > > > -DACC_DEVICE_TYPE_host_nonshm=1 -DACC_MEM_SHARED=0 execution > > > > test FAIL: libgomp.oacc-c/../libgomp.oacc-c-c++-common/if-1.c > > > > -DACC_DEVICE_TYPE_host_nonshm=1 -DACC_MEM_SHARED=0 execution > > > > test > > > > > > I think there may be multiple issues here. The attached patch > > > addresses one -- acc_device_type not distinguishing between > > > "offloaded" and host code with the host_nonshm plugin. > > > > (You mean acc_on_device?) > > > > > --- libgomp/oacc-init.c (revision 221922) > > > +++ libgomp/oacc-init.c (working copy) > > > @@ -548,7 +549,14 @@ ialias (acc_set_device_num) > > > int > > > acc_on_device (acc_device_t dev) > > > { > > > - if (acc_get_device_type () == acc_device_host_nonshm) > > > + struct goacc_thread *thr = goacc_thread (); > > > + > > > + /* We only want to appear to be the "host_nonshm" plugin from > > > "offloaded" > > > + code -- i.e. within a parallel region. Test a flag set by > > > the > > > + openacc_parallel hook of the host_nonshm plugin to > > > determine that. */ > > > + if (acc_get_device_type () == acc_device_host_nonshm > > > + && thr && thr->target_tls > > > + && ((struct nonshm_thread *)thr->target_tls)->nonshm_exec) > > > return dev == acc_device_host_nonshm || dev == > > > acc_device_not_host; > > > /* Just rely on the compiler builtin. */ > > > > Really, acc_on_device is implemented as a compiler builtin (which > > is just disabled for a few libgomp test cases, in order to test the > > acc_on_device library function in libgomp), and I never understood > > why the "fallback" implementation in libgomp (cited above) should > > be doing anything different from the GCC builtin. Is the "problem" > > actually, that some > > The question is if the builtin expansion isn't wrong, at least as > long as the host_nonshm device is meant to be supported. The > #ifdef ACCEL_COMPILER > case is easier, at least as long as ACCEL_COMPILER compiled code is > not meant to be able to offload to other devices (or host again), but > the non-ACCEL_COMPILER case means the code is either on the host, or > host_nonshm, or e.g. with Intel MIC you could have some shared > library be compiled by the host compiler, but then actuall linked > into the MIC offloaded path. In all those cases, I think it is just > the library that can determine the return value. > > E.g. OpenMP omp_is_initial_device function is also only implemented > in the library, perhaps at some point I could expand it for #ifdef > ACCEL_COMPILER as builtin, but not for the host code, at least not > due to the host-nonshm plugin. Here's a new version of the patch that doesn't use the open-coded expansion for acc_on_device for the host compiler at all. This means that the host and the host_nonshm plugin should DTRT without any special compiler options (which have thus been removed from the libgomp tests that set them or refer to them). So now, for the host, acc_on_device returns: acc_on_device (acc_device_none): true acc_on_device (acc_device_host): true otherwise: false When the host_nonshm plugin is active, acc_on_device returns: acc_on_device (acc_device_host_nonshm): true (except when "host fallback" is in effect, i.e. because of a false "if" clause). acc_on_device (acc_device_not_host): likewise. otherwise: false In particular, the host_nonshm plugin doesn't consider itself to be running code "on the host". OK for trunk? Julian ChangeLog PR libgomp/65742 gcc/ * builtins.c (expand_builtin_acc_on_device): Don't use open-coded sequence for !ACCEL_COMPILER. libgomp/ * oacc-init.c (plugin/plugin-host.h): Include. (acc_on_device): Check whether we're in an offloaded region for host_nonshm plugin. Don't use __builtin_acc_on_device. * plugin/plugin-host.c (GOMP_OFFLOAD_openacc_parallel): Set nonshm_exec flag in thread-local data. (GOMP_OFFLOAD_openacc_create_thread_data): Allocate thread-local data for host_nonshm plugin. (GOMP_OFFLOAD_openacc_destroy_thread_data): Free thread-local data for host_nonshm plugin. * plugin/plugin-host.h: New. * testsuite/libgomp.oacc-c-c++-common/acc_on_device-1.c: Remove -fno-builtin-acc_on_device flag. * testsuite/libgomp.oacc-c-c++-common/if-1.c: Likewise. * testsuite/libgomp.oacc-fortran/acc_on_device-1-1.f90: Remove comment re: acc_on_device builtin. * testsuite/libgomp.oacc-fortran/acc_on_device-1-2.f: Likewise. * testsuite/libgomp.oacc-fortran/acc_on_device-1-3.f: Likewise. commit adccf2e7d313263d585f63e752a4d36653d47811 Author: Julian Brown Date: Tue Apr 21 12:40:45 2015 -0700 Non-SHM acc_on_device fixes diff --git a/gcc/builtins.c b/gcc/builtins.c index 6fe1456..5930fe4 100644 --- a/gcc/builtins.c +++ b/gcc/builtins.c @@ -5917,6 +5917,7 @@ expand_stack_save (void) static rtx expand_builtin_acc_on_device (tree exp, rtx target) { +#ifdef ACCEL_COMPILER if (!validate_arglist (exp, INTEGER_TYPE, VOID_TYPE)) return NULL_RTX; @@ -5925,13 +5926,8 @@ expand_builtin_acc_on_device (tree exp, rtx target) /* Return (arg == v1 || arg == v2) ? 1 : 0. */ machine_mode v_mode = TYPE_MODE (TREE_TYPE (arg)); rtx v = expand_normal (arg), v1, v2; -#ifdef ACCEL_COMPILER v1 = GEN_INT (GOMP_DEVICE_NOT_HOST); v2 = GEN_INT (ACCEL_COMPILER_acc_device); -#else - v1 = GEN_INT (GOMP_DEVICE_NONE); - v2 = GEN_INT (GOMP_DEVICE_HOST); -#endif machine_mode target_mode = TYPE_MODE (integer_type_node); if (!target || !register_operand (target, target_mode)) target = gen_reg_rtx (target_mode); @@ -5945,6 +5941,9 @@ expand_builtin_acc_on_device (tree exp, rtx target) emit_label (done_label); return target; +#else + return NULL; +#endif } diff --git a/libgomp/oacc-init.c b/libgomp/oacc-init.c index 335ffd4..157147a 100644 --- a/libgomp/oacc-init.c +++ b/libgomp/oacc-init.c @@ -29,6 +29,7 @@ #include "libgomp.h" #include "oacc-int.h" #include "openacc.h" +#include "plugin/plugin-host.h" #include #include #include @@ -611,11 +612,18 @@ ialias (acc_set_device_num) int acc_on_device (acc_device_t dev) { - if (acc_get_device_type () == acc_device_host_nonshm) + struct goacc_thread *thr = goacc_thread (); + + /* We only want to appear to be the "host_nonshm" plugin from "offloaded" + code -- i.e. within a parallel region. Test a flag set by the + openacc_parallel hook of the host_nonshm plugin to determine that. */ + if (acc_get_device_type () == acc_device_host_nonshm + && thr && thr->target_tls + && ((struct nonshm_thread *)thr->target_tls)->nonshm_exec) return dev == acc_device_host_nonshm || dev == acc_device_not_host; - /* Just rely on the compiler builtin. */ - return __builtin_acc_on_device (dev); + /* For OpenACC, libgomp is only built for the host, so this is sufficient. */ + return dev == acc_device_host || dev == acc_device_none; } ialias (acc_on_device) diff --git a/libgomp/plugin/plugin-host.c b/libgomp/plugin/plugin-host.c index 1faf5bc..3cb4dab 100644 --- a/libgomp/plugin/plugin-host.c +++ b/libgomp/plugin/plugin-host.c @@ -44,6 +44,7 @@ #include #include #include +#include #ifdef HOST_NONSHM_PLUGIN #define STATIC @@ -55,6 +56,10 @@ #define SELF "host: " #endif +#ifdef HOST_NONSHM_PLUGIN +#include "plugin-host.h" +#endif + STATIC const char * GOMP_OFFLOAD_get_name (void) { @@ -174,7 +179,10 @@ GOMP_OFFLOAD_openacc_parallel (void (*fn) (void *), void *targ_mem_desc __attribute__ ((unused))) { #ifdef HOST_NONSHM_PLUGIN + struct nonshm_thread *thd = GOMP_PLUGIN_acc_thread (); + thd->nonshm_exec = true; fn (devaddrs); + thd->nonshm_exec = false; #else fn (hostaddrs); #endif @@ -232,11 +240,20 @@ STATIC void * GOMP_OFFLOAD_openacc_create_thread_data (int ord __attribute__ ((unused))) { +#ifdef HOST_NONSHM_PLUGIN + struct nonshm_thread *thd + = GOMP_PLUGIN_malloc (sizeof (struct nonshm_thread)); + thd->nonshm_exec = false; + return thd; +#else return NULL; +#endif } STATIC void -GOMP_OFFLOAD_openacc_destroy_thread_data (void *tls_data - __attribute__ ((unused))) +GOMP_OFFLOAD_openacc_destroy_thread_data (void *tls_data) { +#ifdef HOST_NONSHM_PLUGIN + free (tls_data); +#endif } diff --git a/libgomp/plugin/plugin-host.h b/libgomp/plugin/plugin-host.h new file mode 100644 index 0000000..96955d1 --- /dev/null +++ b/libgomp/plugin/plugin-host.h @@ -0,0 +1,37 @@ +/* OpenACC Runtime Library: acc_device_host, acc_device_host_nonshm. + + Copyright (C) 2015 Free Software Foundation, Inc. + + Contributed by Mentor Embedded. + + This file is part of the GNU Offloading and Multi Processing Library + (libgomp). + + Libgomp is free software; you can redistribute it and/or modify it + under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3, or (at your option) + any later version. + + Libgomp is distributed in the hope that it will be useful, but WITHOUT ANY + WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS + FOR A PARTICULAR PURPOSE. See the GNU General Public License for + more details. + + Under Section 7 of GPL version 3, you are granted additional + permissions described in the GCC Runtime Library Exception, version + 3.1, as published by the Free Software Foundation. + + You should have received a copy of the GNU General Public License and + a copy of the GCC Runtime Library Exception along with this program; + see the files COPYING3 and COPYING.RUNTIME respectively. If not, see + . */ + +#ifndef PLUGIN_HOST_H +#define PLUGIN_HOST_H + +struct nonshm_thread +{ + bool nonshm_exec; +}; + +#endif diff --git a/libgomp/testsuite/libgomp.oacc-c-c++-common/acc_on_device-1.c b/libgomp/testsuite/libgomp.oacc-c-c++-common/acc_on_device-1.c index 81ea476..25cc15a 100644 --- a/libgomp/testsuite/libgomp.oacc-c-c++-common/acc_on_device-1.c +++ b/libgomp/testsuite/libgomp.oacc-c-c++-common/acc_on_device-1.c @@ -1,7 +1,3 @@ -/* Disable the acc_on_device builtin; we want to test the libgomp library - function. */ -/* { dg-additional-options "-fno-builtin-acc_on_device" } */ - #include #include diff --git a/libgomp/testsuite/libgomp.oacc-c-c++-common/if-1.c b/libgomp/testsuite/libgomp.oacc-c-c++-common/if-1.c index 184b355..6aa3bb7 100644 --- a/libgomp/testsuite/libgomp.oacc-c-c++-common/if-1.c +++ b/libgomp/testsuite/libgomp.oacc-c-c++-common/if-1.c @@ -1,5 +1,4 @@ /* { dg-do run } */ -/* { dg-additional-options "-fno-builtin-acc_on_device" } */ #include #include diff --git a/libgomp/testsuite/libgomp.oacc-fortran/acc_on_device-1-1.f90 b/libgomp/testsuite/libgomp.oacc-fortran/acc_on_device-1-1.f90 index 4488818..729b685 100644 --- a/libgomp/testsuite/libgomp.oacc-fortran/acc_on_device-1-1.f90 +++ b/libgomp/testsuite/libgomp.oacc-fortran/acc_on_device-1-1.f90 @@ -1,8 +1,4 @@ ! { dg-additional-options "-cpp" } -! TODO: Have to disable the acc_on_device builtin for we want to test the -! libgomp library function? The command line option -! '-fno-builtin-acc_on_device' is valid for C/C++/ObjC/ObjC++ but not for -! Fortran. use openacc implicit none diff --git a/libgomp/testsuite/libgomp.oacc-fortran/acc_on_device-1-2.f b/libgomp/testsuite/libgomp.oacc-fortran/acc_on_device-1-2.f index 0047a19..19ff4a5 100644 --- a/libgomp/testsuite/libgomp.oacc-fortran/acc_on_device-1-2.f +++ b/libgomp/testsuite/libgomp.oacc-fortran/acc_on_device-1-2.f @@ -1,8 +1,4 @@ ! { dg-additional-options "-cpp" } -! TODO: Have to disable the acc_on_device builtin for we want to test -! the libgomp library function? The command line option -! '-fno-builtin-acc_on_device' is valid for C/C++/ObjC/ObjC++ but not -! for Fortran. USE OPENACC IMPLICIT NONE diff --git a/libgomp/testsuite/libgomp.oacc-fortran/acc_on_device-1-3.f b/libgomp/testsuite/libgomp.oacc-fortran/acc_on_device-1-3.f index 49d7a72..b01c553 100644 --- a/libgomp/testsuite/libgomp.oacc-fortran/acc_on_device-1-3.f +++ b/libgomp/testsuite/libgomp.oacc-fortran/acc_on_device-1-3.f @@ -1,8 +1,4 @@ ! { dg-additional-options "-cpp" } -! TODO: Have to disable the acc_on_device builtin for we want to test -! the libgomp library function? The command line option -! '-fno-builtin-acc_on_device' is valid for C/C++/ObjC/ObjC++ but not -! for Fortran. IMPLICIT NONE INCLUDE "openacc_lib.h"