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=-3.3 required=5.0 tests=MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 Received: (qmail 16363 invoked from network); 22 Mar 2022 12:59:53 -0000 Received: from mother.openwall.net (195.42.179.200) by inbox.vuxu.org with ESMTPUTF8; 22 Mar 2022 12:59:53 -0000 Received: (qmail 12094 invoked by uid 550); 22 Mar 2022 12:59:50 -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 12056 invoked from network); 22 Mar 2022 12:59:49 -0000 Date: Tue, 22 Mar 2022 08:59:36 -0400 From: Rich Felker To: =?utf-8?B?546L5rSq5Lqu?= Cc: musl@lists.openwall.com, liuxue@loongson.cn, lvjianmin@loongson.cn Message-ID: <20220322125933.GB7074@brightrain.aerifal.cx> References: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Subject: Re: [musl] add loongarch64 port On Tue, Mar 22, 2022 at 11:52:35AM +0800, 王洪亮 wrote: > Hi, > > Thank you for give us some useful suggestions on the first submit > > from 翟小娟 . > > we review code and make changes on code specification, code style > > and code errors. > > > The new patch has been published in > > https://github.com/loongson/musl/tree/loongarch-v1.0. > > > The linux kernel has been published in > > https://github.com/loongson/linux/tree/loongarch-next. > > > we alse published gcc, glibc in https://github.com/loongson > > and submitting to the community. > > > we can supply a physical machine remote login for test musl. > > It has been compiled and run the libc-test successfully. > > > Please code review, thanks! > > > Hongliang Wang > > >From f5df725607675b370daec09a14ad130c792fa0d2 Mon Sep 17 00:00:00 2001 > From: wanghongliang > Date: Mon, 21 Mar 2022 06:13:20 +0800 > Subject: [PATCH] add loongarch64 port. > > Author: xiaojuanZhai > Author: meidanLi > Author: guoqiChen > Author: xiaolinZhao > Author: Fanpeng > Author: jiantaoShan > Author: xuhuiQiang > Author: jingyunHua > Author: liuxue > Author: wanghongliang > > Signed-off-by: wanghongliang > --- > arch/loongarch64/atomic_arch.h | 53 ++++ > arch/loongarch64/bits/alltypes.h.in | 20 ++ > arch/loongarch64/bits/fcntl.h | 40 +++ > arch/loongarch64/bits/fenv.h | 20 ++ > arch/loongarch64/bits/float.h | 16 ++ > arch/loongarch64/bits/hwcap.h | 14 + > arch/loongarch64/bits/posix.h | 2 + > arch/loongarch64/bits/ptrace.h | 4 + > arch/loongarch64/bits/reg.h | 66 +++++ > arch/loongarch64/bits/setjmp.h | 1 + > arch/loongarch64/bits/signal.h | 79 ++++++ > arch/loongarch64/bits/stat.h | 20 ++ > arch/loongarch64/bits/stdint.h | 20 ++ > arch/loongarch64/bits/syscall.h.in | 307 +++++++++++++++++++++ > arch/loongarch64/bits/user.h | 5 + > arch/loongarch64/crt_arch.h | 14 + > arch/loongarch64/kstat.h | 21 ++ > arch/loongarch64/pthread_arch.h | 13 + > arch/loongarch64/reloc.h | 35 +++ > arch/loongarch64/syscall_arch.h | 137 +++++++++ > configure | 1 + > crt/loongarch64/crti.s | 15 + > crt/loongarch64/crtn.s | 12 + > include/elf.h | 66 +++++ > src/fenv/loongarch64/fenv.S | 72 +++++ > src/ldso/loongarch64/dlsym.s | 12 + > src/setjmp/loongarch64/longjmp.S | 37 +++ > src/setjmp/loongarch64/setjmp.S | 34 +++ > src/signal/loongarch64/restore.s | 10 + > src/signal/loongarch64/sigsetjmp.s | 29 ++ > src/thread/loongarch64/__set_thread_area.s | 7 + > src/thread/loongarch64/__unmapself.s | 7 + > src/thread/loongarch64/clone.s | 28 ++ > src/thread/loongarch64/syscall_cp.s | 29 ++ > 34 files changed, 1246 insertions(+) > create mode 100644 arch/loongarch64/atomic_arch.h > create mode 100644 arch/loongarch64/bits/alltypes.h.in > create mode 100644 arch/loongarch64/bits/fcntl.h > create mode 100644 arch/loongarch64/bits/fenv.h > create mode 100644 arch/loongarch64/bits/float.h > create mode 100644 arch/loongarch64/bits/hwcap.h > create mode 100644 arch/loongarch64/bits/posix.h > create mode 100644 arch/loongarch64/bits/ptrace.h > create mode 100644 arch/loongarch64/bits/reg.h > create mode 100644 arch/loongarch64/bits/setjmp.h > create mode 100644 arch/loongarch64/bits/signal.h > create mode 100644 arch/loongarch64/bits/stat.h > create mode 100644 arch/loongarch64/bits/stdint.h > create mode 100644 arch/loongarch64/bits/syscall.h.in > create mode 100644 arch/loongarch64/bits/user.h > create mode 100644 arch/loongarch64/crt_arch.h > create mode 100644 arch/loongarch64/kstat.h > create mode 100644 arch/loongarch64/pthread_arch.h > create mode 100644 arch/loongarch64/reloc.h > create mode 100644 arch/loongarch64/syscall_arch.h > create mode 100644 crt/loongarch64/crti.s > create mode 100644 crt/loongarch64/crtn.s > create mode 100644 src/fenv/loongarch64/fenv.S > create mode 100644 src/ldso/loongarch64/dlsym.s > create mode 100644 src/setjmp/loongarch64/longjmp.S > create mode 100644 src/setjmp/loongarch64/setjmp.S > create mode 100644 src/signal/loongarch64/restore.s > create mode 100644 src/signal/loongarch64/sigsetjmp.s > create mode 100644 src/thread/loongarch64/__set_thread_area.s > create mode 100644 src/thread/loongarch64/__unmapself.s > create mode 100644 src/thread/loongarch64/clone.s > create mode 100644 src/thread/loongarch64/syscall_cp.s > > diff --git a/arch/loongarch64/atomic_arch.h b/arch/loongarch64/atomic_arch.h > new file mode 100644 > index 00000000..bf4805c9 > --- /dev/null > +++ b/arch/loongarch64/atomic_arch.h > @@ -0,0 +1,53 @@ > +#define a_ll a_ll > +static inline int a_ll(volatile int *p) > +{ > + int v; > + __asm__ __volatile__ ( > + "ll.w %0, %1" > + : "=r"(v) > + : "ZC"(*p)); > + return v; > +} > + > +#define a_sc a_sc > +static inline int a_sc(volatile int *p, int v) > +{ > + int r; > + __asm__ __volatile__ ( > + "sc.w %0, %1" > + : "=r"(r), "=ZC"(*p) > + : "0"(v) : "memory"); > + return r; > +} > + > +#define a_ll_p a_ll_p > +static inline void *a_ll_p(volatile void *p) > +{ > + void *v; > + __asm__ __volatile__ ( > + "ll.d %0, %1" > + : "=r"(v) > + : "ZC"(*(void *volatile *)p)); > + return v; > +} > + > +#define a_sc_p a_sc_p > +static inline int a_sc_p(volatile void *p, void *v) > +{ > + long r; > + __asm__ __volatile__ ( > + "sc.d %0, %1" > + : "=r"(r), "=ZC"(*(void *volatile *)p) > + : "0"(v) > + : "memory"); > + return r; > +} > + > +#define a_barrier a_barrier > +static inline void a_barrier() > +{ > + __asm__ __volatile__ ("dbar 0" : : : "memory"); > +} > + > +#define a_pre_llsc a_barrier > +#define a_post_llsc a_barrier I don't see anything wrong here standing out, but haven't reviewed the ISA's requirements on the use of ll/sc to be sure. If anyone else has knowledge to review this part that would be great. > diff --git a/arch/loongarch64/bits/alltypes.h.in b/arch/loongarch64/bits/alltypes.h.in > new file mode 100644 > index 00000000..38871b5f > --- /dev/null > +++ b/arch/loongarch64/bits/alltypes.h.in > @@ -0,0 +1,20 @@ > +#define _Addr long > +#define _Int64 long > +#define _Reg long > + > +#define __BYTE_ORDER 1234 > +#define __LONG_MAX 0x7fffffffffffffffL > + > +TYPEDEF __builtin_va_list va_list; > +TYPEDEF __builtin_va_list __isoc_va_list; > + > +#ifndef __cplusplus > +TYPEDEF int wchar_t; > +#endif > + > +TYPEDEF float float_t; > +TYPEDEF double double_t; > + > +TYPEDEF struct { long long __ll; long double __ld; } max_align_t; > + > +TYPEDEF unsigned nlink_t; va_list and __isoc_va_list are no longer defined by the arch's alltypes.h.in, so they can be removed. I think the rest of that is ok. > diff --git a/arch/loongarch64/bits/fcntl.h b/arch/loongarch64/bits/fcntl.h > new file mode 100644 > index 00000000..9bcbb7ff > --- /dev/null > +++ b/arch/loongarch64/bits/fcntl.h > @@ -0,0 +1,40 @@ > +#define O_CREAT 0100 > +#define O_EXCL 0200 > +#define O_NOCTTY 0400 > +#define O_TRUNC 01000 > +#define O_APPEND 02000 > +#define O_NONBLOCK 04000 > +#define O_DSYNC 010000 > +#define O_SYNC 04010000 > +#define O_RSYNC 04010000 > +#define O_DIRECTORY 0200000 > +#define O_NOFOLLOW 0400000 > +#define O_CLOEXEC 02000000 > + > +#define O_ASYNC 020000 > +#define O_DIRECT 040000 > +#define O_LARGEFILE 0100000 > +#define O_NOATIME 01000000 > +#define O_PATH 010000000 > +#define O_TMPFILE 020000000 > +#define O_NDELAY O_NONBLOCK > + > +#define F_DUPFD 0 > +#define F_GETFD 1 > +#define F_SETFD 2 > +#define F_GETFL 3 > +#define F_SETFL 4 > + > +#define F_SETOWN 8 > +#define F_GETOWN 9 > +#define F_SETSIG 10 > +#define F_GETSIG 11 > + > +#define F_GETLK 5 > +#define F_SETLK 6 > +#define F_SETLKW 7 > + > +#define F_SETOWN_EX 15 > +#define F_GETOWN_EX 16 > + > +#define F_GETOWNER_UIDS 17 AFAICT this file is identical to the generic one (with 32/64 bit #if evaluated already) so it can and should be dropped. > +++ b/arch/loongarch64/bits/signal.h > @@ -0,0 +1,79 @@ > +#if defined(_POSIX_SOURCE) || defined(_POSIX_C_SOURCE) \ > + || defined(_XOPEN_SOURCE) || defined(_GNU_SOURCE) || defined(_BSD_SOURCE) > + > +#if defined(_XOPEN_SOURCE) || defined(_GNU_SOURCE) || defined(_BSD_SOURCE) > +#define MINSIGSTKSZ 4096 > +#define SIGSTKSZ 16384 > +#endif > + > +typedef unsigned long greg_t, gregset_t[32]; > + > +typedef struct sigcontext { > + unsigned long pc; > + gregset_t gregs; > + unsigned int flags; > + unsigned long extcontext[0] __attribute__((__aligned__(16))); > +}mcontext_t; [0] is not valid, and having a flexible array member here is possibly not even useful since I don't think it would be valid to access it via uc->uc_mcontext.extcontext[] since the instance of mcontext_t inside the ucontext struct does not have FAM space belonging to it, even if additional space was allocated past the end of the ucontext_t. In other words, I think compilers would be justified in treating attempts to access it this way as UB and optimizing them out. > diff --git a/arch/loongarch64/bits/stat.h b/arch/loongarch64/bits/stat.h > new file mode 100644 > index 00000000..b620e142 > --- /dev/null > +++ b/arch/loongarch64/bits/stat.h > @@ -0,0 +1,20 @@ > +struct stat { > + dev_t st_dev; > + int __pad1[3]; > + ino_t st_ino; > + mode_t st_mode; > + nlink_t st_nlink; > + uid_t st_uid; > + gid_t st_gid; > + dev_t st_rdev; > + unsigned int __pad2[2]; > + off_t st_size; > + int __pad3; > + struct timespec st_atim; > + struct timespec st_mtim; > + struct timespec st_ctim; > + blksize_t st_blksize; > + unsigned int __pad4; > + blkcnt_t st_blocks; > + int __pad5[14]; > +}; __pad1[3] looks wrong -- st_ino will be aligned on a 64-bit boundary so there can't be an odd number of 32-bit ints before it. Naturally, it will be aligned with additional padding, but the intent here was probably to make all the padding explicit. Moreover, this isn't reflecting a kernel structure, just a userspace type that musl gets to define, so I'm not sure why it's being defined for a new arch with lots of gratuitous padding rather than just using a generic definition. I guess it was just copied from the mips64 one (which also has these problems). If you're already using this in production and don't want to break ABI, I don't see any real need to change it though. It doesn't really matter. Making it __pad1[4] would still be a good idea (and we should fix that on mips64) -- that doesn't break ABI. > diff --git a/arch/loongarch64/pthread_arch.h b/arch/loongarch64/pthread_arch.h > new file mode 100644 > index 00000000..27f50e4c > --- /dev/null > +++ b/arch/loongarch64/pthread_arch.h > @@ -0,0 +1,13 @@ > +static inline uintptr_t __get_tp() > +{ > + uintptr_t tp; > + __asm__ ("or %0, $tp, $zero" : "=r" (tp) ); > + return tp; > +} > + > +#define TLS_ABOVE_TP > +#define GAP_ABOVE_TP 0 > + > +#define DTP_OFFSET 0 > + > +#define MC_PC pc This can be done as something like: static inline uintptr_t __get_tp() { register uintptr_t tp __asm__("tp"); __asm__ ("" : "=r" (tp) ); return tp; } if the compiler doesn't botch it. Historically clang had trouble with that, so if that's the case, just stick with what you had -- it's not too costly doing a gratuitous move. Rich