xiaoxiang781216 commented on code in PR #12475: URL: https://github.com/apache/nuttx/pull/12475#discussion_r1634081995
########## arch/risc-v/src/qemu-rv/qemu_rv_irq_dispatch.c: ########## @@ -53,7 +53,7 @@ * riscv_dispatch_irq ****************************************************************************/ -void *riscv_dispatch_irq(uintptr_t vector, uintptr_t *regs) +void *riscv_dispatch_irq(uintreg_t vector, uintreg_t *regs) Review Comment: all riscv_dispatch_irq need update ########## arch/risc-v/src/common/Toolchain.defs: ########## @@ -117,7 +119,11 @@ endif ifeq ($(CONFIG_ARCH_RV32),y) LDFLAGS += -melf32lriscv else -LDFLAGS += -melf64lriscv + ifeq ($(CONFIG_ARCH_RV64ILP32),y) Review Comment: ``` else ifeq ($(CONFIG_ARCH_RV64ILP32),y) ``` ########## arch/risc-v/include/types.h: ########## @@ -78,6 +87,20 @@ typedef __WCHAR_TYPE__ _wchar_t; typedef int _wchar_t; #endif +/* With rv64ilp32, uintptr_t is 32-bit thus no longer proper for register + * width integers. Thus we introduce the `uintreg_t` here, it can be made + * more general but let's stick to arch/risc-v scope for now. + * + * We could use typedef for uintreg_t also but it goes beyond arch/risc-v + * scope. let's revisit later. + */ + +#ifndef CONFIG_ARCH_RV64ILP32 +#define uintreg_t uintptr_t Review Comment: ``` typedef _uint32_t uintreg_t; ``` ########## arch/risc-v/include/types.h: ########## @@ -54,12 +54,21 @@ typedef unsigned char _uint8_t; typedef signed short _int16_t; typedef unsigned short _uint16_t; +/* We could use `long long` as 64-bit for all ABIs, but that causes + * "format warings" beyond arch/risc-v scope. So let's revisit it later. Review Comment: you need update inttypes.h and limits.h too ########## arch/risc-v/include/types.h: ########## @@ -78,6 +87,20 @@ typedef __WCHAR_TYPE__ _wchar_t; typedef int _wchar_t; #endif +/* With rv64ilp32, uintptr_t is 32-bit thus no longer proper for register + * width integers. Thus we introduce the `uintreg_t` here, it can be made + * more general but let's stick to arch/risc-v scope for now. + * + * We could use typedef for uintreg_t also but it goes beyond arch/risc-v + * scope. let's revisit later. + */ + +#ifndef CONFIG_ARCH_RV64ILP32 +#define uintreg_t uintptr_t +#else +typedef _uint64_t uintreg_t; Review Comment: but why not use typedef -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
