public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Andrew Stubbs <ams@codesourcery.com>
To: Tobias Burnus <tobias@codesourcery.com>,
	gcc-patches <gcc-patches@gcc.gnu.org>
Cc: Jakub Jelinek <jakub@redhat.com>
Subject: Re: [Patch] libgomp/gcn: Prepare for reverse-offload callback handling
Date: Wed, 12 Oct 2022 18:09:51 +0100	[thread overview]
Message-ID: <3a0eb685-6bb7-ed30-4024-887452c015fd@codesourcery.com> (raw)
In-Reply-To: <1c5166f4-91d2-b320-7fd9-6831c7e26342@codesourcery.com>

On 12/10/2022 15:29, Tobias Burnus wrote:
> On 29.09.22 18:24, Andrew Stubbs wrote:
>> On 27/09/2022 14:16, Tobias Burnus wrote:
>>> Andrew did suggest a while back to piggyback on the console_output 
>>> handling,
>>> avoiding another atomic access. - If this is still wanted, I like to 
>>> have some
>>> guidance regarding how to actually implement it.
>> [...]
>> The point is that you can use the "msg" and "text" fields for whatever 
>> data you want, as long as you invent a new value for "type".
>> [....]
>> You can make "case 4" do whatever you want. There are enough bytes for 
>> 4 pointers, and you could use multiple packets (although it's not safe 
>> to assume they're contiguous or already arrived; maybe "case 4" for 
>> part 1, "case 5" for part 2). It's possible to change this structure, 
>> of course, but the target implementation is in newlib so versioning 
>> becomes a problem.
> 
> I think  – also looking at the Newlib write.c implementation - that the 
> data is contiguous: there is an atomic add, where instead of passing '1' 
> for a single slot, I could also add '2' for two slots.

Right, sorry, the buffer is circular, but the counter is linear.  It 
simplified reservation that way, but it does mean that there's a limit 
to the number of times the buffer can cycle before the counter 
saturates. (You'd need to stream out gigabytes of data to hit the limit 
though.)

> Attached is one variant – for the decl of the GOMP_OFFLOAD_target_rev, 
> it needs the generic parts of the sister nvptx patch.*
> 
> 2*128 bytes were not enough, I need 3*128 bytes. (Or rather 5*64 + 32.) 
> As target_ext is blocking, I decided to use a stack local variable for 
> the remaining arguments and pass it along. Alternatively, I could also 
> use 2 slots - and process them together. This would avoid one 
> device->host memory copy but would make console_output less clear.

> PS: Currently, device stack variables are private and cannot be accessed 
> from the host; this will change in a separate patch. It not only affects 
> the "rest" part as used in this patch but also the actual arrays behind 
> addr, kinds, and sizes. And quite likely a lot of the map/firstprivate 
> variables passed to addr.
> 
> As num_devices() will return 0 or -1, this is for now a non-issue.

So, the patch, as is, is known to be non-functional? How can you have 
tested it? For the addrs_sizes_kind data to be accessible the asm("s8") 
has to be wrong.

I think the patch looks good, in principle. The use of the existing 
ring-buffer is the right way to do it, IMO.

Can we get the manually allocated stacks patch in first and then follow 
up with these patches when they actually work?

Andrew

  reply	other threads:[~2022-10-12 17:09 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-27 13:15 Tobias Burnus
2022-09-27 13:16 ` Tobias Burnus
2022-09-29 16:24   ` Andrew Stubbs
2022-10-12 14:29     ` Tobias Burnus
2022-10-12 17:09       ` Andrew Stubbs [this message]
2022-10-12 17:25         ` Tobias Burnus
2022-11-18 17:41       ` Tobias Burnus
2022-11-18 17:56         ` Andrew Stubbs
2022-11-21 13:40           ` [Patch] libgomp/gcn: fix/improve struct output (was: [Patch] libgomp/gcn: Prepare for reverse-offload callback handling) Tobias Burnus
2022-11-21 14:19             ` Andrew Stubbs

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=3a0eb685-6bb7-ed30-4024-887452c015fd@codesourcery.com \
    --to=ams@codesourcery.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jakub@redhat.com \
    --cc=tobias@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).