Re: [PATCH] [libgomp] Add a stream communication framework to GOMP

["Antoniu Pop" <antoniu dot pop at gmail dot com>]

Hi,

I have a correction to make on the previous patch. The code for the
function wait_used_space should read:

+/* Wait until the producer has slided the write window in stream S.  */
+
+static inline void
+wait_used_space (gomp_stream s)
+{
+  while (s->write_buffer_index == s->read_buffer_index)
+    do_wait ((int *) &s->write_buffer_index, s->read_buffer_index);
+}

instead of:

+/* Wait until the producer has slided the write window in stream S.  */
+
+static inline void
+wait_used_space (gomp_stream s)
+{
+  while (s->read_buffer_index == s->write_buffer_index)
+    do_wait ((int *) &s->read_buffer_index, s->write_buffer_index);
+}

The write_buffer_index and read_buffer_index were reversed and though
it is not semantically incorrect, it might not be properly optimized
or even incorrect in the do_wait (or more precisely in the
__builtin_expect). The address where a value change will allow to
continue is that of the write_buffer_index as the producer needs to
slide his write window.

Antoniu

On Tue, Jun 3, 2008 at 1:05 AM, Antoniu Pop <antoniu.pop@gmail.com> wrote:
> Hi,
>
> This is a revised version of the patch, targetting the gomp-3_0-branch.
> The gomp-3_0-branch plus patch have been bootstrapped and tested on amd64-linux.
>
> Antoniu
>
> ChangeLog:
>
> 2008-04-22  Antoniu Pop  <antoniu.pop@gmail.com>
>           Sebastian Pop  <sebastian.pop@amd.com>
>
> libgomp/
>       * libgomp.h (gomp_stream): Declared.
>
>       * stream.c: New.
>       * libgomp_g.h (GOMP_stream_create, GOMP_stream_push,
>       GOMP_stream_head, GOMP_stream_pop, GOMP_stream_eos_p,
>       GOMP_stream_set_eos, GOMP_stream_destroy, GOMP_stream_align_push,
>       GOMP_stream_align_pop): Declared.
>       * libgomp.map: Export GOMP_*.
>
>       * Makefile.am: Added stream.c to libgomp_la_SOURCES.
>       * Makefile.in: Regenerate.
>
> gcc/
>       * builtin-types.def (BT_FN_BOOL_PTR, BT_FN_VOID_PTR_PTR_SIZE):
>       New types.
>       * fortran/types.def (BT_FN_BOOL_PTR, BT_FN_PTR_SIZE_SIZE,
>       BT_FN_VOID_PTR_SIZE, BT_FN_VOID_PTR_PTR_SIZE): New types.
>
>       * omp-builtins.def (BUILT_IN_GOMP_STREAM_CREATE,
>       BUILT_IN_GOMP_STREAM_PUSH, BUILT_IN_GOMP_STREAM_HEAD,
>       BUILT_IN_GOMP_STREAM_POP, BUILT_IN_GOMP_STREAM_EOS_P,
>       BUILT_IN_GOMP_STREAM_SET_EOS, BUILT_IN_GOMP_STREAM_DESTROY,
>       BUILT_IN_GOMP_STREAM_ALIGN_PUSH, BUILT_IN_GOMP_STREAM_ALIGN_POP):
>       New builtins.
>
>
> On Wed, Apr 30, 2008 at 6:37 PM, Sebastian Pop <sebpop@gmail.com> wrote:
>> On Fri, Apr 25, 2008 at 3:39 AM, Jakub Jelinek <jakub@redhat.com> wrote:
>>>  Unconditional busy waiting and sched_yield is a bad idea.  IMHO you
>>>  want a proper synchronization primitive, with optional busy
>>>  waiting.  See gomp-3_0-branch, where user can determine how long if
>>>  at all threads should be busy waiting through GOMP_SPINCOUNT and
>>>  OMP_WAIT_POLICY env vars, throttling is used for number of active
>>>  threads bigger than number of available CPUs and after that falls
>>>  back to futexes (on Linux, on other OSes whatever is supported).
>>
>> If I understand correctly, we should use do_wait instead of
>> sched_yield, so we will have to work in the gomp-3_0-branch.
>>
>> We started working on the gomp-2.5 and we were quite limited by the
>> tasking capabilities: we used parallel sections instead of tasks; but
>> the goal was to use the more flexible task support of gomp-3.0 for being
>> able to dynamically create tasks.
>>
>> Sebastian
>>
>