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 [63.128.21.124]) by sourceware.org (Postfix) with ESMTP id 34C68385E83A for ; Tue, 6 Oct 2020 12:20:28 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 34C68385E83A Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-452-3GLqY5XsNxidYzHdLlED-g-1; Tue, 06 Oct 2020 08:20:25 -0400 X-MC-Unique: 3GLqY5XsNxidYzHdLlED-g-1 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 3129118CBC40; Tue, 6 Oct 2020 12:20:24 +0000 (UTC) Received: from oldenburg2.str.redhat.com (ovpn-113-154.ams2.redhat.com [10.36.113.154]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 78AE119D7D; Tue, 6 Oct 2020 12:20:23 +0000 (UTC) From: Florian Weimer To: "Paul A. Clarke" Cc: libc-alpha@sourceware.org Subject: Re: [PATCH 18/28] powerpc64le: Add glibc-hwcaps support References: <01faff4932d02c7e3224b50a1cdb5956354b1fc2.1601569371.git.fweimer@redhat.com> <20201001185639.GA132840@li-24c3614c-2adc-11b2-a85c-85f334518bdb.ibm.com> <87ft6txavf.fsf@oldenburg2.str.redhat.com> <20201005191500.GA69013@li-24c3614c-2adc-11b2-a85c-85f334518bdb.ibm.com> Date: Tue, 06 Oct 2020 14:20:21 +0200 In-Reply-To: <20201005191500.GA69013@li-24c3614c-2adc-11b2-a85c-85f334518bdb.ibm.com> (Paul A. Clarke's message of "Mon, 5 Oct 2020 14:15:00 -0500") Message-ID: <87y2kjsfzu.fsf@oldenburg2.str.redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain X-Spam-Status: No, score=-11.8 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H5, RCVD_IN_MSPIKE_WL, 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 X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 06 Oct 2020 12:20:46 -0000 * Paul A. Clarke: > On Mon, Oct 05, 2020 at 11:47:32AM +0200, Florian Weimer wrote: >> * Paul A. Clarke: >> >> diff --git a/sysdeps/powerpc/powerpc64/le/dl-hwcaps-subdirs.c b/sysdeps/powerpc/powerpc64/le/dl-hwcaps-subdirs.c >> >> new file mode 100644 >> >> index 0000000000..496daf0fa0 >> >> --- /dev/null >> >> +++ b/sysdeps/powerpc/powerpc64/le/dl-hwcaps-subdirs.c > [...snip...] >> >> +_dl_hwcaps_subdirs_active (void) >> >> +{ >> >> + if (GLRO (dl_hwcap2) & PPC_FEATURE2_ARCH_3_1) >> >> + return 3; >> >> + >> >> + if (GLRO (dl_hwcap2) & PPC_FEATURE2_ARCH_3_00) >> >> + return 1; Meh, the second test should do: return 2; 8-( Maybe we should write it this way: if ((GLRO (dl_hwcap2) & PPC_FEATURE2_ARCH_3_00)) == 0) return 0; /* No subdirectories active. */ if ((GLRO (dl_hwcap2) & PPC_FEATURE2_ARCH_3_1)) == 0) return 2; /* Only the second directory (power9) is active. */ return 3; /* Both directories (power10, power9) are active. */ > ...which still creates magic numbers and really doesn't explain what the > result represents. > The entire API is documented in a 2-line comment before count_hwcaps(). > For someone to update the implementation for a new hwcap, which will > need to be done fairly often, seems to me to be a bit challenging for > the uninitiated (like me ;-). The comments are in elf/dl-hwcaps.h: /* Colon-separated string of glibc-hwcaps subdirectories, without the "glibc-hwcaps/" prefix. The most preferred subdirectory needs to be listed first. */ extern const char _dl_hwcaps_subdirs[] attribute_hidden; /* Returns a bitmap of active subdirectories in _dl_hwcaps_subdirs. Bit 0 (the LSB) corresponds to the first substring in _dl_hwcaps_subdirs, bit 1 to the second substring, and so on. There is no direct correspondence between HWCAP bitmasks and this bitmask. */ int32_t _dl_hwcaps_subdirs_active (void) attribute_hidden; >> > Perhaps something like (not tested): >> > -- >> > const char * const _dl_hwcaps_subdirs[] = { >> > #define _DL_HWCAPS_SUBDIR_POWER10_BIT 0x2 /* or 1 to preserve same order. */ >> > "power10", >> > #define _DL_HWCAPS_SUBDIR_POWER9_BIT 0x1 /* or 2. */ >> > "power9" >> > }; >> > >> > int32_t >> > _dl_hwcaps_subdirs_active (void) >> > { >> > int32_t result = 0; >> > >> > if (GLRO (dl_hwcap2) & PPC_FEATURE2_ARCH_3_1) >> > result |= _DL_HWCAPS_SUBDIR_POWER10_BIT; >> > >> > if (GLRO (dl_hwcap2) & PPC_FEATURE2_ARCH_3_00) >> > result |= _DL_HWCAPS_SUBDIR_POWER9_BIT; >> > >> > return result; >> > } >> > -- >> > >> > Of course, that would require changes to the code that parses >> > _dl_hwcaps_subdirs. >> >> I chose the current approach to avoid relocations and memory allocations >> for processing hwcaps settings (e.g. from the ld.so command line). > > Does the "array of strings" approach introduce new/additional relocations > and memory allocations? It would seem to avoid the need for the splitting > code, at least. The string pointers need runtime relocations (although they probably do not matter in the grand scheme of things). We still need the splitting code for the ld.so command line argument. Thanks, Florian -- Red Hat GmbH, https://de.redhat.com/ , Registered seat: Grasbrunn, Commercial register: Amtsgericht Muenchen, HRB 153243, Managing Directors: Charles Cachera, Brian Klemm, Laurie Krebs, Michael O'Neill