Hello Jakub,
On Tue, Jul 14, 2020 at 6:07 AM Jakub Jelinek <ja...@redhat.com> wrote:
> On Sat, Jul 11, 2020 at 06:05:36PM -0400, y2s1982 wrote:
> > 2020-07-11 Tony Sim <y2s1...@gmail.com>
> >
> > libgomp/ChangeLog:
> >
> > * libgompd.h (ompd_thread_handle_t): Add.
> > (ompd_parallel_handle_t): Add.
> > (ompd_task_handle_t): Add.
> > * ompd-parallel.c: New file.
>
> So you add a new file, but don't add it to Makefile.am - that means
> nothing will compile it.
> +}ompd_thread_handle_t;
>
> Formatting, space after }
> > +
> > +typedef struct _ompd_parallel_handle{
>
> and space bef before { (etc.).
> > + ompd_address_space_handle_t *ah;
> > + ompd_parallel_handle_t *enclosing_ph;
> > + ompd_size_t enclosed_ph;
> > + ompd_address_t thread_pool; /* Stores gomp_thread_pool *. */
> > +}ompd_parallel_handle_t;
> > +
> > +typedef struct _ompd_task_handle{
> > + ompd_parallel_handle_t *ph;
> > +}ompd_task_handle_t;
> > +
> > #endif /* LIBGOMPD_H */
> > + ompd_rc_t ret = ompd_rc_error;
> > + ompd_size_t i = 0;
> > + struct gomp_thread_pool * pool =
> > + (struct gomp_thread_pool *)ph->thread_pool.address;
>
> Formatting, = should never be at the end of line. And no space between
> * and pool, so:
> struct gomp_thread_pool *pool
> = (struct gomp_thread_pool *) ph->thread_pool.address;
>
Ah! I was hoping to find some information on this. I kept looking around the
code base but the formatting wasn't consistent. Thank you for spelling this
out for me.
>
> > + for (i = 0; i < pool->threads_used && ret == ompd_rc_error; i++)
> > + {
> > + if (pool->threads[i]->ts.team == NULL)
> > + ret = ompd_rc_ok;
> > + }
>
> Like I said on other patches, { would need to be indented by 2 spaces from
> for, but as the body contains a single statement, just leave the {}s out
> completely and then it can stay as is.
>
Yes, of course. I am sorry, my old habbit is dieing hard. I will keep it in
mind.
>
> More important, I don't see any function that would initialize
> e.g. threads_used, etc., IMNSHO you should start with those and
> there write the reading of those from the inferior.
>
I started from parallel only because it seemed easier haha.
Okay, I will get working on the thread section, which was just previous to
this section.
Does this mean I should scrap this patch for now and submit something on
this section after the thread section is completed?
And, unless that routine copies everything from the inferior, which is risky
> because it can change there any time, I think the above is not really what
> you want, you instead want to read it from the inferior.
>
When you say inferior, are you referring to the gomp_thread and
gomp_thread_pool?
The debugged process (if it is a process and not e.g. a core file) is not in
> the same address space as the debugger (that loads the OMPD library), so
> even if you get pointers copied from the debugged process, you can't
> dereference them, but need to use the callbacks to read the corresponding
> memory.
>
Okay. I was afraid I might have made a mistake with this regard. The use of
callbacks was to make the debugging process decoupled, right? I think I
remember reading the example of running the process and debugging in a
different machine.
Thank you again for the help.
Cheers,
Tony Sim
>
> Jakub
>
>
