cutlass icon indicating copy to clipboard operation
cutlass copied to clipboard

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

Open rchardx opened this issue 1 year ago • 5 comments

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: 7d49e6c7e2f8896c47f586706e67e1fb215529dc
  • CUDA 12.4
  • GCC 11.4

rchardx avatar Apr 15 '24 09:04 rchardx

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.

rchardx avatar Apr 15 '24 09:04 rchardx

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.

thakkarV avatar Apr 15 '24 16:04 thakkarV

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.

rchardx avatar Apr 16 '24 02:04 rchardx

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.

github-actions[bot] avatar May 16 '24 03:05 github-actions[bot]

This issue has been labeled inactive-90d due to no recent activity in the past 90 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.

github-actions[bot] avatar Aug 14 '24 03:08 github-actions[bot]