public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jakub Jelinek <jakub@redhat.com>
To: Julian Brown <julian@codesourcery.com>
Cc: James Norris <jnorris@codesourcery.com>,
	       GCC Patches <gcc-patches@gcc.gnu.org>,
	       "Joseph S. Myers" <joseph@codesourcery.com>,
	       Nathan Sidwell <Nathan_Sidwell@mentor.com>
Subject: Re: [OpenACC 0/7] host_data construct
Date: Thu, 19 Nov 2015 15:57:00 -0000	[thread overview]
Message-ID: <20151119155723.GA5675@tucnak.redhat.com> (raw)
In-Reply-To: <20151119142650.5a8842e4@octopus>

On Thu, Nov 19, 2015 at 02:26:50PM +0000, Julian Brown wrote:
> OK, thanks -- as to what the standard says, it's so ill-specified in
> this area that nothing can be learned about the behaviour of offloaded
> regions within host_data constructs, and my question about that on the
> technical mailing list is still unanswered (actually Nathan suggested
> in private mail that the conservative thing to do would be to disallow
> offloaded regions entirely within host_data constructs, so maybe that's
> the way to go).
> 
> OpenMP 4.5 seems to *not* specify the skipping-over behaviour for
> use_device_ptr variables (p105, lines 20-23):
> 
> "The is_device_ptr clause is used to indicate that a list item is a
> device pointer already in the device data environment and that it
> should be used directly. Support for device pointers created outside
> of OpenMP, specifically outside of the omp_target_alloc routine and the
> use_device_ptr clause, is implementation defined."
> 
> That suggests that use_device_ptr is a valid way to create device
> pointers for use in enclosed target regions: the behaviour I assumed
> was wrong for OpenACC. So I think my guess at the "most-obvious"
> behaviour was probably misguided anyway.

use_device_ptr kind of privatizes the variable, the private variable being
the device pointer corresponding to the host pointer outside of the target
data with use_device_ptr clause.

And, if you want to use that device pointer in a target region, it should be
on the is_device_ptr clause on the target construct.  See e.g.
libgomp.c/target-18.c testcase.
  int a[4];
...
  #pragma omp target data map(to:a)
  #pragma omp target data use_device_ptr(a) map(from:err)
  #pragma omp target is_device_ptr(a) private(i) map(from:err)
  {
    err = 0;
    for (i = 0; i < 4; i++)
      if (a[i] != 23 + i)
        err = 1;
  }
The implementation has this way a choice how to implement device pointers
(what use_device_ptr gives you, or say omp_target_alloc returns)
- either (GCC's choice at least for the XeonPhi and hopefully PTX, HSA does
not care, as it shares address space) implement them as host pointer
encoding the bits the target device wants to use, or some kind of
descriptor.  In the former case, is_device_ptr is essentially a
firstprivate, you bitwise copy the device pointer from the host to target
device, where you can dereference it etc.  In the descriptor case you'd
do some transformation of the host side representation of the device pointer
to the device side.

> 
> It's maybe even more complicated. Consider the example:
> 
> char x[1024];
> 
> #pragma acc enter data copyin(x)
> 
> #pragma acc host_data use_device(x)
> {
>   target_primitive(x);
>   #pragma acc parallel present(x)    [1]
>   {
>     x[5] = 0;                        [2]
>   }
> }

If it is unclear, I think disallowing acc {parallel,kernels} inside of
acc host_data might be too big hammer, but perhaps just erroring out
or warning during gimplification that if you (explicitly or implicitly)
try to map a var that is in use_device clause in some outer context,
it is either wrong, unsupported or will not do what users think?

I will double check on omp-lang, but supposedly we could for OpenMP
warn in similar cases (use_device_ptr clause instead of use_device),
except when it is passed to is_device_ptr clause, because I think the
behavior is just unspecified otherwise.
> 
> Here, the "present" clause marked [1] will fail (because 'x' is a
> target pointer now). If it's omitted, the array access [2] will cause an
> implicit present_or_copy to be used for the 'x' pointer (which again
> will fail, because now 'x' points to target data). Maybe what we
> actually need is,
> 
> #pragma acc host_data use_device(x)
> {
>   target_primitive(x);
>   #pragma acc parallel deviceptr(x)
>   {
>     ...
>   }
> }
> 
> with the deviceptr(x) clause magically substituted in the parallel
> construct, but I'm struggling to see how we could justify doing that
> when that behaviour's not mentioned in the spec at all.

Is deviceptr as above meant to work?  That is the OpenACC counterpart
of is_device_ptr, right?  If yes, then I'd suggest just warning if you
try to implicitly or explicitly map something use_device in outer contexts,
and just make sure you don't ICE on the cases where you warn.
If the standard does not say what it means, then it is unspecified
behavior...

	Jakub

  reply	other threads:[~2015-11-19 15:57 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-10-22 19:14 James Norris
2015-10-22 19:15 ` [OpenACC 2/7] host_data construct (C FE) James Norris
2015-10-22 19:15 ` [OpenACC 1/7] host_data construct (C/C++ common) James Norris
2015-10-22 19:16 ` [OpenACC 3/7] host_data construct (C front-end) James Norris
2015-10-22 19:18 ` [OpenACC 4/7] host_data construct (middle end) James Norris
2015-10-22 19:19 ` [OpenACC 5/7] host_data construct (gcc tests) James Norris
2015-10-22 19:20 ` [OpenACC 6/7] host_data construct James Norris
2015-10-22 19:22 ` [OpenACC 7/7] host_data construct (runtime tests) James Norris
2015-10-22 20:42 ` [OpenACC 0/7] host_data construct Joseph Myers
2015-10-22 20:53   ` James Norris
2015-10-23 16:01 ` [Bulk] " James Norris
2015-10-26 18:36   ` Jakub Jelinek
2015-10-27 15:57     ` Cesar Philippidis
2015-11-02 18:33     ` Julian Brown
2015-11-02 19:29       ` Jakub Jelinek
2015-11-12 11:16       ` Julian Brown
2015-11-18 12:48         ` Julian Brown
2015-11-19 13:13           ` Jakub Jelinek
2015-11-19 14:29             ` Julian Brown
2015-11-19 15:57               ` Jakub Jelinek [this message]
2015-11-30 19:34                 ` Julian Brown
2015-12-01  8:30                   ` Jakub Jelinek
2015-12-02 15:27                   ` Tom de Vries
2015-12-02 15:59                   ` Thomas Schwinge
2015-12-02 19:16                     ` Cesar Philippidis
2015-12-02 19:28                       ` Steve Kargl
2015-12-02 19:35                       ` Jakub Jelinek
2015-12-02 19:54                         ` Cesar Philippidis
2015-12-02 22:14                     ` [gomp4] " Thomas Schwinge
2016-04-08 13:41                       ` Fortran OpenACC host_data construct ICE (was: [gomp4] Re: [OpenACC 0/7] host_data construct) Thomas Schwinge
2016-02-02 13:57                     ` [OpenACC 0/7] host_data construct Thomas Schwinge
2015-11-13 15:31       ` [Bulk] " Jakub Jelinek
2015-12-23 11:02     ` Thomas Schwinge

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20151119155723.GA5675@tucnak.redhat.com \
    --to=jakub@redhat.com \
    --cc=Nathan_Sidwell@mentor.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jnorris@codesourcery.com \
    --cc=joseph@codesourcery.com \
    --cc=julian@codesourcery.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).