Re: [PATCH] libgompd: Fix sizes in OMPD support and add local ICVs finctions.

[Mohamed Atef <mohamedatef1698@gmail.com>]

I forgot the DCO line. And I edited the commit message, but I can't push,
even forced push doesn't work.
Now I get the non-fast-forward error, is there any way to solve this?

On Mon, Jun 20, 2022 at 9:31 AM Jakub Jelinek <jakub@redhat.com> wrote:

> On Fri, Jun 17, 2022 at 01:20:28AM +0200, Mohamed Atef wrote:
> > libgomp/ChangeLog
> >
> > 2022-06-17  Mohamed Atef  <mohamedatef1698@gmail.com>
> >
> > * ompd-helper.h (DEREFERENCE, ACCESS_VALUE): New macros.
> > (gompd_get_proc_bind): Change the returned value from ompd_word_t
> > to const char *.
> > (gompd_get_max_task_priority): Fix format.
> > (gompd_stringize_gompd_enabled): Removed.
> > (gompd_get_gompd_enabled): New function prototype.
> > * ompd-helper.c (gompd_get_affinity_format): Call CHECK_RET.
> > Fix format in gompd_enabled GET_VALUE.
> > (gompd_stringize_gompd_enabled): Removed.
> > (gompd_get_nthread, gompd_get_thread_limit, gompd_get_run_sched,
> > gompd_get_run_sched_chunk_size, gompd_get_default_device,
> > gompd_get_dynamic, gompd_get_max_active_levels, gompd_get_proc_bind,
> > gompd_is_final,
> > gompd_is_implicit, gompd_get_team_size): New functions.
> > (gompd_get_gompd_enabled): Change the returned value from
> > ompd_word_t to const char *.
> > * ompd-init.c (ompd_process_initialize): Use sizeof_short instead of
> > sizeof_long_long in GET_VALUE argument.
> > * ompd-support.h: Change type from __UINT64_TYPE__ to unsigned short.
> > (GOMPD_FOREACH_ACCESS): Add entries for gomp_task kind
> > and final_task and gomp_team nthreads.
> > * ompd-support.c (gompd_get_offset, gompd_get_sizeof_member,
> > gompd_get_size, OMPD_SECTION): Define.
> > (gompd_access_gomp_thread_handle,
> > gompd_sizeof_gomp_thread_handle): New variables.
> > (gompd_state): Change type from __UNIT64_TYPE__ to
> > unsigned short.
> > (gompd_load): Remove gompd_init_access, gompd_init_sizeof_members,
> > gompd_init_sizes, gompd_access_gomp_thread_handle,
> > gompd_sizeof_gomp_thread_handle.
> > * ompd-icv.c (ompd_get_icv_from_scope): Add thread_handle,
> > task_handle and parallel_handle. Fix format in ashandle definition.
>
> Just a nit.  After . there should be 2 spaces instead of one
> unless it is at the end of line.
>
> > Call gompd_get_nthread, gompd_get_thread_limit, gomp_get_run_shed,
> > gompd_get_run_sched_chunk_size, gompd_get_default_device,
> > gompd_get_dynamic, gompd_get_max_active_levels, gompd_get_proc_bind,
> > gompd_is_final,
> > gompd_is_implicit,
> > and gompd_get_team_size.
> > (ompd_get_icv_string_from_scope): Fix format in ashandle definition.
> > Add task_handle. Call gompd_get_gompd_enabled, and
>
> Here too.
>
> > gompd_get_proc_bind. Remove the call to
> > gompd_stringize_gompd_enabled.
>
> > +
> > +unsigned short gompd_access_gomp_thread_handle;
> > +unsigned short gompd_sizeof_gomp_thread_handle;
>
> This is undesirable, both because you are then mixing
> const and non-const objects in OMPD_SECTION if GOMP_NEEDS_THREAD_HANDLE
> is defined and because you need to duplicate the stuff in the macros.
> I'd suggest
> #ifndef GOMP_NEEDS_THREAD_HANDLE
> const unsigned short gompd_access_gomp_thread_handle
>   __attribute__ ((used)) OMPD_SECTION = 0;
> const unsigned short gompd_sizeof_gomp_thread_handle
>   __attribute__ ((used)) OMPD_SECTION = 0;
> #endif
>
> > +/* Get offset of the member m in struct t.  */
> > +#define gompd_get_offset(t, m) \
> > +  const unsigned short gompd_access_##t##_##m __attribute__ ((used)) \
> > +    OMPD_SECTION \
> > +      = (unsigned short) offsetof (struct t, m);
> > +  GOMPD_FOREACH_ACCESS (gompd_get_offset)
> > +#ifdef GOMP_NEEDS_THREAD_HANDLE
> > +  gompd_access_gomp_thread_handle __attribute__ ((used)) OMPD_SECTION
> > +    = (unsigned short) offsetof (gomp_thread, handle);
> > +#endif
>
> Remove the above 4 lines.
>
> > +#undef gompd_get_offset
> > +/* Get size of member m in struct t.  */
> > +#define gompd_get_sizeof_member(t, m) \
> > +  const unsigned short gompd_sizeof_##t##_##m __attribute__ ((used)) \
> > +    OMPD_SECTION \
> > +      = sizeof (((struct t *) NULL)->m);
> > +  GOMPD_FOREACH_ACCESS (gompd_get_sizeof_member)
> > +#ifdef GOMP_NEEDS_THREAD_HANDLE
> > +  gompd_sizeof_gomp_thread_handle __attribute__ ((used)) OMPD_SECTION
> > +    = sizeof (((struct gomp_thread *) NULL)->handle);
> > +#endif
>
> And these.
>
> > +#undef gompd_get_sizeof_member
> > +/* Get size of struct t.  */
> > +#define gompd_get_size(t) \
> > +  const unsigned short gompd_sizeof_##t##_ __attribute__ ((used)) \
> > +    OMPD_SECTION \
> > +      = sizeof (struct t);
> > +  GOMPD_SIZES (gompd_get_size)
> > +#undef gompd_get_size
> >
> > --- a/libgomp/ompd-support.h
> > +++ b/libgomp/ompd-support.h
> > @@ -67,7 +67,7 @@
> >  #endif
> >
> >  void gompd_load (void);
> > -extern __UINT64_TYPE__ gompd_state;
> > +extern unsigned short gompd_state;
> >
> >  #define OMPD_ENABLED 0x1
>
> #ifdef GOMP_NEEDS_THREAD_HANDLE
> #define gompd_thread_handle_access gompd_access (gomp_thread, handle)
> #else
> #define gompd_thread_handle_access
> #endif
>
> above the following macro.
>
> > @@ -83,7 +83,10 @@ extern __UINT64_TYPE__ gompd_state;
> >    gompd_access (gomp_thread_pool, threads) \
> >    gompd_access (gomp_thread, ts) \
> >    gompd_access (gomp_team_state, team_id) \
> > -  gompd_access (gomp_task, icv)
> > +  gompd_access (gomp_task, icv) \
> > +  gompd_access (gomp_task, kind) \
> > +  gompd_access (gomp_task, final_task) \
> > +  gompd_access (gomp_team, nthreads)
>
> and add \
>   gompd_thread_handle_access
> here.
>
> Otherwise LGTM.
>
>         Jakub
>
>