Re: [musl] add loongarch64 port v9.

[Rich Felker <dalias@libc.org>]

On Fri, Nov 17, 2023 at 03:20:58PM +0800, Hongliang Wang wrote:
> 
> 
> 在 2023/11/17 上午12:10, Rich Felker 写道:
> >On Thu, Nov 16, 2023 at 10:54:44AM +0800, Hongliang Wang wrote:
> >>Hi,
> >>
> >>Thank you for your suggestion, I have modified the dynamic linker
> >>name according to the basic ABI types are specified in the ABI
> >>document of the LoongArch, and post 0001-add-loongarch64-port-v9.patch,
> >>as shown in the attachment.
> >>
> >>Based on 0001-add-loongarch64-port-v8.patch,the modifications for
> >>0001-add-loongarch64-port-v9.patch are as follows:
> >>
> >>---
> >>  arch/loongarch64/reloc.h | 10 ++++++----
> >>  configure                |  4 +++-
> >>  2 files changed, 9 insertions(+), 5 deletions(-)
> >>
> >>diff --git a/arch/loongarch64/reloc.h b/arch/loongarch64/reloc.h
> >>index a4482b48..6907de8e 100644
> >>--- a/arch/loongarch64/reloc.h
> >>+++ b/arch/loongarch64/reloc.h
> >>@@ -1,7 +1,9 @@
> >>-#ifdef __loongarch_soft_float
> >>-#define FP_SUFFIX "-sf"
> >>-#else
> >>-#define FP_SUFFIX ""
> >>+#if defined __loongarch_double_float
> >>+#define FP_SUFFIX "-lp64d"
> >>+#elif defined __loongarch_single_float
> >>+#define FP_SUFFIX "-lp64f"
> >>+#elif defined __loongarch_soft_float
> >>+#define FP_SUFFIX "-lp64s"
> >>  #endif
> >>
> >>  #define LDSO_ARCH "loongarch64"  FP_SUFFIX
> >>diff --git a/configure b/configure
> >>index 55d179f1..93b06287 100755
> >>--- a/configure
> >>+++ b/configure
> >>@@ -673,7 +673,9 @@ trycppif __AARCH64EB__ "$t" && SUBARCH=${SUBARCH}_be
> >>  fi
> >>
> >>  if test "$ARCH" = "loongarch64" ; then
> >>-trycppif __loongarch_soft_float "$t" && SUBARCH=${SUBARCH}-sf
> >>+trycppif __loongarch_double_float "$t" && SUBARCH=${SUBARCH}-lp64d
> >>+trycppif __loongarch_single_float "$t" && SUBARCH=${SUBARCH}-lp64f
> >>+trycppif __loongarch_soft_float "$t" && SUBARCH=${SUBARCH}-lp64s
> >>  printf "checking whether compiler support FCSRs... "
> >>  echo "__asm__(\"movfcsr2gr \$t0,\$fcsr0\");" > "$tmpc"
> >>  if $CC -c -o /dev/null "$tmpc" >/dev/null 2>&1 ; then
> >>-- 
> >>
> >>Please review again, and point them out if any questions need to be
> >>modified, thanks.
> >
> >Why are you changing the ABI name for the existing one to something
> >different rather than just adding the missing ones, and doing it with
> >a name that's less descriptive ("-sf" is widely recognized as a
> >softfloat suffix, -lp64s not so much) and adding a redundant "lp64"
> >part to each one that does not seem to be part of distinguishing the
> >float ABI?
> >
> >Rich
> >
> We change the ABI name based on the LoongArch ELF ABI specification,
> which can be seen:
> https://loongson.github.io/LoongArch-Documentation/LoongArch-ELF-ABI-EN.html
> (Table 7. Base ABI Types.)
> The specification defines lp64d, lp64s, lp64f:
> lp64d indicates uses 64-bit FPRs, d indicates double float.
> lp64s indicates uses 32-bit FPRs, s indicates single float.
> lp64f indicates uses no FPRs,  f indicates soft float.
> 
> The specification does not define sf, so I removed it.
> The define in musl is also consistent with gcc.

Please use naming consistent with what we do for other archs in musl
for a proposal to be included in musl. This means:

- Subarch should be empty for the default (I assume that means
  hardware floating point with full double precision) ABI that you
  expect most Linux-compatible systems to be using.

- Don't include extraneous stuff like "lp64" that's universal to the
  architecture in the subarch name. There isn't a need to align these
  names with anything outside of musl.

Please stick with what has already been approved, with changes
well-motivated -- in this case, that means just proposing a name for
the single-precision subarch. My preference would be to use "-sp" like
we did for riscv64.

The reason this has taken so long to get merged is that *every* time I
set aside some time to apply it, there are new gratuitous changes,
many of which seem to be motivated by style musl does not follow. I'd
like to merge precisely what I reviewed last time, with the gratuitous
changes I found reverted, plus the new subarch/ldso name for single
precision. Does this sound good?

Rich