xrt: fix all warnings #5915
Merged
xrt: fix all warnings #5915
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
hjchen2
reviewed
Aug 17, 2021
| #include "tensorflow/core/platform/logging.h" | ||
| #include "tensorflow/stream_executor/platform/port.h" | ||
|
|
||
| -#if !defined(PLATFORM_GOOGLE) && !defined(PLATFORM_GOOGLE_ANDROID) |
There was a problem hiding this comment.
能否针对xrt模块的编译禁掉某些warning呢，或者把trait warning as error设为false也行。
There was a problem hiding this comment.
这里的问题好像是 tf 和 glog 都定义了 CHECK/CHECK_EQ 这些宏，所以 tf 头文件和 glog 头文件的先后顺序不同会导致宏的定义不同，这确实该被当作 error 吧
There was a problem hiding this comment.
我记得tf的也是把glog的包了一层，实际上用哪一个都不会出问题。但这里确实是重复定义了，所以是不是可以用其他办法来解决，而不是直接修改tf的代码，比如可以尝试在我们的代码里加一个头文件，把重复的宏都undef掉，
#ifdef MACRO
#undef MACRO
#endif然后再单独include tf/glog的头文件。
There was a problem hiding this comment.
tf 本身就可以直接使用 glog，他们设定了 GOOGLE_LOGGING 宏，只不过 stream_executor 里忘了加了，这是 tf 的一个 bug，我已经给 tf 提了 pr。
使用 undef 的方法会提高头文件引用逻辑的复杂性，许多 glog 的头文件实际上是 oneflow/core 里包含的，这样要非常小心地处理头文件引用的顺序，同时 xla 中头文件引用关系比较复杂，预处理出来的单一源文件就有 30 多万行，所以这种方式（比起改正 tf 的一行代码）可能并不是更优秀的、可维护的方式。
hjchen2
approved these changes
Aug 23, 2021
oneflow-ci-bot
requested review from
oneflow-ci-bot
and removed request for
oneflow-ci-bot
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.