public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: "Jose E. Marchesi" <jose.marchesi@oracle.com>
To: gcc-patches@gcc.gnu.org
Cc: Jakub Jelinek <jakub@redhat.com>
Subject: [PATCH] Emit funcall external declarations only if actually used.
Date: Fri, 18 Aug 2023 15:53:51 +0200	[thread overview]
Message-ID: <20230818135351.9177-1-jose.marchesi@oracle.com> (raw)

[Previous thread:
 https://gcc.gnu.org/pipermail/gcc-patches/2022-December/608162.html]

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 emit_library_call_value_1 to mark the target
SYMBOL_REF as a libcall.  Then, the emission of the external
declaration is done in the first loop of final.cc:shorten_branches.
This happens only if the corresponding sequence has been kept.

Regtested in x86_64-linux-gnu.
Tested with host x86_64-linux-gnu with target bpf-unknown-none.

gcc/ChangeLog

	* rtl.h: New flag is_libcall.
	(SYMBOL_REF_LIBCALL): Define.
	* calls.cc (emit_library_call_value_1): Do not emit external
	libcall declaration here.
	* final.cc (shorten_branches): Do it here.

gcc/testsuite/ChangeLog

	* gcc.target/bpf/divmod-libcall-1.c: New test.
	* gcc.target/bpf/divmod-libcall-2.c: Likewise.
---
 gcc/calls.cc                                  |  7 ++++---
 gcc/final.cc                                  | 16 ++++++++++++++++
 gcc/rtl.h                                     |  6 ++++++
 .../gcc.target/bpf/divmod-libcall-1.c         | 19 +++++++++++++++++++
 .../gcc.target/bpf/divmod-libcall-2.c         | 16 ++++++++++++++++
 5 files changed, 61 insertions(+), 3 deletions(-)
 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/calls.cc b/gcc/calls.cc
index 1f3a6d5c450..e0ddda42442 100644
--- a/gcc/calls.cc
+++ b/gcc/calls.cc
@@ -4388,9 +4388,10 @@ emit_library_call_value_1 (int retval, rtx orgfun, rtx value,
 	|| argvec[i].partial != 0)
       update_stack_alignment_for_call (&argvec[i].locate);
 
-  /* If this machine requires an external definition for library
-     functions, write one out.  */
-  assemble_external_libcall (fun);
+  /* Mark the emitted target as a libcall.  This will be used by final
+     in order to emit an external symbol declaration if the libcall is
+     ever used.  */
+  SYMBOL_REF_LIBCALL (fun) = 1;
 
   original_args_size = args_size;
   args_size.constant = (aligned_upper_bound (args_size.constant
diff --git a/gcc/final.cc b/gcc/final.cc
index dd3e22547ac..80c112b91f7 100644
--- a/gcc/final.cc
+++ b/gcc/final.cc
@@ -815,6 +815,8 @@ make_pass_compute_alignments (gcc::context *ctxt)
    reorg.cc, since the branch splitting exposes new instructions with delay
    slots.  */
 
+static rtx call_from_call_insn (rtx_call_insn *insn);
+
 void
 shorten_branches (rtx_insn *first)
 {
@@ -850,6 +852,20 @@ shorten_branches (rtx_insn *first)
   for (insn = get_insns (), i = 1; insn; insn = NEXT_INSN (insn))
     {
       INSN_SHUID (insn) = i++;
+
+      /* If this is a `call' instruction implementing a libcall,
+         and this machine requires an external definition for library
+         functions, write one out.  */
+      if (CALL_P (insn))
+        {
+          rtx x = call_from_call_insn (dyn_cast <rtx_call_insn *> (insn));
+          x = XEXP (x, 0);
+          if (x && MEM_P (x)
+              && SYMBOL_REF_P (XEXP (x, 0))
+              && SYMBOL_REF_LIBCALL (XEXP (x, 0)))
+            assemble_external_libcall (XEXP (x, 0));
+        }
+
       if (INSN_P (insn))
 	continue;
 
diff --git a/gcc/rtl.h b/gcc/rtl.h
index e1c51156f90..945e3267a34 100644
--- a/gcc/rtl.h
+++ b/gcc/rtl.h
@@ -402,6 +402,8 @@ struct GTY((desc("0"), tag("0"),
      1 in a VALUE or DEBUG_EXPR is NO_LOC_P in var-tracking.cc.
      Dumped as "/i" in RTL dumps.  */
   unsigned return_val : 1;
+  /* 1 in a SYMBOL_REF if it is the target of a libcall.  */
+  unsigned is_libcall : 1;
 
   union {
     /* The final union field is aligned to 64 bits on LP64 hosts,
@@ -2734,6 +2736,10 @@ do {								        \
 #define SYMBOL_REF_USED(RTX)						\
   (RTL_FLAG_CHECK1 ("SYMBOL_REF_USED", (RTX), SYMBOL_REF)->used)
 
+/* 1 if RTX is a symbol_ref that represents a libcall target.  */
+#define SYMBOL_REF_LIBCALL(RTX)                                         \
+  (RTL_FLAG_CHECK1 ("SYMBOL_REF_LIBCALL", (RTX), SYMBOL_REF)->is_libcall)
+
 /* 1 if RTX is a symbol_ref for a weak symbol.  */
 #define SYMBOL_REF_WEAK(RTX)						\
   (RTL_FLAG_CHECK1 ("SYMBOL_REF_WEAK", (RTX), SYMBOL_REF)->return_val)
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..f769c7a6deb
--- /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 "__divdi3" } } */
+/* { dg-final { scan-assembler-not "__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..c6e86391a7b
--- /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 "__divdi3" } } */
+/* { dg-final { scan-assembler "__moddi3" } } */
+
+int
+foo (unsigned int len)
+{
+  return ((long)len) * 234 / 5;
+}
+
+int
+bar (unsigned int len)
+{
+  return ((long)len) * 234 % 5;
+}
-- 
2.30.2


             reply	other threads:[~2023-08-18 13:54 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-18 13:53 Jose E. Marchesi [this message]
2023-08-18 14:19 ` Jakub Jelinek
2023-08-18 16:31   ` Jose E. Marchesi
2023-08-18 16:40     ` Jakub Jelinek
2023-11-25  9:47 Jose E. Marchesi
2023-11-28 12:04 ` Richard Sandiford
2023-11-28 15:23   ` Jose E. Marchesi
2023-11-29  5:40     ` Jose E. Marchesi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230818135351.9177-1-jose.marchesi@oracle.com \
    --to=jose.marchesi@oracle.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jakub@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).