Skip to content

Conversation

@charlifu
Copy link
Contributor

The compiler is rejecting an unused var.

Signed-off-by: charlifu <charlifu@amd.com>
@mergify mergify bot added the rocm Related to AMD ROCm label Aug 28, 2025
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request fixes a build issue on ROCm caused by an unused variable. The proposed change adds [[maybe_unused]] to suppress the compiler warning. While this works, a cleaner solution would be to remove the unused variable entirely, as it appears to be dead code. I've suggested this change.


const bool is_active_split = (split_start < tot_slots);
const bool is_last_split = (split_end == tot_slots);
[[maybe_unused]] const bool is_last_split = (split_end == tot_slots);
Copy link
Contributor

Choose a reason for hiding this comment

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

high

Since is_last_split is not used anywhere in this kernel, it's better to remove this line completely rather than marking it as [[maybe_unused]]. This helps in keeping the code clean by removing dead code.

@hongxiayang
Copy link
Collaborator

hongxiayang commented Aug 28, 2025

@charlifu if the variable is not used, you can safely delete it.

Signed-off-by: charlifu <charlifu@amd.com>
@gshtras gshtras added the ready ONLY add when PR is ready to merge/full CI is needed label Aug 28, 2025
Copy link
Member

@yewentao256 yewentao256 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the work!

@gshtras gshtras enabled auto-merge (squash) August 28, 2025 22:31
@vllm-bot vllm-bot merged commit 006477e into vllm-project:main Aug 29, 2025
70 of 72 checks passed
eicherseiji pushed a commit to eicherseiji/vllm that referenced this pull request Sep 9, 2025
FeiDaLI pushed a commit to FeiDaLI/vllm that referenced this pull request Sep 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready ONLY add when PR is ready to merge/full CI is needed rocm Related to AMD ROCm

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants