This is an archive of the discontinued LLVM Phabricator instance.

[Clang][NVPTX]Add NVPTX intrinsics and builtins for CUDA PTX cvt sm80 instructions
ClosedPublic

Authored by JackAKirk on Jan 5 2022, 9:50 AM.

Details

Summary

Adds NVPTX intrinsics and builtins for CUDA PTX cvt instructions for sm80 architectures and above. Requires ptx 7.0.

PTX ISA description of cvt instructions : https://docs.nvidia.com/cuda/parallel-thread-execution/index.html#data-movement-and-conversion-instructions-cvt

Signed-off-by: JackAKirk <jack.kirk@codeplay.com>

Diff Detail

Event Timeline

JackAKirk created this revision.Jan 5 2022, 9:50 AM
JackAKirk requested review of this revision.Jan 5 2022, 9:50 AM
tra added a comment.Jan 5 2022, 11:45 AM

LGTM overall.

clang/include/clang/Basic/BuiltinsNVPTX.def
405

Nit: ff2v2bf is a bit hard to parse. I initially tried to interpret it as "convert ff2v to bf" and was confused about what exactly does 2v part mean -- we already have ff to denote two floats.

Perhaps ff2bf16x2 would be a bit easier to read and understand. It would also work consistently for f16 and tf32 variants below.

JackAKirk updated this revision to Diff 397884.Jan 6 2022, 7:27 AM

Made suggested change to naming convention.

Added a few missing lines from the original patch.

tra accepted this revision.Jan 6 2022, 11:29 AM

LGTM.

clang/test/CodeGen/builtins-nvptx.c
760

Can you try compiling this test file all the way to .o so we're sure that ptxas does accept the PTX we end up generating.

This revision is now accepted and ready to land.Jan 6 2022, 11:29 AM
JackAKirk marked 2 inline comments as done.Jan 12 2022, 6:15 AM

ping @tra

I thought I should let you know that I do not have commit access to land the patch. I'm also happy to wait a little longer in case you think other interested parties might still chime in.

Thanks

clang/include/clang/Basic/BuiltinsNVPTX.def
405

Thanks for the comment. I also think your suggestion is better. I've now switched to this new naming convention.

clang/test/CodeGen/builtins-nvptx.c
760

Sure, I've attached the output ptx for the convert portion of the test file, along with the test: I added store operations in order for the convert instructions to not be optimised away.

Also please be aware that there appears to be a bug in the existing builtins-nvptx.c test file that is apparent when trying to compile down to ptx using the shfl part of the test file: I get:

fatal error: error in backend: Cannot select: intrinsic %llvm.nvvm.shfl.idx.f32
tra added a comment.Jan 12 2022, 10:01 AM

I thought I should let you know that I do not have commit access to land the patch. I'm also happy to wait a little longer in case you think other interested parties might still chime in.

I can land the patch on your behalf. Are you OK to use the name/email in this patch or do you prefer to use a different email for the LLVM commit?

fatal error: error in backend: Cannot select: intrinsic %llvm.nvvm.shfl.idx.f32

Thank you for letting me know. I'll take a look.

JackAKirk marked 2 inline comments as done.EditedJan 12 2022, 10:03 AM

I can land the patch on your behalf. Are you OK to use the name/email in this patch or do you prefer to use a different email for the LLVM commit?

Thanks very much. Yes the name/email in the patch is fine.

tra added a comment.Jan 13 2022, 1:38 PM

I can land the patch on your behalf. Are you OK to use the name/email in this patch or do you prefer to use a different email for the LLVM commit?

Thanks very much. Yes the name/email in the patch is fine.

Note for the future. It would make it easier to land the patch if it was submitted to phabricator as a git patch, according to the instructions here: https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface

Review tracker would then include info about relevant base commit, author, commit message, etc. and it can be easily applied with arc patch to my tree. Using plain diff requires a bit of extra manual work to transfer all of that from the review tracker to the git commit.

I can land the patch on your behalf. Are you OK to use the name/email in this patch or do you prefer to use a different email for the LLVM commit?

Thanks very much. Yes the name/email in the patch is fine.

Note for the future. It would make it easier to land the patch if it was submitted to phabricator as a git patch, according to the instructions here: https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface

Review tracker would then include info about relevant base commit, author, commit message, etc. and it can be easily applied with arc patch to my tree. Using plain diff requires a bit of extra manual work to transfer all of that from the review tracker to the git commit.

OK thanks for the tip. I'll make sure to submit it to Phabricator as a git patch next time. Thanks again for making the commit.