Skip to content
This repository has been archived by the owner on Apr 11, 2024. It is now read-only.

关于gcc预定义宏 #46

Open
yetist opened this issue Mar 24, 2022 · 4 comments
Open

关于gcc预定义宏 #46

yetist opened this issue Mar 24, 2022 · 4 comments

Comments

@yetist
Copy link
Contributor

yetist commented Mar 24, 2022

起因

给 systemd 增加三元组的 补丁,在不经意间写了以下代码:

#if defined(__loongarch__)
#  if defined(__loongarch32)
...
# elif defined(__loongarch64)
...
#  else
...
#endif

3个人4次 review通过,准备向上游发起 PR前,鬼使神差地查了一下 gcc 源码,发现并没有定义 __loongarch32 这个宏。

讨论

由于存在 __loongarch64 这个宏,想当然的认为也存在 __loongarch32 宏,导致代码出错,并通过了代码评审。

类似的,一些外部客户发现存在 __loongarch__这个宏,往往会使用 __loongarch64__ 这个错误写法,对称美也好,直觉也好,总之是错了。

大部分开发者在写代码时,并不能完全做到时刻都去查代码、文档,大部分时候会凭借直觉和常识来写出貌似正确的代码,所以想在此讨论一下,是否有必要增加类似 __loongarch32__loongarch64__ 这样的宏定义。

@xen0n
Copy link
Contributor

xen0n commented Mar 24, 2022

FYI: RISC-V 最早是定义 __riscv____riscv32 __riscv64 __riscv_hard_float 这样的,但后来他们基于类似的考虑,把 __riscv__ 后边的两个下划线去掉了(所有 RV 预定义宏就都是 __riscv_xxx 形状),把 32/64 也去掉了(他们的姿势是 #if defined(__riscv) && __riscv_xlen = 32 这样)。

按说我们现在还有机会改掉。我在 #28 写了一些别的,也要改掉最好。

@yetist
Copy link
Contributor Author

yetist commented Apr 7, 2022

这个热心老外为 libffcall 增加的 LoongArch 架构补丁,把我给坑了一下,因为他写出了 __loongarch64__ 这个宏。

https://git.savannah.gnu.org/cgit/libffcall.git/tree/trampoline/trampoline.c#n333

#if defined(__loongarch64__)
#define TRAMP_LENGTH 48
#define TRAMP_ALIGN 8
#endif

https://git.savannah.gnu.org/cgit/libffcall.git/tree/trampoline/trampoline.c#n1700

#if defined(__loongarch64__)
  /* Use the GCC built-in. It expands to 'ibar 0'. */
  __clear_cache((void*)function_x,(void*)(function_x+TRAMP_CODE_LENGTH));
#endif
#endif
#endif

@loongson/dev-team 各位大佬,你们真的觉得这不是个问题吗?

@zhuyaliang
Copy link

不论是从美观和习惯来说__loongarch64__更为合适

@xen0n
Copy link
Contributor

xen0n commented Apr 7, 2022

工具链应该是服务开发者的,而不要成为开发者生产力的掣肘,例如 AMD64 就带不带 __ 后缀的符号都提供。

$ gcc -dM -E - < /dev/null | grep -i 'x86\|amd'
#define __amd64 1
#define __x86_64 1
#define __x86_64__ 1
#define __amd64__ 1

鉴于多提供一些符号又不会少块肉,而且目前 LA 的规范其实也不干净了(__loongarch__ but __loongarch64),我觉得还是都加了吧。

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants