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

[LLVM] Include LLVM headers in files that use them, not in llvm_common.h #11888

Merged
merged 1 commit into from
Jun 25, 2022
Merged

[LLVM] Include LLVM headers in files that use them, not in llvm_common.h #11888

merged 1 commit into from
Jun 25, 2022

Conversation

kparzysz-quic
Copy link
Contributor

This is following the same principle we use everywhere else in TVM, that is, every source file includes headers that it depends on. While including unnecessary LLVM headers (which may happen by including llvm_common.h) is not actively harmful, it makes the header dependencies much less transparent.

This is following the same principle we use everywhere else in TVM, that
is, every source file includes headers that it depends on. While including
unnecessary LLVM headers (which may happen by including llvm_common.h)
is not actively harmful, it makes the header dependencies much less trans-
parent.
@kparzysz-quic kparzysz-quic requested review from Lunderberg, mehrdadh and areusch and removed request for Lunderberg and mehrdadh June 24, 2022 19:14
Copy link
Member

@junrushao junrushao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Usually there isnt much difference between the two approaches, but I agree it makes dependency looks more clear

@junrushao junrushao merged commit 59fb421 into apache:main Jun 25, 2022
@kparzysz-quic kparzysz-quic deleted the llvm.2-common-includes branch June 25, 2022 14:12
zxybazh pushed a commit to zxybazh/tvm that referenced this pull request Jun 26, 2022
…n.h (apache#11888)

This is following the same principle we use everywhere else in TVM, that
is, every source file includes headers that it depends on. While including
unnecessary LLVM headers (which may happen by including llvm_common.h)
is not actively harmful, it makes the header dependencies much less trans-
parent.
blackkker pushed a commit to blackkker/tvm that referenced this pull request Jul 7, 2022
…n.h (apache#11888)

This is following the same principle we use everywhere else in TVM, that
is, every source file includes headers that it depends on. While including
unnecessary LLVM headers (which may happen by including llvm_common.h)
is not actively harmful, it makes the header dependencies much less trans-
parent.
mikeseven pushed a commit to mikeseven/tvm that referenced this pull request Sep 27, 2023
…n.h (apache#11888)

This is following the same principle we use everywhere else in TVM, that
is, every source file includes headers that it depends on. While including
unnecessary LLVM headers (which may happen by including llvm_common.h)
is not actively harmful, it makes the header dependencies much less trans-
parent.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants