Re: [musl] add loongarch64 port v9.

[Hongliang Wang <wanghongliang@loongson.cn>]

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