From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 94C1C384B834 for ; Mon, 6 Jun 2022 17:34:29 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 94C1C384B834 Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-235-UxyuzAqmOFGN4ILtWZA1vQ-1; Mon, 06 Jun 2022 13:34:28 -0400 X-MC-Unique: UxyuzAqmOFGN4ILtWZA1vQ-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 9F4AB801228; Mon, 6 Jun 2022 17:34:27 +0000 (UTC) Received: from tucnak.zalov.cz (unknown [10.39.192.11]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 604771121314; Mon, 6 Jun 2022 17:34:27 +0000 (UTC) Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.17.1/8.17.1) with ESMTPS id 256HYOvG192646 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NOT); Mon, 6 Jun 2022 19:34:25 +0200 Received: (from jakub@localhost) by tucnak.zalov.cz (8.17.1/8.17.1/Submit) id 256HYOoU192645; Mon, 6 Jun 2022 19:34:24 +0200 Date: Mon, 6 Jun 2022 19:34:23 +0200 From: Jakub Jelinek To: Mohamed Sayed Cc: gcc-patches@gcc.gnu.org Subject: Re: [PATCH]: libgompd add parallel handle functions Message-ID: Reply-To: Jakub Jelinek References: MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.78 on 10.11.54.3 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=us-ascii Content-Disposition: inline X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org 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: Mon, 06 Jun 2022 17:34:31 -0000 On Mon, Jun 06, 2022 at 05:13:24PM +0200, Mohamed Sayed via Gcc-patches wrote: > > 2022-06-06 Mohamed Sayed > > > > > > * Makefile.am: (libgompd_la_SOURCES): Add ompd-parallel.c. The ChangeLog formatting is wrong. The above line should be: * Makefile.am (libgompd_la_SOURCES): Add ompd-parallel.c. i.e. tab, * space filename and if there is something in the file that is being modified, followed by space ( name ) All this then followed by : and description. > > * Makefile.in: Regenerate. > > * libgompd.map: Add ompd_get_curr_parallel_handle, > > ompd_get_enclosing_parallel_handle, ompd_rel_parallel_handle > > and ompd_parallel_handle_compare symbol versions. We aren't very consistent on the map files, either it should be * libgompd.map (ompd_get_curr_parallel_handle, ompd_get_enclosing_parallel_handle, ompd_rel_parallel_handle, ompd_parallel_handle_compare): Export at OMPD_5.1. or * libgompd.map (OMPD_5.1): Export ompd_get_curr_parallel_handle, ompd_get_enclosing_parallel_handle, ompd_rel_parallel_handle and ompd_parallel_handle_compare. > > * ompd-support.h:() : Add gompd_access (gomp_team_state, team) and > > gompd_access (gomp_team, prev_ts). This is totally wrong. You are modifying the GOMPD_FOREACH_ACCESS macro I think. So you should say * ompd-support.h (GOMPD_FOREACH_ACCESS): Add gompd_access (gomp_team_state, team) and gompd_access (gomp_team, prev_ts). > + return ompd_rc_stale_handle; > + > + CHECK (parallel_handle->ah); > + > + ompd_address_space_context_t *context = parallel_handle->ah->context; > + ompd_address_t symbol_address = parallel_handle->th; > + ompd_address_t temp_symbol_address = {OMPD_SEGMENT_UNSPECIFIED, 0}; > + ompd_word_t temp_offset; > + ompd_rc_t ret; > + > + /* get prev_ts offset. */ > + GET_VALUE (context, NULL, "gompd_access_gomp_team_prev_ts", > + temp_offset, temp_offset, target_sizes.sizeof_long_long, 1, ret, > + temp_symbol_address); Formatting. You can see the two lines are badly indented, each one in a different wrong way. The indenting rule is that the lines after the first one in this case should start aligned below context on the GET_VALUE line, and each 8 spaces should be replaced by a tab. > + > + symbol_address.address += temp_offset; > + > + /* get team offset. */ > + GET_VALUE (context, NULL, "gompd_access_gomp_team_state_team", > + temp_offset, temp_offset, target_sizes.sizeof_long_long, 1, ret, > + temp_symbol_address); Likewise. > + > + symbol_address.address += temp_offset; > + > + ret = callbacks->read_memory (context, NULL, &symbol_address, > + target_sizes.sizeof_pointer, > + &symbol_address.address); > + CHECK_RET (ret); > + ret = callbacks->alloc_memory (sizeof (ompd_parallel_handle_t), > + (void **) (enc_par_handle)); Why the ()s around enc_par_handle? Anyway, seems like an aliasing violation, if the callback stores through void **, then you should just pass it address of a local void * variable and *enc_par_handle = (ompd_parallel_handle_t) var; > +ompd_rc_t > +ompd_rel_parallel_handle (ompd_parallel_handle_t *parallel_handle) > +{ > + if (parallel_handle == NULL) > + return ompd_rc_stale_handle; > + > + ompd_rc_t ret = callbacks->free_memory ((void *) (parallel_handle)); Likewise (both aliasing and the unnecessary ()s. > #define GOMPD_SIZES(gompd_size) \ > gompd_size (gomp_thread) \ > gompd_size (gomp_task_icv) \ > - gompd_size (gomp_task) > + gompd_size (gomp_task) > + Why? Jakub