Skip to content

fix potential issues in allreduce fusion kernel and ut - #4226

Merged
yilin-void merged 1 commit into
NVIDIA:mainfrom
yilin-void:fix/allreduce
May 19, 2025
Merged

fix potential issues in allreduce fusion kernel and ut#4226
yilin-void merged 1 commit into
NVIDIA:mainfrom
yilin-void:fix/allreduce

Conversation

@yilin-void

@yilin-void yilin-void commented May 12, 2025

Copy link
Copy Markdown
Collaborator

potential issues:

  1. The float val[4] is not aligned to 16 bytes, which may result in a misaligned address when reinterpret_cast val to float4*.
  2. There is a typo in the Lamport kernel when handling -0 in the data.
  3. FP4 quant is not supported when sm < 100.

@yilin-void

Copy link
Copy Markdown
Collaborator Author

/bot run --disable-fail-fast

@yilin-void yilin-void self-assigned this May 12, 2025
@yilin-void
yilin-void requested a review from hyukn May 12, 2025 08:26
@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #4860 [ run ] triggered by Bot

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #4860 [ run ] completed with state SUCCESS
/LLM/main/L0_MergeRequest_PR pipeline #3518 completed with status: 'FAILURE'

@yilin-void

Copy link
Copy Markdown
Collaborator Author

/bot run --disable-fail-fast

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #4947 [ run ] triggered by Bot

@yilin-void

Copy link
Copy Markdown
Collaborator Author

/bot kill

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #4964 [ kill ] triggered by Bot

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #4947 [ run ] completed with state ABORTED

@yilin-void

Copy link
Copy Markdown
Collaborator Author

/bot run

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #4964 [ kill ] completed with state SUCCESS
Successfully killed previous jobs for commit 92249f1

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #4967 [ run ] triggered by Bot

@hyukn

hyukn commented May 13, 2025

Copy link
Copy Markdown
Collaborator

Some questions about these potential issues, not sure if I understand them correctly:
1 will not cause illegal memory access, but some precision issue? Because the data is misaligned in the wrong order.
2 will cause overflow/precision issues?
3 will not be a true issue because we have high level path to guarantee that this kernel will not be triggered before Blackwell?

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #4967 [ run ] completed with state SUCCESS
/LLM/main/L0_MergeRequest_PR pipeline #3606 completed with status: 'FAILURE'

@yilin-void

Copy link
Copy Markdown
Collaborator Author

hi @hyukn ,

1 will not cause illegal memory access, but some precision issue? Because the data is misaligned in the wrong order.

This does not cause any problems at present, but customers have reported that this will cause compute-sanitizer to prompt misaligned address.

2 will cause overflow/precision issues?

If the last 8 bytes of a consecutive 16-byte segment in the actual data, when interpreted as two floats, happen to be [-0.f, 0.f], it will cause a hang. However, this bug has not been observed to be triggered so far.

3 will not be a true issue because we have high level path to guarantee that this kernel will not be triggered before Blackwell?

This issue is only relevant to the cpp unit test, in actual use, we will not use fp4 on pre-Blackwell.

@yilin-void

Copy link
Copy Markdown
Collaborator Author

/bot run

@hyukn hyukn left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks a lot for the detailed explanation. LGTM.

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #5006 [ run ] triggered by Bot

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #5006 [ run ] completed with state SUCCESS
/LLM/main/L0_MergeRequest_PR pipeline #3639 completed with status: 'FAILURE'

@yilin-void

Copy link
Copy Markdown
Collaborator Author

/bot run

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #5119 [ run ] triggered by Bot

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #5119 [ run ] completed with state SUCCESS
/LLM/main/L0_MergeRequest_PR pipeline #3731 completed with status: 'FAILURE'

@yilin-void

Copy link
Copy Markdown
Collaborator Author

/bot run

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #5319 [ run ] triggered by Bot

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #5319 [ run ] completed with state SUCCESS
/LLM/main/L0_MergeRequest_PR pipeline #3879 completed with status: 'FAILURE'

Signed-off-by: Yilin Zhang <18275976+yilin-void@users.noreply.github.com>
@yilin-void

Copy link
Copy Markdown
Collaborator Author

/bot run

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #5676 [ run ] triggered by Bot

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #5676 [ run ] completed with state SUCCESS
/LLM/main/L0_MergeRequest_PR pipeline #4147 completed with status: 'SUCCESS'

@yilin-void
yilin-void merged commit 62bb7f9 into NVIDIA:main May 19, 2025
@yilin-void
yilin-void deleted the fix/allreduce branch September 28, 2025 03:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants