[NVPTX] Custom lower integer<->bf16 conversions for sm_80 #74827
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sm_80 only has f32->bf16 conversions, the remaining integer conversions arrived with sm_90. Use a two-step conversion for sm_80. There doesn't seem to be a way to express this promotion directly within the legalization framework, so fallback on Custom lowering.
copybara-service bot
pushed a commit
to openxla/xla
that referenced
this pull request
Dec 11, 2023
We tried this before with an intrinsic, but that breaks vectorization. Relying on native LLVM types doesn't while delivering the same code improvements. The downside is that LLVM now knows that it's a bfloat instead of a i16 and will optimize based on it. While making this change I had to patch a bunch of holes in the NVPTX LLVM backend, there might be more. Depends on llvm/llvm-project#74827 PiperOrigin-RevId: 589102456
Artem-B
approved these changes
Dec 11, 2023
| @@ -766,6 +766,12 @@ NVPTXTargetLowering::NVPTXTargetLowering(const NVPTXTargetMachine &TM, | |||
| AddPromotedToType(Op, MVT::bf16, MVT::f32); | |||
| } | |||
|
|
|||
| for (MVT VT : {MVT::i1, MVT::i16, MVT::i32, MVT::i64}) { | |||
| setOperationAction( | |||
| {ISD::SINT_TO_FP, ISD::UINT_TO_FP, ISD::FP_TO_SINT, ISD::FP_TO_UINT}, | |||
There was a problem hiding this comment.
Should we make it conditional on SM/PTX here, instead of checking in the custom lowering?
There was a problem hiding this comment.
It would be useful to update the review with the latest changes. I got puzzled for a bit to see this pull request closed with this item marked as done, but unchanged. And then I went to check the actual commit and find the expected changes to be present there.
|
Thanks for the review :) |
d0k
added a commit
that referenced
this pull request
Dec 11, 2023
sm_80 only has f32->bf16 conversions, the remaining integer conversions arrived with sm_90. Use a two-step conversion for sm_80. There doesn't seem to be a way to express this promotion directly within the legalization framework, so fallback on Custom lowering.
copybara-service bot
pushed a commit
to openxla/xla
that referenced
this pull request
Dec 12, 2023
We tried this before with an intrinsic, but that breaks vectorization. Relying on native LLVM types doesn't while delivering the same code improvements. The downside is that LLVM now knows that it's a bfloat instead of a i16 and will optimize based on it. While making this change I had to patch a bunch of holes in the NVPTX LLVM backend, there might be more. Depends on llvm/llvm-project#74827 PiperOrigin-RevId: 589102456
copybara-service bot
pushed a commit
to openxla/xla
that referenced
this pull request
Dec 12, 2023
We tried this before with an intrinsic, but that breaks vectorization. Relying on native LLVM types doesn't while delivering the same code improvements. The downside is that LLVM now knows that it's a bfloat instead of a i16 and will optimize based on it. While making this change I had to patch a bunch of holes in the NVPTX LLVM backend, there might be more. Depends on llvm/llvm-project#74827 PiperOrigin-RevId: 590118269
copybara-service bot
pushed a commit
to tensorflow/tensorflow
that referenced
this pull request
Dec 12, 2023
We tried this before with an intrinsic, but that breaks vectorization. Relying on native LLVM types doesn't while delivering the same code improvements. The downside is that LLVM now knows that it's a bfloat instead of a i16 and will optimize based on it. While making this change I had to patch a bunch of holes in the NVPTX LLVM backend, there might be more. Depends on llvm/llvm-project#74827 PiperOrigin-RevId: 590118269
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
sm_80 only has f32->bf16 conversions, the remaining integer conversions arrived with sm_90. Use a two-step conversion for sm_80.
There doesn't seem to be a way to express this promotion directly within the legalization framework, so fallback on Custom lowering.