From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by sourceware.org (Postfix) with ESMTP id 1880B3858C74; Fri, 2 Sep 2022 12:36:26 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 1880B3858C74 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=arm.com Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id DF6EED6E; Fri, 2 Sep 2022 05:36:31 -0700 (PDT) Received: from localhost (e121540-lin.manchester.arm.com [10.32.98.62]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 20B973F71A; Fri, 2 Sep 2022 05:36:25 -0700 (PDT) From: Richard Sandiford To: Keith Packard via Gcc-patches Mail-Followup-To: Keith Packard via Gcc-patches ,gcc@gcc.gnu.org, Keith Packard , richard.sandiford@arm.com Cc: gcc@gcc.gnu.org, Keith Packard Subject: Re: [PATCH 0/3] picolibc: Add picolibc linking help References: <20220824180426.820576-1-keithp@keithp.com> Date: Fri, 02 Sep 2022 13:36:23 +0100 In-Reply-To: <20220824180426.820576-1-keithp@keithp.com> (Keith Packard via Gcc-patches's message of "Wed, 24 Aug 2022 11:04:23 -0700") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-Spam-Status: No, score=-43.4 required=5.0 tests=BAYES_00,KAM_DMARC_NONE,KAM_DMARC_STATUS,KAM_LAZY_DOMAIN_SECURITY,SPF_HELO_NONE,SPF_NONE,TXREP,T_SCC_BODY_TEXT_LINE autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Keith Packard via Gcc-patches writes: > Picolibc is a C library for embedded systems based on code from newlib > and avr libc. To connect some system-dependent picolibc functions > (like stdio) to an underlying platform, the platform may provide an OS > library. > > This OS library must follow the C library in the link command line. In > current picolibc, that is done by providing an alternate .specs file > which can rewrite the *lib spec to insert the OS library in the right > spot. > > This patch series adds the ability to specify the OS library on the > gcc command line when GCC is configured to us picolibc as the default > C library, and then hooks that up for arm, nds32, riscv and sh targets. Not really my area, but the approach LGTM FWIW. Main question/points: - In: +case "${with_default_libc}" in +glibc) + default_libc=LIBC_GLIBC + ;; should there be a default case that raises an error for unrecognised libcs? Command-line checking for configure isn't very tight, but we do raise similar errors for things like invalid --enable-threads values. - I'm not sure either way about adding LIBC_NEWLIB. On the one hand it makes sense for completeness, but on the other it's write-only. Adding it means that --with-default-libc=newlib toolchains have a different macro configuration from a default toolchain even in cases where newlib is the default. On balance I think it would be better to accept --with-default-libc=newlib but set default_libc to the empty string. - Should we raise an error for toolchains that don't support the given C library? It feels like we should, but I realise that could be difficult to do. - Very minor, but in lines like: +#if defined(DEFAULT_LIBC) && defined(LIBC_PICOLIBC) && DEFAULT_LIBC == LIBC_PICOLIBC is LIBC_PICOLIB ever undefined? It looks like config.gcc provides an unconditional definition. If it is always defined then: #if DEFAULT_LIBC == LIBC_PICOLIB would be clearer. Thanks, Richard > > Keith Packard (3): > Allow default libc to be specified to configure > Add newlib and picolibc as default C library choices > Add '--oslib=' option when default C library is picolibc > > gcc/config.gcc | 56 ++++++++++++++++++++++++++++++++------- > gcc/config/arm/elf.h | 5 ++++ > gcc/config/nds32/elf.h | 4 +++ > gcc/config/picolibc.opt | 26 ++++++++++++++++++ > gcc/config/riscv/elf.h | 4 +++ > gcc/config/sh/embed-elf.h | 5 ++++ > gcc/configure.ac | 4 +++ > 7 files changed, 95 insertions(+), 9 deletions(-) > create mode 100644 gcc/config/picolibc.opt