diff mbox series

Darwin, crts: Provide scalb and significand as a crt [PR107631]

Message ID 20221230102046.8287-1-iain@sandoe.co.uk
State New
Headers
Series Darwin, crts: Provide scalb and significand as a crt [PR107631] |

Commit Message

Iain Sandoe Dec. 30, 2022, 10:20 a.m. UTC 
  This patch is providing functions used by the modula-2 implementation.

At present, I've used a crt rather than adding symbols to libgcc, since
it is not clear if the modula-2 might alter the use of scalb to scalbn
(although that will not solve the missing significand* symbols).

I plan to apply the patch early next week (it is Darwin-specific) unless
there are any comments on the implementation.

tested on powerpc/i688-darwin9 .. x86_64-darwin10,17,21, the prototype
aarch64-darwin branch on darwin21 and x86_64-linux-gnu.

thanks
Iain

--- 8< ---

The Darwin libc does not provide the obsolete scalb() functions, nor does
it supply the glibc significand() functions.

These are used via builtins that resolve to libcalls by modula-2 which leads
to many link fails on all Darwin versions.

We provide them here as a crt implemented as a convenience lib which means
that they will only be linked when required.

Signed-off-by: Iain Sandoe <iain@sandoe.co.uk>

gcc/ChangeLog:

	* config/darwin.h (LINK_COMMAND_SPEC_A): Add link for float extra
	crt.

libgcc/ChangeLog:

	* config.host (*-*_darwin*): Add libfltext.a.
	* config/t-darwin: Build libfltext.a.
	* config/darwin-scalb.c: New file.
	* config/darwin-significand.c: New file.

Signed-off-by: Iain Sandoe <iain@sandoe.co.uk>
---
 gcc/config/darwin.h                |  1 +
 libgcc/config.host                 |  2 +-
 libgcc/config/darwin-scalb.c       | 58 ++++++++++++++++++++++++++++++
 libgcc/config/darwin-significand.c | 39 ++++++++++++++++++++
 libgcc/config/t-darwin             | 10 ++++++
 5 files changed, 109 insertions(+), 1 deletion(-)
 create mode 100644 libgcc/config/darwin-scalb.c
 create mode 100644 libgcc/config/darwin-significand.c

Comments

Joseph Myers Dec. 30, 2022, 10:26 p.m. UTC | #1 
On Fri, 30 Dec 2022, Iain Sandoe via Gcc-patches wrote:

> This patch is providing functions used by the modula-2 implementation.
> 
> At present, I've used a crt rather than adding symbols to libgcc, since
> it is not clear if the modula-2 might alter the use of scalb to scalbn
> (although that will not solve the missing significand* symbols).
> 
> I plan to apply the patch early next week (it is Darwin-specific) unless
> there are any comments on the implementation.

I think it would be better to change Modula-2 to avoid using these 
obsolescent functions, rather than providing them in GCC.  (But if 
provided, the libgcc runtime license exception should be used.)
Iain Sandoe Dec. 31, 2022, 12:12 p.m. UTC | #2 
Hi Joseph,

> On 30 Dec 2022, at 22:26, Joseph Myers <joseph@codesourcery.com> wrote:
> 
> On Fri, 30 Dec 2022, Iain Sandoe via Gcc-patches wrote:
> 
>> This patch is providing functions used by the modula-2 implementation.
>> 
>> At present, I've used a crt rather than adding symbols to libgcc, since
>> it is not clear if the modula-2 might alter the use of scalb to scalbn
>> (although that will not solve the missing significand* symbols).
>> 
>> I plan to apply the patch early next week (it is Darwin-specific) unless
>> there are any comments on the implementation.
> 
> I think it would be better to change Modula-2 to avoid using these 
> obsolescent functions, rather than providing them in GCC.

It seems that the Modula-2 use has drawn my attention to a pre-existing issue.

builtins.def unconditionally defines these builtins to be DEF_EXT_LIB_BUILTIN
which expands to the libcall, this is currently hard-wired to FALLBACK_P = true.

but, AFAIU the builtins.def descriptions:

 FALLBACK_P should be false if the libc (or libm, I suppose, if that’s different)
 does not have the function, perhaps that’s an underlying bug or at least an
 oversight?

 (or, of course, I misunderstood the intent of that param)

>  (But if provided, the libgcc runtime license exception should be used.)

thanks, I’d missed this.

- at present, it seems that this crt might be the least invasive solution (since 
‘significand*()’ are not obsolete AFAIU, we still need to provide those implementations,
regardless of any subsitution of scalbn*() in Modula-2).

thanks
Iain

> 
> -- 
> Joseph S. Myers
> joseph@codesourcery.com
Joseph Myers Jan. 3, 2023, 6:15 p.m. UTC | #3 
On Sat, 31 Dec 2022, Iain Sandoe wrote:

> builtins.def unconditionally defines these builtins to be DEF_EXT_LIB_BUILTIN
> which expands to the libcall, this is currently hard-wired to FALLBACK_P = true.
> 
> but, AFAIU the builtins.def descriptions:
> 
>  FALLBACK_P should be false if the libc (or libm, I suppose, if that’s different)
>  does not have the function, perhaps that’s an underlying bug or at least an
>  oversight?
> 
>  (or, of course, I misunderstood the intent of that param)

FALLBACK_P true means that it's the user's responsibility, if calling 
__builtin_X, to make sure the library function X is also available in 
cases where the call doesn't get expected inline - that is, that the API 
for that __builtin_X function is that it may call an underlying library 
function X, which is expected to exist and have a compatible interface.

Information about whether a function is present in libc / libm is 
generally only relevant when __builtin_X might expand to call Y instead of 
X; then GCC needs to know whether Y is available.

> - at present, it seems that this crt might be the least invasive 
> solution (since ‘significand*()’ are not obsolete AFAIU, we still need 
> to provide those implementations, regardless of any subsitution of 
> scalbn*() in Modula-2).

The significand functions can be considered obsolete and were never in any 
standard (thus glibc does not provide a version for _Float128, for 
example).
Iain Sandoe Jan. 3, 2023, 8:07 p.m. UTC | #4 
Thanks Joseph,

> On 3 Jan 2023, at 18:15, Joseph Myers <joseph@codesourcery.com> wrote:
> 
> On Sat, 31 Dec 2022, Iain Sandoe wrote:
> 
>> builtins.def unconditionally defines these builtins to be DEF_EXT_LIB_BUILTIN
>> which expands to the libcall, this is currently hard-wired to FALLBACK_P = true.
>> 
>> but, AFAIU the builtins.def descriptions:
>> 
>> FALLBACK_P should be false if the libc (or libm, I suppose, if that’s different)
>> does not have the function, perhaps that’s an underlying bug or at least an
>> oversight?
>> 
>> (or, of course, I misunderstood the intent of that param)
> 
> FALLBACK_P true means that it's the user's responsibility, if calling 
> __builtin_X, to make sure the library function X is also available in 
> cases where the call doesn't get expected inline - that is, that the API 
> for that __builtin_X function is that it may call an underlying library 
> function X, which is expected to exist and have a compatible interface.
> 
> Information about whether a function is present in libc / libm is 
> generally only relevant when __builtin_X might expand to call Y instead of 
> X; then GCC needs to know whether Y is available.

Ah I had misunderstood the param.

Since the mechanism used by Modula-2 ‘forwards’ the builins by using them,
that means that it always produces the libcalls which results in link errors when
the m2 libraries are used (i.e. the user is not in control of the use/non-use).

>> - at present, it seems that this crt might be the least invasive 
>> solution (since ‘significand*()’ are not obsolete AFAIU, we still need 
>> to provide those implementations, regardless of any subsitution of 
>> scalbn*() in Modula-2).
> 
> The significand functions can be considered obsolete and were never in any 
> standard (thus glibc does not provide a version for _Float128, for 
> example).

So,  it seems that either:

 1. Modula-2 should not forward the builtins unless the target supports them,
    either by expansion or the relevant lib functions.  So that would need some
   configury and conditional build code.

 2. Preferrably it should not forward the obsolete/obsolescent cases:
       scalb*()
       significand*()

Additionally, if there is an actual use in the Modula-2 runtime (as opposed to
forwarding the functionality to the end user), then it should implement that avoiding
these obsolete functions.

I will withdraw my Darwin patch, and discuss with Gaius how to resolve this in
Modula-2.

thanks
Iain


> 
> -- 
> Joseph S. Myers
> joseph@codesourcery.com
Joseph Myers Jan. 3, 2023, 9:34 p.m. UTC | #5 
On Tue, 3 Jan 2023, Iain Sandoe wrote:

>  1. Modula-2 should not forward the builtins unless the target supports them,
>     either by expansion or the relevant lib functions.  So that would need some
>    configury and conditional build code.

Note that such configure tests could only readily be in the library 
configure scripts, not in the compiler configure scripts - you can't do 
target compile or link tests in host-side configure.  (To a limited extent 
it's possible to grep target headers in host-side configure, though not to 
actually preprocess them since the compiler required for such 
preprocessing doesn't exist at that point.)
diff mbox series

Patch

diff --git a/gcc/config/darwin.h b/gcc/config/darwin.h
index d1b4f277c2e..87b45b3d9d1 100644
--- a/gcc/config/darwin.h
+++ b/gcc/config/darwin.h
@@ -370,6 +370,7 @@  extern GTY(()) int darwin_ms_struct;
       %{%:sanitize(undefined): -lubsan } \
       %(link_ssp) \
       %:version-compare(>< 10.6 10.7 mmacosx-version-min= -ld10-uwfef) \
+      -lfltext \
       %(link_gcc_c_sequence) \
       %{!nodefaultexport:%{dylib|dynamiclib|bundle: \
 	%:version-compare(>= 10.11 asm_macosx_version_min= -U) \
diff --git a/libgcc/config.host b/libgcc/config.host
index d2087654c40..5ccce2726bc 100644
--- a/libgcc/config.host
+++ b/libgcc/config.host
@@ -233,7 +233,7 @@  case ${host} in
       ;;
   esac
   tmake_file="$tmake_file t-slibgcc-darwin"
-  extra_parts="crt3.o libd10-uwfef.a crttms.o crttme.o libemutls_w.a"
+  extra_parts="crt3.o libd10-uwfef.a crttms.o crttme.o libemutls_w.a libfltext.a"
   ;;
 *-*-dragonfly*)
   tmake_file="$tmake_file t-crtstuff-pic t-libgcc-pic t-eh-dw2-dip"
diff --git a/libgcc/config/darwin-scalb.c b/libgcc/config/darwin-scalb.c
new file mode 100644
index 00000000000..d5d653c56bf
--- /dev/null
+++ b/libgcc/config/darwin-scalb.c
@@ -0,0 +1,58 @@ 
+/* Implementation of the scalb functions for Darwin.
+
+Copyright The GNU Toolchain Authors.
+Contributed by Iain Sandoe.
+
+This file is part of GCC.
+
+GCC is free software; you can redistribute it and/or modify
+it under the terms of the GNU General Public License as published by
+the Free Software Foundation; either version 3, or (at your option)
+any later version.
+
+GCC is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+GNU General Public License for more details.
+
+You should have received a copy of the GNU General Public License
+along with GCC; see the file COPYING3.  If not see
+<http://www.gnu.org/licenses/>.  */
+
+/* These functions [scalb* (f, n)] are not provided by the system
+   As of IEEE Std 1003.1, 2004 Edition they are obsoleted.
+   As of IEEE Std 1003.1-2017 (Revision of IEEE Std 1003.1-2008) they are
+   removed.
+
+   Furthermore the informational in the 2004 edition documentation says:
+
+   1. Applications should use either scalbln(), scalblnf(), or scalblnl() in
+      preference to this function.
+
+   2. IEEE Std 1003.1-2001 only defines the behavior for the scalb() function
+      when the n argument is an integer, a NaN, or Inf. The
+      behavior of other values for the n argument is unspecified.
+
+   IEEE Std 1003.1, 2004 does not provide any additional normative description
+   of the behaviour except 'When radix is 2, scalb() shall be equivalent to
+   ldexp()' which is also only specified for integer n; Darwin's float radix
+   is defined to be 2.
+
+   These observations are used to justify a placeholder implementation of scalb
+   in terms of scalbln, since there is clear intent in the Posix documentation
+   to limit the functionality to integral values of n.*/
+
+double scalb (double r, double n)
+{
+  return __builtin_scalbln (r, (long) n);
+}
+
+float scalbf (float r, float n)
+{
+  return __builtin_scalblnf (r, (long) n);
+}
+
+long double scalbl (long double r, long double n)
+{
+  return __builtin_scalblnl (r, (long) n);
+}
diff --git a/libgcc/config/darwin-significand.c b/libgcc/config/darwin-significand.c
new file mode 100644
index 00000000000..19f59ef0b67
--- /dev/null
+++ b/libgcc/config/darwin-significand.c
@@ -0,0 +1,39 @@ 
+/* Implementation of the Glibc significand() functions for Darwin.
+
+Copyright The GNU Toolchain Authors.
+Contributed by Iain Sandoe.
+
+This file is part of GCC.
+
+GCC is free software; you can redistribute it and/or modify
+it under the terms of the GNU General Public License as published by
+the Free Software Foundation; either version 3, or (at your option)
+any later version.
+
+GCC is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+GNU General Public License for more details.
+
+You should have received a copy of the GNU General Public License
+along with GCC; see the file COPYING3.  If not see
+<http://www.gnu.org/licenses/>.  */
+
+/* These just implement the underlying definition.  */
+double
+significand (double r)
+{
+  return __builtin_scalbn (r, -__builtin_ilogb (r));
+}
+
+float
+significandf (float r)
+{
+  return __builtin_scalbnf (r, -__builtin_ilogbf (r));
+}
+
+long double
+significandl (long double r)
+{
+  return __builtin_scalbnl (r, -__builtin_ilogbl (r));
+}
diff --git a/libgcc/config/t-darwin b/libgcc/config/t-darwin
index 299d26c2c96..b9058ad7b27 100644
--- a/libgcc/config/t-darwin
+++ b/libgcc/config/t-darwin
@@ -11,6 +11,16 @@  crttms.o: $(srcdir)/config/darwin-crt-tm.c
 crttme.o: $(srcdir)/config/darwin-crt-tm.c
 	$(crt_compile) -mmacosx-version-min=10.4 -DEND -c $<
 
+darwin-scalb.o: $(srcdir)/config/darwin-scalb.c
+	$(crt_compile) -mmacosx-version-min=10.4 -DEND -c $<
+
+darwin-significand.o: $(srcdir)/config/darwin-significand.c
+	$(crt_compile) -mmacosx-version-min=10.4 -DEND -c $<
+
+libfltext.a: darwin-scalb.o darwin-significand.o
+	$(AR_CREATE_FOR_TARGET) $@ darwin-scalb.o darwin-significand.o
+	$(RANLIB_FOR_TARGET) $@
+
 # Make emutls weak so that we can deal with -static-libgcc, override the
 # hidden visibility when this is present in libgcc_eh.
 emutls.o: HOST_LIBGCC2_CFLAGS += \