From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-io1-xd42.google.com (mail-io1-xd42.google.com [IPv6:2607:f8b0:4864:20::d42]) by sourceware.org (Postfix) with ESMTPS id 00BA23851C12 for ; Tue, 14 Jul 2020 15:10:12 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 00BA23851C12 Received: by mail-io1-xd42.google.com with SMTP id a12so17584178ion.13 for ; Tue, 14 Jul 2020 08:10:12 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=mRPChhUIiA5Hc29H9BLF9dLrQZRB4lIZ0Fa1b7SzV8U=; b=PIBLtV07A1vu7L1B1849j3FY/C0ZNGc1DLNNcvq1hFwYu/uZPV6lvo/BOFxRJCXj+K Q1IiQYrJwzDmfWL4AAUX2HypjW+vuVnwkzdFIV0y9XeCjaBha2hObWwtTnTVPy/uxzxf kyx4gHrjP/Nr8uL/R4hZex0LQcBZVK+hBrl5qOG8xawyVdW2GRCcv4njtJr6gD2EC0hU VwSjrRFngPDrOJ1EZpr3XvIfwxXn/HfZPWGW2RkNyKJTOJTAR+YS6LWBk3m/tvEAjYki EREPB3rcM8PRjOSHkDBlyudB1T63//ndt9C9AN/JAxnYovBahtYrbyf0oGzyuJOqE1fw 4Bow== X-Gm-Message-State: AOAM533deF+U+0dhdPWtmXYxaSFU6KIjxulmJ5pihcErpLm4XFcOC+je XhzUoAFZMOQNHKPc0TXE6W202N06eXaZ2AwUJCI= X-Google-Smtp-Source: ABdhPJwlogVBQxYNDtckSPCpdo2XfeuHwugAnC0hFLj+l4EZj0MupKtueXwZFyzYLJJ+Goz/S2qy3nQIt+3yWsMbE7k= X-Received: by 2002:a02:7616:: with SMTP id z22mr6303590jab.62.1594739412232; Tue, 14 Jul 2020 08:10:12 -0700 (PDT) MIME-Version: 1.0 References: <20200711220536.2728898-1-y2s1982@gmail.com> <20200714100714.GR2363@tucnak> In-Reply-To: <20200714100714.GR2363@tucnak> From: y2s1982 Date: Tue, 14 Jul 2020 11:10:01 -0400 Message-ID: Subject: Re: [PATCH] libgomp: Add OMPD functions in 5.5.6 and related data types. To: Jakub Jelinek Cc: Martin Jambor , gcc-patches@gcc.gnu.org X-Spam-Status: No, score=-3.0 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_ENVFROM_END_DIGIT, FREEMAIL_FROM, HTML_MESSAGE, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.29 X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 14 Jul 2020 15:10:14 -0000 Hello Jakub, On Tue, Jul 14, 2020 at 6:07 AM Jakub Jelinek wrote: > On Sat, Jul 11, 2020 at 06:05:36PM -0400, y2s1982 wrote: > > 2020-07-11 Tony Sim > > > > 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 > >