[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Fri, 23 Feb 2024 11:26:38 +0800
From: Hongliang Wang <wanghongliang@...ngson.cn>
To: Rich Felker <dalias@...c.org>, musl@...ts.openwall.com
Subject: Re: loongarch64 merge
Hi, Rich
We are very excited to see that LoongArch has been merged in musl.
Next, we will continue to work hard to improve LoongArch, and we
still require your strong support.
Thank you for your help on LoongArch.
Regards,
Hongliang Wang
在 2024/1/30 下午3:35, Hongliang Wang 写道:
>
>
> 在 2024/1/30 上午12:47, Rich Felker 写道:
>> On Thu, Jan 25, 2024 at 12:43:54PM -0500, Rich Felker wrote:
>>> I'm going through where everything was left on this topic and
>>> preparing a patch for merge. This message/thread is to document what
>>> I'm actually doing vs the various submitted versions of the patch
>>> since v5/v6 where the major review took place.
>>>
>>>
>>> Subsequent changes I'm reverting:
>>>
>>> - De-optimization of __get_tp. No motivation for removing the
>>> potentially in-place $tp was provided, and we generally use the
>>> arch's tp in-place unless there's a compiler bug to be worked
>>> around. See powerpc{,64} for an example where it's used, or1k where
>>> we have a probably-obsolete workaround for ancient clang being
>>> broken.
>>>
>>> - unsigned -> unsigned int, etc.
>>>
>>> - Gratuitous whitespace changes in headers that obscure the fact that
>>> a header is a complete duplicate that could eventually be shared
>>> between archs (e.g. bits/float.h, bits/posix.h) or just obscure
>>> what differs from other archs when running diff.
>>>
>>>
>>> Fixes from previous review that were overlooked:
>>>
>>> - Removing SA_RESTORER -- its presence defined as 0 produces wrong
>>> sigaction ABI.
>>>
>>>
>>> Additions:
>>>
>>> - Adding the reloc.h/configure case for single-only float.
>>>
>>> - The new member names for mcontext_t are all in reserved namespace,
>>> so there's no reason to have a separate namespace-clean version of
>>> mcontext_t, and I'm removing the latter.
>>>
>>> - Public member uc_flags with no __, macro for compat with any
>>> existing software using the __-prefixed name.
>>>
>>>
>>> Still TODO:
>>>
>>> I don't think I ever reviewed the apparent rewrite of sigsetjmp and
>>> possibly some other asm that changed between v5 and v8. I'm about to
>>> start looking at that and will follow up.
>>
>> OK, sigsetjmp appears to have been non-working in v5 (it failed to
>> save the argument anywhere, so attempted it use it after it was
>> clobbered in the code path after calling setjmp).
>>
>> v8 fixes this and seems like it should work, but I don't understand
>> the offsets used.
>>
>> The loongarch64 __jmp_buf has 23 slots, but only appears to use 21 of
>> them, with sigsetjmp storing its extra data in the last 2 unused
>> slots. The contract here is supposed to be that the entirety of
>> __jmp_buf belongs to setjmp/longjmp, with sigsetjmp using the extra
>> space in the full jmp_buf/sigjmp_buf type with signal mask and lots of
>> extra space.
>>
>> Were these slots added just for sigsetjmp? If so, that was an ABI
>> mistake, but I don't think it warrants a late change; it's probably
>> better to just leave them there as extensibility.
>
> Yes, the purpose of the last 2 slots of __jmp_buf just used for
> sigsetjmp to storing its extra data.
> Now leave them there as extensibility is OK.
>
> Hongliang Wang
>
>>
>> Either way, I think sigsetjmp should be modified not to rely on
>> storage in space that setjmp is entitled to be able to modify, i.e. it
>> should be using offsets 184 and 184+16, not 168 and 176.
>>
>> Rich
>>
> I understand. I attached a patch to fix this issue. please review it.
>
> diff --git a/src/signal/loongarch64/sigsetjmp.s
> b/src/signal/loongarch64/sigsetjmp.s
> index 992ab1a4..9c0e3ae2 100644
> --- a/src/signal/loongarch64/sigsetjmp.s
> +++ b/src/signal/loongarch64/sigsetjmp.s
> @@ -5,8 +5,8 @@
> sigsetjmp:
> __sigsetjmp:
> beq $a1, $zero, 1f
> - st.d $ra, $a0, 168
> - st.d $s0, $a0, 176
> + st.d $ra, $a0, 184
> + st.d $s0, $a0, 200 #184+8+8
> move $s0, $a0
>
> la.global $t0, setjmp
> @@ -14,8 +14,8 @@ __sigsetjmp:
>
> move $a1, $a0 # Return from 'setjmp' or 'longjmp'
> move $a0, $s0
> - ld.d $ra, $a0, 168
> - ld.d $s0, $a0, 176
> + ld.d $ra, $a0, 184
> + ld.d $s0, $a0, 200 #184+8+8
>
> .hidden __sigsetjmp_tail
> la.global $t0, __sigsetjmp_tail
> (END)
>
> Hongliang Wang
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.
