Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

set_warnings 隔离 nvcc 和 cxx #5380

Open
TOMO-CAT opened this issue Jul 23, 2024 · 15 comments
Open

set_warnings 隔离 nvcc 和 cxx #5380

TOMO-CAT opened this issue Jul 23, 2024 · 15 comments

Comments

@TOMO-CAT
Copy link

你在什么场景下需要该功能?

目前 set_warnings 会对所有 nvcc 和 clang 生效,假设我设置:

set_warnings("all", "extra", "error")

nvcc 编译会直接带上 -Werror cross-execution-space-call,reorder,deprecated-declarations ,按照 nvcc 官网的描述和 clang 区别不大:

https://docs.nvidia.com/cuda/cuda-compiler-driver-nvcc/#command-option-description
image

问题在于 clang 可以通过 -Wno 关闭一些 werror,但是 nvcc 即使带上 supress 也无法屏蔽 error,所以 set_warnings 对我们而言杀伤力太大,希望可以分离:
image
image
image

描述可能的解决方案

目前 add_cxxflags 和 add_cuflags 分别控制 warning,但是和 set_warnings 可能存在参数顺序的问题导致不生效

描述你认为的候选方案

No response

其他信息

No response

@Issues-translate-bot
Copy link

Bot detected the issue body's language is not English, translate it automatically.


Title: set_warnings isolate nvcc and cxx

@waruqi
Copy link
Member

waruqi commented Jul 23, 2024

set_warnings 的目的就是抽象化,消除编译器之间的配置差异,所以区分不了。

目前 add_cxxflags 和 add_cuflags 分别控制 warning,但是和 set_warnings 可能存在参数顺序的问题导致不生效

想要区分,就用 add_cxxflags 单独设置 c warnings,完全不使用 set_warnings 。

或者自己研究下代码里压制,clang/gcc 都是有类似压制方式的,nvcc 本身也是调用的 gcc/clang/msvc ccbin ,按理应该也是可以的。

#pragma clang diagnostic push
#pragma clang diagnostic ignored "-wpadded" 
// warnings
#pragma clang diagnostic pop

问题在于 clang 可以通过 -Wno 关闭一些 werror,但是 nvcc 即使带上 supress 也无法屏蔽 error,所以 set_warnings 对我们而言杀伤力太大,希望可以分离:

nvcc 本身也是调用的clang/gcc,既然 clang 可以,nvcc 应该也是可以的,但是要额外传递 -Xcompiler flag ,才能透传给 clang

add_cuflags("-Xcompiler flag")

@TOMO-CAT
Copy link
Author

想要区分,就用 add_cxxflags 单独设置 c warnings,完全不使用 set_warnings 。

我们是在公司内部的 xmake-repo 下发自己的 mode.release,要求所有人 -Werror,然后自己仓库有特殊需求的自己再加,rule 里面的是 target:on_config 会将 -Werror 加到用户自己定义 cxxflags 后面,导致用户自定义参数失效。所以这个方法行不通。我们也不可能让所有仓库用相同的 cxxflags。

或者自己研究下代码里压制,clang/gcc 都是有类似压制方式的,nvcc 本身也是调用的 gcc/clang/msvc ccbin ,按理应该也是可以的。

这个肯定是每个 repo 的人自己加,他们解决不了 warning 再特异性增加对应的 supress,这个没有问题,但是不影响我全局设置 -Werror。

nvcc 本身也是调用的clang/gcc,既然 clang 可以,nvcc 应该也是可以的,但是要额外传递 -Xcompiler flag ,才能透传给 clang

-Xcompiler flag 只能控制 nvcc 编译 cpp 相关的代码,但是 cu 里面的 warning 关不掉,这是 nvcc 的文档:
image

@Issues-translate-bot
Copy link

Bot detected the issue body's language is not English, translate it automatically.


The purpose of set_warnings is to abstract and eliminate configuration differences between compilers, so they cannot be distinguished.

Currently add_cxxflags and add_cuflags respectively control warnings, but there may be a parameter order problem with set_warnings, causing it to not take effect.

If you want to distinguish, use add_cxxflags to set c warnings separately, and do not use set_warnings at all.

@waruqi
Copy link
Member

waruqi commented Jul 23, 2024

我们是在公司内部的 xmake-repo 下发自己的 mode.release,要求所有人 -Werror,然后自己仓库有特殊需求的自己再加,rule 里面的是 target:on_config 会将 -Werror 加到用户自己定义 cxxflags 后面,导致用户自定义参数失效。所以这个方法行不通。我们也不可能让所有仓库用相同的 cxxflags。

既然 add_cxxflags("-Werror") 顺序靠后,为啥 set_warnings("error") 就行?

还是那句话,set_warnings 没法分开,只能自己用 add_cxxflags, add_cuflags 自己分。。顺序问题,自己 target:get()/target:set() table.insert 自己处理

@Issues-translate-bot
Copy link

Bot detected the issue body's language is not English, translate it automatically.


We issue our own mode.release in the company's internal xmake-repo, requiring everyone to -Werror, and then add it if you have special needs in your own warehouse. The rule inside is target:on_config, which will add -Werror to the user himself. After defining cxxflags, user-defined parameters will become invalid. So this method doesn't work. It is also impossible for all warehouses to use the same cxxflags.

Since add_cxxflags("-Werror") is later in the order, why set_warnings("error") is OK?

Again, set_warnings cannot be separated, you can only use add_cxxflags, add_cuflags to separate them yourself. . Sequence issues, handle it yourself target:get()/target:set() table.insert

@TOMO-CAT
Copy link
Author

TOMO-CAT commented Jul 23, 2024

既然 add_cxxflags("-Werror") 顺序靠后,为啥 set_warnings("error") 就行?

因为set_warnings 在 xmake 里有特殊的处理逻辑,会放到编译参数前面。

还有另一个问题:为什么 set_warnings("all", "extra", "error") 后 nvcc 是只开了 -Werror cross-execution-space-call,reorder,deprecated-declarations 这和预期也不符合呀?官方支持 all warnings be treated as error 的:
image

@TOMO-CAT
Copy link
Author

还是那句话,set_warnings 没法分开,只能自己用 add_cxxflags, add_cuflags 自己分。。顺序问题,自己 target:get()/target:set() table.insert 自己处理

这里面还有 add_cxxflags 和 add_cxflags 的问题,处理起来也相当麻烦。很难要求公司所有 repo 都用一样的写法。

@Issues-translate-bot
Copy link

Bot detected the issue body's language is not English, translate it automatically.


Again, set_warnings cannot be separated, you can only use add_cxxflags, add_cuflags to separate them yourself. . Sequence issues, handle it yourself target:get()/target:set() table.insert

There are also issues with add_cxxflags and add_cxflags, which are quite troublesome to deal with. It is difficult to require all repos of the company to be written in the same way.

@waruqi
Copy link
Member

waruqi commented Jul 23, 2024

还有另一个问题:为什么 set_warnings("all", "extra", "error") 后 nvcc 是只开了 -Werror cross-execution-space-call,reorder,deprecated-declarations 这和预期也不符合呀?官方支持 all warnings be treated as error 的:

这个得问下 @OpportunityLiu 也不是我这加的。1671667

@waruqi
Copy link
Member

waruqi commented Jul 23, 2024

还是那句话,set_warnings 没法分开,只能自己用 add_cxxflags, add_cuflags 自己分。。顺序问题,自己 target:get()/target:set() table.insert 自己处理

这里面还有 add_cxxflags 和 add_cxflags 的问题,处理起来也相当麻烦。很难要求公司所有 repo 都用一样的写法。

搞个统一的 rule 么,就跟 set_warnings("error") 一样,走 target:add("cxflags", "-Werror") table.insert 插入到 flags 最开头,后面每个 -Wno-xxx repo 爱怎么加其他 flags 就自己加啥。。``

@Issues-translate-bot
Copy link

Bot detected the issue body's language is not English, translate it automatically.


Again, set_warnings cannot be separated, you can only use add_cxxflags, add_cuflags to separate them yourself. . Sequence issues, handle it yourself target:get()/target:set() table.insert

There are also issues with add_cxxflags and add_cxflags, which are quite troublesome to deal with. It is difficult to require all repos of the company to be written in the same way.

To make a unified rule, just like set_warnings("error"), use target:add("cxflags", "-Werror") table.insert to insert it at the beginning of flags, and each - Wno-xxx repo Add other flags as you like. . ``

@TOMO-CAT
Copy link
Author

搞个统一的 rule 么,就跟 set_warnings("error") 一样,走 target:add("cxflags", "-Werror") table.insert 插入到 flags 最开头,后面每个 -Wno-xxx repo 爱怎么加其他 flags 就自己加啥。。``

这确实是我的备选方案,就是不太优雅,所以问问看有没有更好的思路。

@Issues-translate-bot
Copy link

Bot detected the issue body's language is not English, translate it automatically.


Make a unified rule, just like set_warnings("error"), use target:add("cxflags", "-Werror") table.insert to insert it at the beginning of flags, and each after -Wno-xxx repo Add other flags as you like. . ``

This is indeed my alternative, but it's not very elegant, so I'm asking if there are any better ideas.

@waruqi
Copy link
Member

waruqi commented Jul 23, 2024

搞个统一的 rule 么,就跟 set_warnings("error") 一样,走 target:add("cxflags", "-Werror") table.insert 插入到 flags 最开头,后面每个 -Wno-xxx repo 爱怎么加其他 flags 就自己加啥。。``

这确实是我的备选方案,就是不太优雅,所以问问看有没有更好的思路。

一种就是调整 error -> -Werror cross-execution-space-call,reorder,deprecated-declarations 的预设值,让它更符合预期 可以跟 @OpportunityLiu 确认下

一种就只能自己调整 flags 顺序

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

No branches or pull requests

3 participants