[PATCH] Emit funcall external declarations only if actually used.

[PATCH] Emit funcall external declarations only if actually used.

[Jose E. Marchesi]

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.
---
 .../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);
     }
 }
 
-- 
2.30.2

Re: [PATCH] Emit funcall external declarations only if actually used.

[Richard Sandiford]

"Jose E. Marchesi" <jose.marchesi@oracle.com> 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);
>      }
>  }

Re: [PATCH] Emit funcall external declarations only if actually used.

[Jose E. Marchesi]

> "Jose E. Marchesi" <jose.marchesi@oracle.com> 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.

Thank you.
Pushed.

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

Re: [PATCH] Emit funcall external declarations only if actually used.

[Jose E. Marchesi]

>> "Jose E. Marchesi" <jose.marchesi@oracle.com> 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.
>
> Thank you.
> Pushed.

I installed the following fix, since the built got broken in targets
that do not define ASM_OUTPUT_EXTERNAL.

diff --git a/gcc/varasm.cc b/gcc/varasm.cc
index deb7eab7af9..167aea87091 100644
--- a/gcc/varasm.cc
+++ b/gcc/varasm.cc
@@ -2607,7 +2607,9 @@ assemble_external_libcall (rtx fun)
   /* Declare library function name external when first used, if nec.  */
   if (! SYMBOL_REF_USED (fun))
     {
+#ifdef ASM_OUTPUT_EXTERNAL
       gcc_assert (!pending_assemble_externals_processed);
+#endif
       SYMBOL_REF_USED (fun) = 1;
       /* Make sure the libcall symbol is in the symtab so any
          reference to it will mark its tree node as referenced, via
-- 
2.30.2


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

Re: [PATCH] Emit funcall external declarations only if actually used.

[Jakub Jelinek]

On Fri, Aug 18, 2023 at 06:31:10PM +0200, Jose E. Marchesi wrote:
> > This won't work if target can't use a direct call instruction.
> > Consider
> > __int128 a, b; void foo () { a = a / b; }
> > on x86_64-linux.  With just -O2, the above works fine, with
> > -O2 -mcmodel=large it will not, the call is indirect, but at least one has
> > REG_CALL_DECL note that could be used as fallback to the above.
> > And with -O0 -mcmodel=large because flag_ipa_ra is false REG_CALL_DECL isn't
> > emitted at all.
> > So, perhaps you could emit the REG_CALL_DECL note even if !flag_ipa_ra
> > when SYMBOL_REF_LIBCALL is set?
> 
> Hmm something like this?

Yes.

	Jakub

Re: [PATCH] Emit funcall external declarations only if actually used.

[Jose E. Marchesi]

Hi Jakub.
Thanks for the review.

> On Fri, Aug 18, 2023 at 03:53:51PM +0200, Jose E. Marchesi via Gcc-patches wrote:
>> --- 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);
>> +
>
> I'd say the forward declaration should go before the function comment, so
> that it is clear the function comment talks about shorten_branches.

Will do.

>
>>  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)
>
> When all conditions don't fit on one line, each && condition should be on
> its own line.

Will fix.

>
>> +              && SYMBOL_REF_P (XEXP (x, 0))
>> +              && SYMBOL_REF_LIBCALL (XEXP (x, 0)))
>> +            assemble_external_libcall (XEXP (x, 0));
>> +        }
>
> This won't work if target can't use a direct call instruction.
> Consider
> __int128 a, b; void foo () { a = a / b; }
> on x86_64-linux.  With just -O2, the above works fine, with
> -O2 -mcmodel=large it will not, the call is indirect, but at least one has
> REG_CALL_DECL note that could be used as fallback to the above.
> And with -O0 -mcmodel=large because flag_ipa_ra is false REG_CALL_DECL isn't
> emitted at all.
> So, perhaps you could emit the REG_CALL_DECL note even if !flag_ipa_ra
> when SYMBOL_REF_LIBCALL is set?

Hmm something like this?

(I am aware that as things stand in emit_library_call_value_1 that
 conditional will be always true, but I think it is good to keep the
 conditional as documentation and in case emit_library_call_value_1
 changes in the future.  Note also that `fun' is known to be `orgfun'
 when the bit it set.  That may change later as per
 prepare_call_address.)

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..53f5d890809 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,19 @@ 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 or 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));
+
+          if ((x = XEXP (x, 0)) && MEM_P (x) && SYMBOL_REF_P (XEXP (x, 0))
+              || (x = find_reg_note (insn, REG_CALL_DECL, NULL_RTX)))
+            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;
>
> This is wrong.  struct rtx_def is carefully designed such that
> it has 16 + 8 + 8 bits before the union is 32-bit and then
> flexible array member which sometimes needs 64-bit alignment.
> So, the above change would grow all RTX by 64 bits.
> I think jump, call and in_struct are unused on SYMBOL_REF, so just
> document the new meaning of the chosen bit for SYMBOL_REF.

Ok good to know.  I will use `call' 8-)

Re: [PATCH] Emit funcall external declarations only if actually used.

[Jakub Jelinek]

On Fri, Aug 18, 2023 at 03:53:51PM +0200, Jose E. Marchesi via Gcc-patches wrote:
> --- 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);
> +

I'd say the forward declaration should go before the function comment, so
that it is clear the function comment talks about shorten_branches.

>  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)

When all conditions don't fit on one line, each && condition should be on
its own line.

> +              && SYMBOL_REF_P (XEXP (x, 0))
> +              && SYMBOL_REF_LIBCALL (XEXP (x, 0)))
> +            assemble_external_libcall (XEXP (x, 0));
> +        }

This won't work if target can't use a direct call instruction.
Consider
__int128 a, b; void foo () { a = a / b; }
on x86_64-linux.  With just -O2, the above works fine, with
-O2 -mcmodel=large it will not, the call is indirect, but at least one has
REG_CALL_DECL note that could be used as fallback to the above.
And with -O0 -mcmodel=large because flag_ipa_ra is false REG_CALL_DECL isn't
emitted at all.
So, perhaps you could emit the REG_CALL_DECL note even if !flag_ipa_ra
when SYMBOL_REF_LIBCALL is set?

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

This is wrong.  struct rtx_def is carefully designed such that
it has 16 + 8 + 8 bits before the union is 32-bit and then
flexible array member which sometimes needs 64-bit alignment.
So, the above change would grow all RTX by 64 bits.
I think jump, call and in_struct are unused on SYMBOL_REF, so just
document the new meaning of the chosen bit for SYMBOL_REF.

> @@ -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)

And change is_libcall to the selected bit-field here.

	Jakub

[PATCH] Emit funcall external declarations only if actually used.

[Jose E. Marchesi]

[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