diff mbox

[gomp4] OpenACC acc_on_device

Message ID 87lhnwed1c.fsf@kepler.schwinge.homeip.net
State New
Headers show

Commit Message

Thomas Schwinge Oct. 31, 2014, 10:42 a.m. UTC 
Hi!

On Thu, 18 Sep 2014 20:01:02 +0200, I wrote:
> Here is my OpenACC acc_on_device patch, in a more complete form, with
> test cases and all that.
> 
> On Wed, 17 Sep 2014 10:49:54 +0200, Jakub Jelinek <jakub@redhat.com> wrote:
> > On Wed, Sep 17, 2014 at 10:44:12AM +0200, Tobias Burnus wrote:
> > > Cesar Philippidis wrote:
> > > > The patch introduces the following OpenACC/PTX-specific built-ins:
> > > ...
> > > 
> > > It is not completely clear how they are supposed to get used. Should the
> > > user call them directly in some cases? Or are they only used internally?
> > > 
> > > acc_on_device sounds like a function which would be in C/C++ made available
> > > to the user via #define acc_on_device __builtin_acc_on_device.
> > 
> > And not just providing acc_on_device prototype in some header?
> 
> Yes, just a prototype.  And next to DEF_GOACC_BUILTIN (configured the
> same as DEF_GOMP_BUILTIN), I add a new DEF_GOACC_BUILTIN_COMPILER that is
> configured to always provide the __builtin_[...] variant, but the
> un-prefixed [...]  only if -fopenacc is in effect.  Does that look
> alright?
> 
> > Without
> > looking at the OpenACC standard, it sounds like this function could be
> > similar to omp_is_initial_device, so can and should be handled supposedly
> > similarly.
> 
> I think we've been talking about this at the Cauldron, where you agreed
> that omp_is_initial_device should also be implemented as a builtin.  (Or
> am I confusing things?)
> 
> > > However, the rest looks as if it should rather be an internal function
> > > instead of a builtin. Or should the user really ever call the builtin
> > > directly?
> > 
> > GOMP_* functions are builtins and not internal functions too, all those
> > functions are library functions, while the user typically doesn't call them
> > directly, they still are implemented in the library.  Internal functions are
> > used for something that doesn't have a library implementation and is not
> > something user can call directly.
> 
> > > Regarding Fortran: Builtins aren't directly available to the user. You have to
> > > wrap them into an intrinsic to make them available. If they have to be made
> > > available via a module (e.g. via "module acc) - you have to create a virtual
> > > module, which provides the intrinsic. If you don't want to convert the whole
> > > module, you could create an auxiliar module (e.g. acc_internal_) which provides
> > > only those bits - and then include it ("use,intrinsic :: ...") it in the
> > > main module - written in normal Fortran.
> 
> This I have not yet addressed -- please see the TODO comments in the
> gcc/fortran/ files as well as Fortran test cases.
> 
> > For the user callable fortran functions, for OpenMP libgomp just provides
> > *_ entrypoints to * functions.  Perhaps acc_on_device_ could be provided
> > too.
> 
> This is what I had done already.
> 
> Does that patch look good?  (With the Fortran things still to be
> addressed.)

(Checked in, back then, to gomp-4_0-branch in r215506.)

>     	gcc/testsuite/
>     	* c-c++-common/goacc/acc_on_device-1.c: New file.
>     	* c-c++-common/goacc/acc_on_device-2.c: Likewise.
>     	* c-c++-common/goacc/acc_on_device-2-off.c: Likewise.

Here is a patch, checked in to gomp-4_0-branch in r216953, to make
acc_on_device-1.c C-only (implicitly declared functions are only
"supported" in C), and make the others actually fit for C++ -- and XFAIL
the C++ case.  How to resolve that one?

commit b1a009fdf340acf1840c1b6c9022be69a8f0a661
Author: tschwinge <tschwinge@138bc75d-0d04-0410-961f-82ee72b054a4>
Date:   Fri Oct 31 10:39:44 2014 +0000

    Make acc_on_device test cases fit for C++.
    
    	gcc/testsuite/
    	* c-c++-common/goacc/acc_on_device-1.c: Move...
    	* gcc.dg/goacc/acc_on_device-1.c: ... here.
    	(dg-additional-options): Add -std=c89.
    	* c-c++-common/goacc/acc_on_device-2-off.c: Extend for C++.
    	* c-c++-common/goacc/acc_on_device-2.c: Likewise.
    
    git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/branches/gomp-4_0-branch@216953 138bc75d-0d04-0410-961f-82ee72b054a4
---
 gcc/testsuite/ChangeLog.gomp                             |  8 ++++++++
 gcc/testsuite/c-c++-common/goacc/acc_on_device-2-off.c   | 10 +++++++++-
 gcc/testsuite/c-c++-common/goacc/acc_on_device-2.c       | 16 ++++++++++++++--
 .../{c-c++-common => gcc.dg}/goacc/acc_on_device-1.c     |  2 +-
 4 files changed, 32 insertions(+), 4 deletions(-)



Grüße,
 Thomas
diff mbox

Patch

diff --git gcc/testsuite/ChangeLog.gomp gcc/testsuite/ChangeLog.gomp
index 10232bc..2489b39 100644
--- gcc/testsuite/ChangeLog.gomp
+++ gcc/testsuite/ChangeLog.gomp
@@ -1,3 +1,11 @@ 
+2014-10-30  Thomas Schwinge  <thomas@codesourcery.com>
+
+	* c-c++-common/goacc/acc_on_device-1.c: Move...
+	* gcc.dg/goacc/acc_on_device-1.c: ... here.
+	(dg-additional-options): Add -std=c89.
+	* c-c++-common/goacc/acc_on_device-2-off.c: Extend for C++.
+	* c-c++-common/goacc/acc_on_device-2.c: Likewise.
+
 2014-10-20  Thomas Schwinge  <thomas@codesourcery.com>
 
 	* gcc.dg/goacc/sb-1.c: Move file...
diff --git gcc/testsuite/c-c++-common/goacc/acc_on_device-2-off.c gcc/testsuite/c-c++-common/goacc/acc_on_device-2-off.c
index ddc43ab..25d21ad 100644
--- gcc/testsuite/c-c++-common/goacc/acc_on_device-2-off.c
+++ gcc/testsuite/c-c++-common/goacc/acc_on_device-2-off.c
@@ -1,13 +1,21 @@ 
 /* Have to enable optimizations, as otherwise builtins won't be expanded.  */
 /* { dg-additional-options "-O -fdump-rtl-expand -fno-openacc" } */
 
+#if __cplusplus
+extern "C" {
+#endif
+
 typedef enum acc_device_t { acc_device_X = 123 } acc_device_t;
 extern int acc_on_device (acc_device_t);
 
+#if __cplusplus
+}
+#endif
+
 int
 f (void)
 {
-  const int dev = acc_device_X;
+  const acc_device_t dev = acc_device_X;
   return acc_on_device (dev);
 }
 
diff --git gcc/testsuite/c-c++-common/goacc/acc_on_device-2.c gcc/testsuite/c-c++-common/goacc/acc_on_device-2.c
index 65b4ae6..d5389a9 100644
--- gcc/testsuite/c-c++-common/goacc/acc_on_device-2.c
+++ gcc/testsuite/c-c++-common/goacc/acc_on_device-2.c
@@ -1,17 +1,29 @@ 
 /* Have to enable optimizations, as otherwise builtins won't be expanded.  */
 /* { dg-additional-options "-O -fdump-rtl-expand" } */
 
+#if __cplusplus
+extern "C" {
+#endif
+
 typedef enum acc_device_t { acc_device_X = 123 } acc_device_t;
 extern int acc_on_device (acc_device_t);
 
+#if __cplusplus
+}
+#endif
+
 int
 f (void)
 {
-  const int dev = acc_device_X;
+  const acc_device_t dev = acc_device_X;
   return acc_on_device (dev);
 }
 
 /* With -fopenacc, we're expecting the builtin to be expanded, so no calls.
-   { dg-final { scan-rtl-dump-times "\\\(call \[^\\n\]*\\\"acc_on_device" 0 "expand" } } */
+   TODO: in C++, even under extern "C", the use of enum for acc_device_t
+   perturbs expansion as a builtin, which expects an int parameter.  It's fine
+   when changing acc_device_t to plain int, but that's not what we're doing in
+   <openacc.h>.
+   { dg-final { scan-rtl-dump-times "\\\(call \[^\\n\]*\\\"acc_on_device" 0 "expand" { xfail c++ } } } */
 
 /* { dg-final { cleanup-rtl-dump "expand" } } */
diff --git gcc/testsuite/c-c++-common/goacc/acc_on_device-1.c gcc/testsuite/gcc.dg/goacc/acc_on_device-1.c
similarity index 82%
rename from gcc/testsuite/c-c++-common/goacc/acc_on_device-1.c
rename to gcc/testsuite/gcc.dg/goacc/acc_on_device-1.c
index e606b88..1a0276e 100644
--- gcc/testsuite/c-c++-common/goacc/acc_on_device-1.c
+++ gcc/testsuite/gcc.dg/goacc/acc_on_device-1.c
@@ -1,5 +1,5 @@ 
 /* Have to enable optimizations, as otherwise builtins won't be expanded.  */
-/* { dg-additional-options "-O -fdump-rtl-expand -Wno-implicit-function-declaration" } */
+/* { dg-additional-options "-O -fdump-rtl-expand -std=c89 -Wno-implicit-function-declaration" } */
 
 int
 f (void)