Add the keyword "template" to member template specialization appears after . or -> in a post-fix expression which is a requirement in C++ standard
#1246
Conversation
…after `.` or `->` in a post-fix expression which is a requirement in C++ standard Signed-off-by: chenwei.sun <chenwei.sun@intel.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @tomflinda, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request addresses a C++ language requirement by introducing the template keyword in specific contexts where member template functions are invoked on dependent types. This change is crucial for ensuring correct compilation under strict C++ standards, particularly in template-heavy codebases, and prevents ambiguity for the compiler.
Highlights
- C++ Standard Compliance: Ensured C++ standard compliance by explicitly adding the
templatekeyword when calling member template functions on dependent types. This resolves potential compilation issues where the compiler might not correctly parse such calls without the keyword, as required by the C++ standard.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request adds the template keyword to member template specializations, which is a requirement in the C++ standard. The changes improve code portability. The review suggests adding the template keyword to other similar instances in the modified files for consistency.
| @@ -194,7 +194,7 @@ __global__ void __launch_bounds__(Ktraits::NUM_WARPS* cutlass::NumThreadsPerWarp | |||
| shared_storage, scheduler, scheduler_params, work_tile_info, block_coord, work_idx, | |||
| num_kv_tiles_outside_items_window, num_kv_tiles_prefix); | |||
| } else { | |||
| collective_mainloop.load<LEFT_SLIDING_WINDOW>( | |||
| collective_mainloop.template load<LEFT_SLIDING_WINDOW>( | |||
There was a problem hiding this comment.
The template keyword is added here to specify that load is a template method. Consider adding the template keyword to other template method calls within this file for consistency and to avoid potential compilation issues with strict compilers.
| @@ -295,7 +295,7 @@ __device__ __forceinline__ void produce_kv(smem_t<KTraits::SWIZZLE_MODE_KV> smem | |||
| for (uint32_t i = 0; i < NUM_MMA_KV * 4 / NUM_WARPS_Q; ++i) { | |||
| #pragma unroll | |||
| for (uint32_t j = 0; j < NUM_MMA_D / (8 / sizeof(DTypeKV)); ++j) { | |||
| smem.load_128b_async<fill_mode>(*smem_offset, *gptr, kv_idx < kv_len); | |||
| smem.template load_128b_async<fill_mode>(*smem_offset, *gptr, kv_idx < kv_len); | |||
There was a problem hiding this comment.
The template keyword is correctly added here. Consider adding the template keyword to the call to advance_offset_by_column on line 299 for consistency, as it's also a member template function call on a dependent name.
| @@ -434,7 +434,7 @@ __device__ __forceinline__ void load_q_global_smem( | |||
| const uint32_t lane_idx = tid.x, warp_idx_x = get_warp_idx_q<KTraits>(tid.y); | |||
|
|
|||
| if (get_warp_idx_kv<KTraits>(tid.z) == 0) { | |||
| uint32_t q_smem_offset_w = q_smem->get_permuted_offset<UPCAST_STRIDE_Q>( | |||
| uint32_t q_smem_offset_w = q_smem->template get_permuted_offset<UPCAST_STRIDE_Q>( | |||
There was a problem hiding this comment.
The template keyword is correctly added here. Consider adding the template keyword to the call to advance_offset_by_column on line 454 for consistency, as it's also a member template function call on a dependent name.
| @@ -1251,12 +1251,12 @@ __device__ __forceinline__ void write_o_reg_gmem( | |||
| vec_cast<DTypeO, float>::cast<8>((DTypeO*)o_frag_f16, o_frag[mma_q][mma_d]); | |||
|
|
|||
| #ifdef FLASHINFER_STMATRIX_M8N8X4_ENABLED | |||
| uint32_t o_smem_offset_w = o_smem->get_permuted_offset<UPCAST_STRIDE_O>( | |||
| uint32_t o_smem_offset_w = o_smem->template get_permuted_offset<UPCAST_STRIDE_O>( | |||
There was a problem hiding this comment.
The template keyword is correctly added here. Consider adding the template keyword to the call to get_permuted_offset on line 1259 and 1271 for consistency, as it's also a member template function call on a dependent name.
| @@ -292,7 +292,7 @@ __device__ __forceinline__ float GetMaxValue(float* in_data, uint32_t row_idx, u | |||
| } | |||
| max_val = max( | |||
| max_val, BlockReduce<float, BLOCK_THREADS, REDUCE_ALGORITHM>(temp_storage.block_prim.reduce) | |||
| .Reduce<VEC_SIZE>(in_data_, cub::Max())); | |||
| .template Reduce<VEC_SIZE>(in_data_, cub::Max())); | |||
There was a problem hiding this comment.
The template keyword is added here to specify that Reduce is a template method. Consider adding the template keyword to other template method calls within this file for consistency and to avoid potential compilation issues with strict compilers.
|
Please install pre-commits and fix the lint checks https://github.com/flashinfer-ai/flashinfer/actions/runs/16261043053/job/45908859346?pr=1246 |
Signed-off-by: chenwei.sun chenwei.sun@intel.com