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: Tue, 03 Oct 2023 12:39:04 +0200	[thread overview]
Message-ID: <87ttr8gfcn.fsf@oracle.com> (raw)
In-Reply-To: <878r9tq77d.fsf@oracle.com> (Jose E. Marchesi's message of "Wed, 30 Aug 2023 10:12:54 +0200")


ping

> 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;
>> +}

  parent reply	other threads:[~2023-10-03 10:39 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
2023-09-05 13:03   ` Jose E. Marchesi
2023-10-03 10:39   ` Jose E. Marchesi [this message]
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=87ttr8gfcn.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).