Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Fix][TIR] LowerThreadAllreduce warp reduction mask #17307

Merged

Conversation

MasterJH5574
Copy link
Contributor

The warp reduction implemented by "shuffle down" primitive takes a mask denoting the active threads within the warp that participate in this shuffle.

Previously we compute the mask, while in practice we find that it results in "CUDA illegal instruction" error on NVIDIA H100 GPU when the mask is set, and the issue is gone if we do not update the mask. Therefore, this PR updates the allreduce lowering to remove the mask update.

Confirmed the correctness on the following devices:

  • NVIDIA H100,
  • NVIDIA RTX 4090,
  • AMD Radeon 7900 XTX,
  • Apple M2 Ultra.

The warp reduction implemented by "shuffle down" primitive takes a
mask denoting the active threads within the warp that participate
in this shuffle.

Previously we compute the mask, while in practice we find that it
results in "CUDA illegal instruction" error on NVIDIA H100 GPU when
the mask is set, and the issue is gone if we do not update the mask.
Therefore, this PR updates the allreduce lowering to remove the mask
update.

Confirmed the correctness on the following devices:
* NVIDIA H100,
* NVIDIA RTX 4090,
* AMD Radeon 7900 XTX,
* Apple M2 Ultra.
@MasterJH5574 MasterJH5574 force-pushed the tvm-dev/2024-08-27-lower-allreduce-h100 branch from 5345b05 to b04fcf1 Compare August 28, 2024 14:56
@vinx13 vinx13 merged commit 6ca0bea into apache:main Aug 28, 2024
18 of 19 checks passed
@LeiWang1999
Copy link
Contributor

awesome, I just came across this bug today..

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