NVIDIA / cutlass

CUDA Templates for Linear Algebra Subroutines
Other
4.83k stars 834 forks source link

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

Open rchardx opened 2 months ago

rchardx commented 2 months ago

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):

rchardx commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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.

github-actions[bot] commented 1 month ago

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.