|
Message-ID: <2a8d55b0-add3-4d90-1119-4e5c28b6626e@loongson.cn> Date: Mon, 20 Nov 2023 14:11:56 +0800 From: Hongliang Wang <wanghongliang@...ngson.cn> To: musl@...ts.openwall.com Subject: Re: add loongarch64 port v9. Hi, Rich The patch for modify musl dynamic linker has been merged to gcc, and also backported to gcc-12 and gcc-13. The 0001-add-loongarch64-port-v8.patch is still as the latest patch. Thank you very much. Hongliang Wang. 在 2023/11/18 下午12:19, Jingyun Hua 写道: > Hi,Rich > > I'm sorry for wasting everyone's time with my suggestion about the wrong > dynamic connector name, and thank you for always taking the time to > review the code for the musl LoongArch port. > > I carefully looked at the musl code and documentation again, LoongArch > should follow the musl style and use naming consistent with other archs > naming. > > and I saw that gcc also submitted a modification for this: > https://gcc.gnu.org/pipermail/gcc-patches/2023-November/637113.html > > This may be a good solution. After waiting for the modifications of gcc > to be merged, we can add "-sp" to __loongarch_single_float based on the > musl v8 patch, and at the same time, gcc will backport the > modifications to gcc-12 and gcc-13. > > Thank you very much. > > Regards, > Jingyun Hua > > On 11/18/23 1:25 AM, Rich Felker wrote: >> 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 >> >
Powered by blists - more mailing lists
Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.