From mboxrd@z Thu Jan 1 00:00:00 1970 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=-4.3 required=5.0 tests=MAILING_LIST_MULTI, NICE_REPLY_A,RCVD_IN_DNSWL_MED,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL autolearn=ham autolearn_force=no version=3.4.4 Received: (qmail 13402 invoked from network); 15 Aug 2023 09:25:16 -0000 Received: from second.openwall.net (193.110.157.125) by inbox.vuxu.org with ESMTPUTF8; 15 Aug 2023 09:25:16 -0000 Received: (qmail 1783 invoked by uid 550); 15 Aug 2023 09:25:11 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Reply-To: musl@lists.openwall.com Received: (qmail 1746 invoked from network); 15 Aug 2023 09:25:10 -0000 To: musl@lists.openwall.com References: <20230418093844.GP3630668@port70.net> <0fd19586-376c-14ec-a50b-8c561f9f82f2@loongson.cn> <72a1a5d1-6f57-9a12-7775-84e1ff2c1df2@loongson.cn> <64955faf-c213-34bf-c7e2-691064da51f9@loongson.cn> <968652d9-3780-0843-908e-f21bc93b6ee1@loongson.cn> <320c8444.bb9a.189c45903f8.Coremail.zhaixiaojuan@loongson.cn> <20230805154307.GS4163@brightrain.aerifal.cx> <20230813014109.GU4163@brightrain.aerifal.cx> From: Hongliang Wang Message-ID: <278c595b-10fb-2180-b634-6bbcbd5a2614@loongson.cn> Date: Tue, 15 Aug 2023 17:24:30 +0800 User-Agent: Mozilla/5.0 (X11; Linux loongarch64; rv:68.0) Gecko/20100101 Thunderbird/68.7.0 MIME-Version: 1.0 In-Reply-To: <20230813014109.GU4163@brightrain.aerifal.cx> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit X-CM-TRANSID:AQAAf8BxrM5ORNtksw1bAA--.26952S3 X-CM-SenderInfo: pzdqwxxrqjzxhdqjqz5rrqw2lrqou0/ X-Coremail-Antispam: 1Uk129KBj93XoWxtrW8KFyrurW7Gr1ruw4Dtrc_yoWxCr4xpF WYka1kKrWDZF1fJ3s7Ga1fXF4SkaykGFyUJa4fJryUZrWY9FyxtryfKr4Yva9rZr4kuF12 vF4qyry8urs5AagCm3ZEXasCq-sJn29KB7ZKAUJUUUUU529EdanIXcx71UUUUU7KY7ZEXa sCq-sGcSsGvfJ3Ic02F40EFcxC0VAKzVAqx4xG6I80ebIjqfuFe4nvWSU5nxnvy29KBjDU 0xBIdaVrnRJUUUv2b4IE77IF4wAFF20E14v26r1j6r4UM7CY07I20VC2zVCF04k26cxKx2 IYs7xG6rWj6s0DM7CIcVAFz4kK6r1j6r18M28lY4IEw2IIxxk0rwA2F7IY1VAKz4vEj48v e4kI8wA2z4x0Y4vE2Ix0cI8IcVAFwI0_JFI_Gr1l84ACjcxK6xIIjxv20xvEc7CjxVAFwI 0_Gr0_Cr1l84ACjcxK6I8E87Iv67AKxVW8Jr0_Cr1UM28EF7xvwVC2z280aVCY1x0267AK xVW8Jr0_Cr1UM2AIxVAIcxkEcVAq07x20xvEncxIr21l57IF6xkI12xvs2x26I8E6xACxx 1l5I8CrVACY4xI64kE6c02F40Ex7xfMcIj6xIIjxv20xvE14v26r1j6r18McIj6I8E87Iv 67AKxVWUJVW8JwAm72CE4IkC6x0Yz7v_Jr0_Gr1lF7xvr2IY64vIr41lc7I2V7IY0VAS07 AlzVAYIcxG8wCF04k20xvY0x0EwIxGrwCFx2IqxVCFs4IE7xkEbVWUJVW8JwC20s026c02 F40E14v26r1j6r18MI8I3I0E7480Y4vE14v26r106r1rMI8E67AF67kF1VAFwI0_Jr0_Jr ylIxkGc2Ij64vIr41lIxAIcVC0I7IYx2IY67AKxVWUJVWUCwCI42IY6xIIjxv20xvEc7Cj xVAFwI0_Jr0_Gr1lIxAIcVCF04k26cxKx2IYs7xG6r1j6r1xMIIF0xvEx4A2jsIE14v26r 1j6r4UMIIF0xvEx4A2jsIEc7CjxVAFwI0_Jr0_GrUvcSsGvfC2KfnxnUUI43ZEXa7IU1CP fJUUUUU== Subject: Re: [musl] add loongarch64 port v7. 在 2023/8/13 上午9:41, Rich Felker 写道: > On Sat, Aug 05, 2023 at 11:43:08AM -0400, Rich Felker wrote: >> On Sat, Aug 05, 2023 at 02:18:35PM +0800, 翟小娟 wrote: >>> We are currently working on bringing loongarch support to the alpine >>> community. Alpine depends on musl, and Alpine and its users are >>> waiting for musl to support loongarch. This patch seems to be a long >>> time ago, we hope to know the review status of this patch. >> >> Last time I set out to look at it, I hit that the exact patch I had >> previously reviewed and tentatively okayed wasn't stored anywhere >> permanent. I realize this has taken quite a while, and now there is a >> patch on the list, so this coming week I will go back and review the >> diff of that against the previous posted version and mailing list >> comments. As long as everything matches up, I expect to commit it. >> >> Thanks for checking back in on this, and sorry it's taken so long. > > OK, I'm looking at this now as the diffs between v5 and v6 (which I > theoretically reviewed before, but the exact v6 I looked at did not > exist in permanent form) and between v6 and v7. > > v5->v6: > >> -+#define __BYTE_ORDER 1234 >> ++#define __BYTE_ORDER __LITTLE_ENDIAN > > This is gratuitous, mismatches what is done on other archs, and is > less safe. > The modification is based on the following review suggestion( WANG Xuerui reviewed in https://www.openwall.com/lists/musl/2022/10/12/1): `#define __BYTE_ORDER __LITTLE_ENDIAN` could be more consistent with other arches. >> -+TYPEDEF unsigned nlink_t; >> ++TYPEDEF unsigned int nlink_t; > > Gratuitous and in opposite direction of coding style used elsewhere in > musl. There are a few other instances of this too. > Based on the following review question(WANG Xuerui reviewed in https://www.openwall.com/lists/musl/2022/10/12/1): `unsigned int`? Same for other bare `unsigned` usages. I fixed it to a explicit definition. >> -+ register uintptr_t tp __asm__("tp"); >> -+ __asm__ ("" : "=r" (tp) ); >> ++ uintptr_t tp; >> ++ __asm__ __volatile__("move %0, $tp" : "=r"(tp)); >> + return tp; > > Not clear what the motivation is here. Is it working around a compiler > bug? The original form was more optimal. > The modification is based on the following review suggestion( WANG Xuerui reviewed in https://www.openwall.com/lists/musl/2022/10/12/1): While the current approach works, it's a bit fragile [1], and the simple and plain riscv version works too: uintptr_t tp; __asm__ ("move %0, $tp" : "=r"(tp)); [1]:https://gcc.gnu.org/onlinedocs/gcc/Local-Register-Variables.html#Local-Register-Variables >> +#define CRTJMP(pc,sp) __asm__ __volatile__( \ >> -+ "move $sp,%1 ; jr %0" : : "r"(pc), "r"(sp) : "memory" ) >> -+ >> -+#define GETFUNCSYM(fp, sym, got) __asm__ ( \ >> -+ ".hidden " #sym "\n" \ >> -+ ".align 8 \n" \ >> -+ " la.local $t1, "#sym" \n" \ >> -+ " move %0, $t1 \n" \ >> -+ : "=r"(*(fp)) : : "memory" ) >> ++ "move $sp, %1 ; jr %0" : : "r"(pc), "r"(sp) : "memory" ) > > Not clear why this was changed. It was never discussed afaik. It looks > somewhat dubious removing GETFUNCSYM and using the maybe-unreliable > default definition. > Based on the following review question( WANG Xuerui reviewed in https://www.openwall.com/lists/musl/2022/10/12/1): Does the generic version residing in ldso/dlstart.c not work? I found the code logic is consistent with the generic version, So I removed the definition here and replaced it with generic version. > It also looks like v5->v6 rewrote sigsetjmp. > Based on the following review question( WANG Xuerui reviewed in https://www.openwall.com/lists/musl/2022/10/12/1): This is crazy complicated compared to the riscv port, why is the juggling between a0/a1 and t5/t6 necessary? I optimized the code implementations. > v6->v7: > > sigcontext/mcontext_t change mostly makes sense, but the > namespace-safe and full mcontext_t differ in their use of the aligned > attribute and it's not clear that the attribute is needed (since the > offset is naturally aligned and any instance of the object is produced > by the kernel, not by userspace). > >> -+ long __uc_pad; > > This change to ucontext_t actually makes the struct ABI-incompatible > in the namespace-safe version without the alignment attribute. IIRC I > explicitly requested the explicit padding field to avoid this kind of > footgun. Removing it does not seem like a good idea. > Initially, we add __uc_pad to ensure uc_mcontext is 16 byte alignment. Now, we added __attribute__((__aligned__(16))) to uc_mcontext.__extcontext[], this can ensure uc_mcontext is also 16 byte alignment. so __uc_pad is not used. Remove __uc_pad, from the point of struct layout, musl and kernel are consistent. otherwise, I think it may bring a sense of inconsistency between kernel and musl. Due to Loongarch is not merged into musl now, Remove it no compatibility issues. > In stat.h: > >> -+ unsigned long __pad; >> ++ unsigned long __pad1; > > This is gratuitous and makes the definition gratuitously mismatch what > will become the "generic" version of bits/stat.h. There is no contract > for applications to be able to access these padding fields by name, so > no motivation to make their names match glibc's names or the kernel's. > OK. > In fenv.S: > >> ++#ifdef __clang__ >> ++#define FCSR $fcsr0 >> ++#else >> ++#define FCSR $r0 >> ++#endif > > It's not clear to me what's going on here. Is there a clang > incompatibility in the assembler language you're working around? Or > what? If so that seems like a tooling bug that should be fixed. > The GNU assembler cannot correctly recognize $fcsr0, but the LLVM IAS does not have this issue, so make a distinction. This issue has been fixed in GNU assembler 2.41. but for compatible with GNU assembler 2.40 and below, $r0 need reserved. The linux kernel also has a similar distinction: commit 38bb46f94544c5385bc35aa2bfc776dcf53a7b5d Author: WANG Xuerui Date: Thu Jun 29 20:58:43 2023 +0800 LoongArch: Prepare for assemblers with proper FCSR class support The GNU assembler (as of 2.40) mis-treats FCSR operands as GPRs, but the LLVM IAS does not. Probe for this and refer to FCSRs as"$fcsrNN" if support is present. Signed-off-by: WANG Xuerui Signed-off-by: Huacai Chen > Otherwise, everything looks as expected, I think. I'm okay with making > any fixups for merging rather than throwing this back on your team for > more revisions, but can you please follow up and clarify the above? > > Rich >