public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: "Jose E. Marchesi" <jose.marchesi@oracle.com>
To: "Jose E. Marchesi via Gcc-patches" <gcc-patches@gcc.gnu.org>
Cc: Jakub Jelinek <jakub@redhat.com>
Subject: Re: [PATCH V2] Emit funcall external declarations only if actually used.
Date: Wed, 30 Aug 2023 10:12:54 +0200	[thread overview]
Message-ID: <878r9tq77d.fsf@oracle.com> (raw)
In-Reply-To: <20230821180718.20489-1-jose.marchesi@oracle.com> (Jose E. Marchesi via Gcc-patches's message of "Mon, 21 Aug 2023 20:07:18 +0200")


ping

> [Differences from V1:
> - Prototype for call_from_call_insn moved before comment block.
> - Reuse the `call' flag for SYMBOL_REF_LIBCALL.
> - Fallback to check REG_CALL_DECL in non-direct calls.
> - New test to check correct behavior for non-direct calls.]
>
> 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	(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.c-torture/compile/libcall-2.c: Likewise.
> ---
>  gcc/calls.cc                                  |  9 +++---
>  gcc/final.cc                                  | 30 +++++++++++++++++++
>  gcc/rtl.h                                     |  5 ++++
>  .../gcc.c-torture/compile/libcall-2.c         |  8 +++++
>  .../gcc.target/bpf/divmod-libcall-1.c         | 19 ++++++++++++
>  .../gcc.target/bpf/divmod-libcall-2.c         | 16 ++++++++++
>  6 files changed, 83 insertions(+), 4 deletions(-)
>  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/calls.cc b/gcc/calls.cc
> index 1f3a6d5c450..219ea599b16 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
> @@ -4735,7 +4736,7 @@ emit_library_call_value_1 (int retval, rtx orgfun, rtx value,
>  	       valreg,
>  	       old_inhibit_defer_pop + 1, call_fusage, flags, args_so_far);
>  
> -  if (flag_ipa_ra)
> +  if (flag_ipa_ra || SYMBOL_REF_LIBCALL (orgfun))
>      {
>        rtx datum = orgfun;
>        gcc_assert (GET_CODE (datum) == SYMBOL_REF);
> diff --git a/gcc/final.cc b/gcc/final.cc
> index dd3e22547ac..2041e43fdd1 100644
> --- a/gcc/final.cc
> +++ b/gcc/final.cc
> @@ -804,6 +804,8 @@ make_pass_compute_alignments (gcc::context *ctxt)
>  }
>  
>  \f
> +static rtx call_from_call_insn (rtx_call_insn *insn);
> +
>  /* Make a pass over all insns and compute their actual lengths by shortening
>     any branches of variable length if possible.  */
>  
> @@ -850,6 +852,34 @@ 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;
> +
> +          if ((x = call_from_call_insn (dyn_cast <rtx_call_insn *> (insn)))
> +              && (x = XEXP (x, 0))
> +              && MEM_P (x)
> +              && (x = XEXP (x, 0))
> +              && SYMBOL_REF_P (x)
> +              && SYMBOL_REF_LIBCALL (x))
> +            {
> +              /* Direct call.  */
> +              assemble_external_libcall (x);
> +            }
> +          else if ((x = find_reg_note (insn, REG_CALL_DECL, NULL_RTX))
> +                   && (x = XEXP (x, 0)))
> +            {
> +              /* Indirect call with REG_CALL_DECL note.  */
> +              gcc_assert (SYMBOL_REF_P (x));
> +              if (SYMBOL_REF_LIBCALL (x))
> +                assemble_external_libcall (x);
> +            }
> +        }
> +
>        if (INSN_P (insn))
>  	continue;
>  
> diff --git a/gcc/rtl.h b/gcc/rtl.h
> index e1c51156f90..28be708a55f 100644
> --- a/gcc/rtl.h
> +++ b/gcc/rtl.h
> @@ -334,6 +334,7 @@ struct GTY((desc("0"), tag("0"),
>       1 in a CALL_INSN logically equivalent to
>         ECF_LOOPING_CONST_OR_PURE and DECL_LOOPING_CONST_OR_PURE_P.
>       1 in a VALUE is SP_DERIVED_VALUE_P in cselib.cc.
> +     1 in a SYMBOL_REF if it is the target of a libcall.
>       Dumped as "/c" in RTL dumps.  */
>    unsigned int call : 1;
>    /* 1 in a REG, MEM, or CONCAT if the value is set at most once, anywhere.
> @@ -2734,6 +2735,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)->call)
> +
>  /* 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.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;
> +}

  reply	other threads:[~2023-08-30  8:13 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-21 18:07 Jose E. Marchesi
2023-08-30  8:12 ` Jose E. Marchesi [this message]
2023-09-05 13:03   ` Jose E. Marchesi
2023-10-03 10:39   ` Jose E. Marchesi
2023-10-05 22:17     ` Richard Sandiford
2023-10-05 22:37       ` Jeff Law
2023-10-12 11:38       ` Jose E. Marchesi
2023-10-12 12:49         ` Richard Sandiford
2023-10-12 13:54           ` Jose E. Marchesi
2023-10-12 14:05             ` Richard Sandiford

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=878r9tq77d.fsf@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).