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 6343F3857B93 for ; Mon, 30 May 2022 16:10:31 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 6343F3857B93 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-303-_PJZKfhVPRK6sHLXln1-5w-1; Mon, 30 May 2022 12:10:27 -0400 X-MC-Unique: _PJZKfhVPRK6sHLXln1-5w-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.rdu2.redhat.com [10.11.54.1]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 42AE2804189; Mon, 30 May 2022 16:10:27 +0000 (UTC) Received: from tucnak.zalov.cz (unknown [10.33.36.77]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 02A0740CFD0B; Mon, 30 May 2022 16:10:26 +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 24UGAOPw3665028 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NOT); Mon, 30 May 2022 18:10:24 +0200 Received: (from jakub@localhost) by tucnak.zalov.cz (8.17.1/8.17.1/Submit) id 24UGANtK3665027; Mon, 30 May 2022 18:10:23 +0200 Date: Mon, 30 May 2022 18:10:23 +0200 From: Jakub Jelinek To: Mohamed Atef Cc: gcc-patches@gcc.gnu.org Subject: Re: [PATCH] libgompd: Add ompd_get/rel_display_control_vars Message-ID: Reply-To: Jakub Jelinek References: MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.84 on 10.11.54.1 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=-9.8 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, 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, 30 May 2022 16:10:33 -0000 On Fri, May 27, 2022 at 02:04:11AM +0200, Mohamed Atef wrote: > libgomp/ChangeLog > > 2022-05-27 Mohamed Atef > > * libgompd.map (ompd_get_display_control_vars, > ompd_rel_display_control_vars): New global symbol versions. > * env.c: (gompd_buffer, gompd_env_buff_size): New Variables. > (dump_icvs): New function. > (initialize_env): call dump_icvs. > * ompd-icv.c: (ompd_get_display_control_vars): New function. > (ompd_rel_display_control_vars): New function. I don't like this solution, I know LLVM libomp does it this way, but that adds complexity to the libgomp library to make it easier for libgompd, and even worse further bloats .data section of every OpenMP program even when debugging isn't enabled (and not just a tiny bit, but by sizeof (char *) + sizeof (unsigned long) + 500 bytes, which is significant). This really should be done on the libgompd side, instead of copying the gompd_buffer you've added, it should instead copy gomp_global_icv and whatever else is necessary to print it, then do that in libgompd. > diff --git a/libgomp/env.c b/libgomp/env.c > index 243c6267ef9..173c9271303 100644 > --- a/libgomp/env.c > +++ b/libgomp/env.c > @@ -91,6 +91,8 @@ unsigned long gomp_places_list_len; > uintptr_t gomp_def_allocator = omp_default_mem_alloc; > int gomp_debug_var; > int gompd_enabled; > +char *gompd_buffer; > +unsigned long gompd_env_buff_size; > unsigned int gomp_num_teams_var; > int gomp_nteams_var; > int gomp_teams_thread_limit_var; > @@ -1453,6 +1455,187 @@ omp_display_env (int verbose) > } > ialias (omp_display_env) > > +/* This function dumps all global ICVs into a buffer > + in the form "icv-name=icv-value\n", so that OMPD can read the > + buffer and display all icvs. */ > + > +static void > +dump_icvs (void) > +{ > + static char temp_buffer[500]; > + char temp_num_str[20]; > + strcat (temp_buffer, "OMP_DYNAMIC="); > + strcat (temp_buffer, gomp_global_icv.dyn_var ? "TRUE\n" : "FALSE\n"); > + strcat (temp_buffer, "OMP_NESTED="); > + strcat (temp_buffer, > + gomp_global_icv.max_active_levels_var > 1 ? "TRUE\n" : "FALSE\n"); > + strcat (temp_buffer, "OMP_NUM_THREADS="); > + sprintf (temp_num_str, "%lu\n", gomp_global_icv.nthreads_var); > + strcat (temp_buffer, temp_num_str); > + strcat (temp_buffer, "OMP_SCHEDULE="); This is terribly inefficient even when done that way on the libgompd side. You really don't want every further strcat to walk all the already previously stored bytes to find the end, and you don't want to hardcode the size of the buffer, see https://www.gnu.org/prep/standards/standards.html#Writing-Robust-Programs So, the buffer should be dynamically allocated, and you should at all times know how many bytes have been already stored. One possibility is to call a printing function twice, once with say NULL as the buffer which will be a mode in which it just computes the length of each addition and just returns the total length, then allocate and then pass non-NULL buffer into which it will actually store it. Or you could allocate keep some extra bytes in the buffer (longer than say longest name of an env variable name, = char, largest representable long long number, \n char plus a few bytes extra) and keep checking before adding any var that is guaranteed to fit into those extra bytes whether there are at least those number of bytes left in the buffer and reallocate otherwise. But for vars that can have arbitrarily long results (e.g. vars with various lists), such checking needs to be done before adding any partial result that can fit). Jakub