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 9CDF83858C50 for ; Tue, 28 Nov 2023 12:04:38 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 9CDF83858C50 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=arm.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 9CDF83858C50 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=217.140.110.172 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1701173080; cv=none; b=HHPT+q1d7sEUf7F43tNtQKzey9P8jbSNz2gYphsyPvAUkFSLld4yAHUJ0Q40mYRUKFLzWOWWRKI4fJXYDKvkA76Glf505+x+SCFMwtl0gXOAer/MRqvVJXlwplqIdWKe+2Tt1XNjJ5cP221pj877OREhHUz1oBcj3VQo0P26bms= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1701173080; c=relaxed/simple; bh=39oxj4Czu7QQgcYxJgA5igVxe7oX7UlFsFP5S6h4NBI=; h=From:To:Subject:Date:Message-ID:MIME-Version; b=HQW+wttn+8UP0kM4bb5ULyNEZHV9u+WIldvX06huOTl7rOKu7zll8RHWWAM16bkWJVh96AUzCA5Yrb6aUpIO2zw7oq9lPx/vTRkmC4QafLjea5WoG2iQ9FY6ljicB5TPR8kbiN2AayiJFMG9wExWqPle+MOJs+8WvU6/jcVCMSM= ARC-Authentication-Results: i=1; server2.sourceware.org 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 10B6CC15; Tue, 28 Nov 2023 04:05:25 -0800 (PST) Received: from localhost (e121540-lin.manchester.arm.com [10.32.110.72]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 989CF3F73F; Tue, 28 Nov 2023 04:04:36 -0800 (PST) From: Richard Sandiford To: "Jose E. Marchesi" Mail-Followup-To: "Jose E. Marchesi" ,gcc-patches@gcc.gnu.org, richard.sandiford@arm.com Cc: gcc-patches@gcc.gnu.org Subject: Re: [PATCH] Emit funcall external declarations only if actually used. References: <20231125094721.18913-1-jose.marchesi@oracle.com> Date: Tue, 28 Nov 2023 12:04:34 +0000 In-Reply-To: <20231125094721.18913-1-jose.marchesi@oracle.com> (Jose E. Marchesi's message of "Sat, 25 Nov 2023 10:47:21 +0100") 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=-21.7 required=5.0 tests=BAYES_00,GIT_PATCH_0,KAM_DMARC_NONE,KAM_DMARC_STATUS,KAM_LAZY_DOMAIN_SECURITY,KAM_SHORT,KAM_STOCKGEN,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 List-Id: "Jose E. Marchesi" writes: > There are many places in GCC where alternative local sequences are > tried in order to determine what is the cheapest or best alternative > to use in the current target. When any of these sequences involve a > libcall, the current implementation of emit_library_call_value_1 > introduce a side-effect consisting on emitting an external declaration > for the funcall (such as __divdi3) which is thus emitted even if the > sequence that does the libcall is not retained. > > This is problematic in targets such as BPF, because the kernel loader > chokes on the spurious symbol __divdi3 and makes the resulting BPF > object unloadable. Note that BPF objects are not linked before being > loaded. > > This patch changes asssemble_external_libcall to defer emitting > declarations of external libcall symbols, by saving the call tree > nodes in a temporary list pending_libcall_symbols and letting > process_pending_assembly_externals to emit them only if they have been > referenced. Solution suggested and sketched by Richard Sandiford. > > Regtested in x86_64-linux-gnu. > Tested with host x86_64-linux-gnu with target bpf-unknown-none. > > gcc/ChangeLog > > * varasm.cc (pending_libcall_symbols): New variable. > (process_pending_assemble_externals): Process > pending_libcall_symbols. > (assemble_external_libcall): Defer emitting external libcall > symbols to process_pending_assemble_externals. > > gcc/testsuite/ChangeLog > > * gcc.target/bpf/divmod-libcall-1.c: New test. > * gcc.target/bpf/divmod-libcall-2.c: Likewise. > * gcc.c-torture/compile/libcall-2.c: Likewise. OK, thanks. Richard > --- > .../gcc.c-torture/compile/libcall-2.c | 8 +++++++ > .../gcc.target/bpf/divmod-libcall-1.c | 19 ++++++++++++++++ > .../gcc.target/bpf/divmod-libcall-2.c | 16 ++++++++++++++ > gcc/varasm.cc | 22 ++++++++++++++++++- > 4 files changed, 64 insertions(+), 1 deletion(-) > create mode 100644 gcc/testsuite/gcc.c-torture/compile/libcall-2.c > create mode 100644 gcc/testsuite/gcc.target/bpf/divmod-libcall-1.c > create mode 100644 gcc/testsuite/gcc.target/bpf/divmod-libcall-2.c > > diff --git a/gcc/testsuite/gcc.c-torture/compile/libcall-2.c b/gcc/testsuite/gcc.c-torture/compile/libcall-2.c > new file mode 100644 > index 00000000000..b33944c83ff > --- /dev/null > +++ b/gcc/testsuite/gcc.c-torture/compile/libcall-2.c > @@ -0,0 +1,8 @@ > +/* Make sure that external refences for libcalls are generated even for > + indirect calls. */ > + > +/* { dg-do compile } */ > +/* { dg-options "-O2 -mcmodel=large" { target x86_64-*-* } } */ > +/* { dg-final { scan-assembler "globl\t__divti3" } } */ > + > +__int128 a, b; void foo () { a = a / b; } > diff --git a/gcc/testsuite/gcc.target/bpf/divmod-libcall-1.c b/gcc/testsuite/gcc.target/bpf/divmod-libcall-1.c > new file mode 100644 > index 00000000000..7481076602a > --- /dev/null > +++ b/gcc/testsuite/gcc.target/bpf/divmod-libcall-1.c > @@ -0,0 +1,19 @@ > +/* This test makes sure that no spurious external symbol declarations are > + emitted for libcalls in tried but eventually not used code sequences. */ > + > +/* { dg-do compile } */ > +/* { dg-options "-O2 -mcpu=v3" } */ > +/* { dg-final { scan-assembler-not "global\t__divdi3" } } */ > +/* { dg-final { scan-assembler-not "global\t__moddi3" } } */ > + > +int > +foo (unsigned int len) > +{ > + return ((unsigned long)len) * 234 / 5; > +} > + > +int > +bar (unsigned int len) > +{ > + return ((unsigned long)len) * 234 % 5; > +} > diff --git a/gcc/testsuite/gcc.target/bpf/divmod-libcall-2.c b/gcc/testsuite/gcc.target/bpf/divmod-libcall-2.c > new file mode 100644 > index 00000000000..792d689395a > --- /dev/null > +++ b/gcc/testsuite/gcc.target/bpf/divmod-libcall-2.c > @@ -0,0 +1,16 @@ > +/* { dg-do compile } */ > +/* { dg-options "-O2 -mcpu=v3" } */ > +/* { dg-final { scan-assembler "global\t__divdi3" } } */ > +/* { dg-final { scan-assembler "global\t__moddi3" } } */ > + > +int > +foo (unsigned int len) > +{ > + return ((long)len) * 234 / 5; > +} > + > +int > +bar (unsigned int len) > +{ > + return ((long)len) * 234 % 5; > +} > diff --git a/gcc/varasm.cc b/gcc/varasm.cc > index 6ae35edc5ae..deb7eab7af9 100644 > --- a/gcc/varasm.cc > +++ b/gcc/varasm.cc > @@ -2461,6 +2461,10 @@ contains_pointers_p (tree type) > it all the way to final. See PR 17982 for further discussion. */ > static GTY(()) tree pending_assemble_externals; > > +/* A similar list of pending libcall symbols. We only want to declare > + symbols that are actually used in the final assembly. */ > +static GTY(()) rtx pending_libcall_symbols; > + > #ifdef ASM_OUTPUT_EXTERNAL > /* Some targets delay some output to final using TARGET_ASM_FILE_END. > As a result, assemble_external can be called after the list of externals > @@ -2520,8 +2524,17 @@ process_pending_assemble_externals (void) > for (list = pending_assemble_externals; list; list = TREE_CHAIN (list)) > assemble_external_real (TREE_VALUE (list)); > > + for (rtx list = pending_libcall_symbols; list; list = XEXP (list, 1)) > + { > + rtx symbol = XEXP (list, 0); > + tree id = get_identifier (XSTR (symbol, 0)); > + if (TREE_SYMBOL_REFERENCED (id)) > + targetm.asm_out.external_libcall (symbol); > + } > + > pending_assemble_externals = 0; > pending_assemble_externals_processed = true; > + pending_libcall_symbols = NULL_RTX; > delete pending_assemble_externals_set; > #endif > } > @@ -2594,8 +2607,15 @@ assemble_external_libcall (rtx fun) > /* Declare library function name external when first used, if nec. */ > if (! SYMBOL_REF_USED (fun)) > { > + gcc_assert (!pending_assemble_externals_processed); > SYMBOL_REF_USED (fun) = 1; > - targetm.asm_out.external_libcall (fun); > + /* Make sure the libcall symbol is in the symtab so any > + reference to it will mark its tree node as referenced, via > + assemble_name_resolve. These are eventually emitted, if > + used, in process_pending_assemble_externals. */ > + get_identifier (XSTR (fun, 0)); > + pending_libcall_symbols = gen_rtx_EXPR_LIST (VOIDmode, fun, > + pending_libcall_symbols); > } > }