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

[BUG] Disordered header files: detail::is_prefetch used before declaration. #1484

Open
rchardx opened this issue Apr 15, 2024 · 4 comments
Open
Labels
bug Something isn't working inactive-30d

Comments

@rchardx
Copy link

rchardx commented Apr 15, 2024

Describe the bug
Disordered header files (use before declaration) in latest CUTLASS version.
If cute/algorithm/prefetch.hpp was included, eventually it will include cutlass/include/cute/arch/copy_sm90_tma.hpp.
This included file uses detail::is_prefetch<CopyOp> while detail::is_prefetch is defined in cute/algorithm/prefetch.hpp but not included yet.

Including link:
cute/algorithm/prefetch.hpp
-> cute/tensor.hpp
-> cute/algorithm/copy.hpp
-> cutlass/include/cute/atom/copy_atom.hpp
-> cutlass/include/cute/atom/copy_traits_sm90_tma.hpp

Steps/Code to reproduce bug
Add

#include "cute/algorithm/prefetch.hpp"

to any example file.
For example, in 57_hopper_grouped_gemm:
bug

Expected behavior
Compilation error occurs:
error

Environment details (please complete the following information):

  • CUTLASS: 7d49e6c
  • CUDA 12.4
  • GCC 11.4
@rchardx rchardx added ? - Needs Triage bug Something isn't working labels Apr 15, 2024
@rchardx
Copy link
Author

rchardx commented Apr 15, 2024

In my opinion, These files (copy/prefetch/...) should not included in cute/tensor.hpp at the end of file:
image

Doing so will let #include "cute/tensor.hpp" introdce too many related objects, declarations, and finally uses of undefined objects causing compilation errors.

@thakkarV
Copy link
Collaborator

thakkarV commented Apr 15, 2024

generally, we like to avoid including internal cute headers directly. is your issue resolved if you include cute/tensor.hpp ? as you noted, tensor.hpp includes prefetch already so you should not have to.

@rchardx
Copy link
Author

rchardx commented Apr 16, 2024

generally, we like to avoid including internal cute headers directly. is your issue resolved if you include cute/tensor.hpp ? as you noted, tensor.hpp includes prefetch already so you should not have to.

Yes, it's resolved. Yet normal users could still get wrong.
If the header files are intended to look like this, CUTLASS should let the preprocessor to generate an error message and causes the compilation to fail when any internal headers are included.

Copy link

This issue has been labeled inactive-30d due to no recent activity in the past 30 days. Please close this issue if no further response or action is needed. Otherwise, please respond with a comment indicating any updates or changes to the original issue and/or confirm this issue still needs to be addressed. This issue will be labeled inactive-90d if there is no activity in the next 60 days.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working inactive-30d
Projects
None yet
Development

No branches or pull requests

2 participants