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

[AMD] Remove cuda2gcn and opencl #3565

Merged
merged 1 commit into from
Apr 4, 2024

Conversation

zhanglx13
Copy link
Collaborator

Since the math functions are no longer lowered to __nv_xxx first before lowered to __ocml/ockl_xxx, we don't need cuda2gcn translation any more on AMD backend.

@@ -52,7 +52,7 @@ def __post_init__(self):
# Ignore user-defined warp size for gfx9
warp_size = 32 if 'gfx10' in self.arch or 'gfx11' in self.arch else 64
object.__setattr__(self, 'warp_size', warp_size)
libs = ["cuda2gcn", "opencl", "ocml", "ockl"]
libs = ["ocml", "ockl"]
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@antiagainst Did you leave opencl for some reason when you cleaned up the bc files?

Copy link
Collaborator

Choose a reason for hiding this comment

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

No particular reason. I was just dropping the ones obviously not needed before due to my change. Didn't inspect whether this one is still needed. Great to know that it can be dropped too!

@zhanglx13 zhanglx13 marked this pull request as ready for review April 4, 2024 14:46
@zhanglx13 zhanglx13 requested a review from ptillet as a code owner April 4, 2024 14:46
@zhanglx13 zhanglx13 requested a review from zahimoud April 4, 2024 14:46
Copy link
Collaborator

@ThomasRaoux ThomasRaoux left a comment

Choose a reason for hiding this comment

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

Nice!

@zhanglx13 zhanglx13 merged commit 70fe526 into triton-lang:main Apr 4, 2024
5 checks passed
micmelesse added a commit to micmelesse/triton that referenced this pull request Apr 10, 2024
zhanglx13 added a commit that referenced this pull request Apr 12, 2024
zahimoud pushed a commit that referenced this pull request Apr 12, 2024
Reverts #3565

Pytorch-inductor's triton codegen utilises math functions not
implemented in tl.math e.g. isinf, signbit
(https://github.com/pytorch/pytorch/blob/41613a0803f7cde7956f039bc80f94253b0843f9/torch/_inductor/codegen/triton.py#L837)
The removal of cuda2gcn causes functionality breakages in inductor until
an alternative lowering is implemented.
This is important for the ongoing effort to utilise upstream triton in
pytorch instead of the ROCm fork.
jataylo added a commit to jataylo/triton that referenced this pull request Apr 15, 2024
jataylo added a commit to ROCm/triton that referenced this pull request Apr 15, 2024
This reverts commit 70fe526.

(cherry picked from commit 8810b53)
zhanglx13 pushed a commit that referenced this pull request Apr 15, 2024
This reverts commit 70fe526.

Initially just cuda2gcn was reverted here
2556127
but some libdevice lowerings in inductor rely on opencl.bc

Example code:
https://gist.github.com/jataylo/a5f804d48d8f831395a208836fd4740a
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