[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1d6e1915-f262-9ae4-8ef2-0856e89596e5@loongson.cn>
Date: Thu, 24 Mar 2022 10:22:42 +0800
From: 王洪亮 <wanghongliang@...ngson.cn>
To: musl@...ts.openwall.com
Subject: Re: add loongarch64 port
Hi,
I fixed these problems pointed out.
1. delete va_list and __isoc_va_list in arch's alltypes.h.in,
add blksize_t for struct stat and struct kstat.
diff --git a/arch/loongarch64/bits/alltypes.h.in
b/arch/loongarch64/bits/alltypes.h.in
index 38871b5f..7c02c63c 100644
--- a/arch/loongarch64/bits/alltypes.h.in
+++ b/arch/loongarch64/bits/alltypes.h.in
@@ -5,9 +5,6 @@
#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
@@ -16,5 +13,5 @@ TYPEDEF float float_t;
TYPEDEF double double_t;
TYPEDEF struct { long long __ll; long double __ld; } max_align_t;
-
TYPEDEF unsigned nlink_t;
+TYPEDEF int blksize_t;
2. remove arch/loongarch64/bits/fcntl.h
3. adjust struct stat and kstat, keep consistent with kernel in
include/uapi/asm-generic/stat.h
diff --git a/arch/loongarch64/bits/stat.h b/arch/loongarch64/bits/stat.h
index b620e142..b7f4221b 100644
--- a/arch/loongarch64/bits/stat.h
+++ b/arch/loongarch64/bits/stat.h
@@ -1,20 +1,18 @@
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];
+ unsigned long __pad;
off_t st_size;
- int __pad3;
+ blksize_t st_blksize;
+ int __pad2;
+ blkcnt_t st_blocks;
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];
+ unsigned __unused[2];
};
diff --git a/arch/loongarch64/kstat.h b/arch/loongarch64/kstat.h
index 4e39ffac..900f119d 100644
--- a/arch/loongarch64/kstat.h
+++ b/arch/loongarch64/kstat.h
@@ -6,9 +6,9 @@ struct kstat {
uid_t st_uid;
gid_t st_gid;
dev_t st_rdev;
- unsigned long __pad1;
+ unsigned long __pad;
off_t st_size;
- unsigned st_blksize;
+ blksize_t st_blksize;
int __pad2;
blkcnt_t st_blocks;
long st_atime_sec;
4. optimize the __get_tp() implement. I already confirmed with
gcc and clang team, this optimize is ok.
diff --git a/arch/loongarch64/pthread_arch.h
b/arch/loongarch64/pthread_arch.h
index 27f50e4c..95ee4c7a 100644
--- a/arch/loongarch64/pthread_arch.h
+++ b/arch/loongarch64/pthread_arch.h
@@ -1,7 +1,7 @@
static inline uintptr_t __get_tp()
{
- uintptr_t tp;
- __asm__ ("or %0, $tp, $zero" : "=r" (tp) );
+ register uintptr_t tp __asm__("tp");
+ __asm__ ("" : "=r" (tp) );
return tp;
}
Hongliang Wang
在 2022/3/22 下午8:59, Rich Felker 写道:
> 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 翟小娟 <zhaixiaojuan@...ngson.cn>.
>>
>> 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 <wanghongliang@...ngson.cn>
>> Date: Mon, 21 Mar 2022 06:13:20 +0800
>> Subject: [PATCH] add loongarch64 port.
>>
>> Author: xiaojuanZhai <zhaixiaojuan@...ngson.cn>
>> Author: meidanLi <limeidan@...ngson.cn>
>> Author: guoqiChen <chenguoqi@...ngson.cn>
>> Author: xiaolinZhao <zhaoxiaolin@...ngson.cn>
>> Author: Fanpeng <fanpeng@...ngson.cn>
>> Author: jiantaoShan <shanjiantao@...ngson.cn>
>> Author: xuhuiQiang <qiangxuhui@...ngson.cn>
>> Author: jingyunHua <huajingyun@...ngson.cn>
>> Author: liuxue <liuxue@...ngson.cn>
>> Author: wanghongliang <wanghongliang@...ngson.cn>
>>
>> Signed-off-by: wanghongliang <wanghongliang@...ngson.cn>
>> ---
>> 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
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.
