xiaoxiang781216 commented on code in PR #12475:
URL: https://github.com/apache/nuttx/pull/12475#discussion_r1637336771
##########
arch/risc-v/src/common/riscv_internal.h:
##########
@@ -87,16 +87,16 @@
/* Format output with register width and hex */
#ifdef CONFIG_ARCH_RV32
Review Comment:
should we check CONFIG_ARCH_RV64ILP32 too?
##########
arch/risc-v/include/types.h:
##########
@@ -78,6 +87,14 @@ typedef __WCHAR_TYPE__ _wchar_t;
typedef int _wchar_t;
#endif
+/* Use uintreg_t for register-width integers */
+
+#ifdef CONFIG_ARCH_RV32
Review Comment:
need check CONFIG_ARCH_RV64ILP32 too
##########
arch/risc-v/include/inttypes.h:
##########
@@ -29,7 +29,18 @@
* Pre-processor Definitions
****************************************************************************/
-#if defined(CONFIG_ARCH_RV64)
+#if defined(CONFIG_ARCH_RV64ILP32)
+#define _PRI32PREFIX
+#define _PRI64PREFIX "ll"
+#define _PRIPTRPREFIX
+#define _SCN32PREFIX
+#define _SCN64PREFIX "ll"
+#define _SCNPTRPREFIX
+#define INT32_C(x) x
+#define INT64_C(x) x ## l
Review Comment:
l->ll
##########
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:
let's remove the false warning comment
##########
arch/risc-v/src/bl602/bl602_irq_dispatch.c:
##########
@@ -41,7 +41,7 @@
* riscv_dispatch_irq
****************************************************************************/
-void *riscv_dispatch_irq(uintptr_t vector, uintptr_t *regs)
+void *riscv_dispatch_irq(uintptr_t vector, uintreg_t *regs)
Review Comment:
this patch should merge to previous one
##########
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.
+ */
+
#ifdef CONFIG_ARCH_RV64
typedef signed int _int32_t;
typedef unsigned int _uint32_t;
+#ifndef CONFIG_ARCH_RV64ILP32
Review Comment:
should share the same CONFIG_ARCH_RV32 part from line 73-77
##########
arch/risc-v/include/inttypes.h:
##########
@@ -29,7 +29,18 @@
* Pre-processor Definitions
****************************************************************************/
-#if defined(CONFIG_ARCH_RV64)
+#if defined(CONFIG_ARCH_RV64ILP32)
Review Comment:
should we reuse the same definition as CONFIG_ARCH_RV32? e..g.:
```
#define _PRI32PREFIX "l"
```
##########
libs/libc/machine/risc-v/arch_elf.c:
##########
@@ -764,8 +764,8 @@ int up_relocateadd(const Elf_Rela *rel, const Elf_Sym *sym,
}
break;
default:
- berr("ERROR: Unsupported relocation: %ld\n",
- ELF_R_TYPE(rel->r_info));
+ berr("ERROR: Unsupported relocation: %" PRId32 "\n",
Review Comment:
PRIu32
--
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]
