[TIR, Relay] improve bfloat16 support #2
Closed
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
yangulei
changed the title
ZhennanQin
reviewed
Jan 20, 2022
.gitignore
Outdated
| @@ -11,7 +11,10 @@ __pycache__/ | |||
| .Python | |||
| env/ | |||
| build/ | |||
| build_debug/ | |||
There was a problem hiding this comment.
Please don't change this. You can change it locally, but don't upsteam.
ZhennanQin
reviewed
Jan 20, 2022
include/tvm/tir/op.h
Outdated
| static const Op& op = Op::Get("tir." #OpName); \ | ||
| if (x.dtype().is_bfloat16()) { \ | ||
| DataType srcType = x.dtype(); \ | ||
| DataType dstType(kDLFloat, 32, srcType.lanes()); \ |
ZhennanQin
reviewed
Jan 20, 2022
| @@ -40,6 +40,8 @@ | |||
| "nn.conv3d_transpose", | |||
| "nn.dense", | |||
| "nn.batch_matmul", | |||
| "nn.bias_add", | |||
There was a problem hiding this comment.
Not sure if we can change this default list. Better to have another CPU list, otherwise you need to evaluate the impact to NV hardware.
ZhennanQin
reviewed
Jan 20, 2022
| @@ -126,3 +155,4 @@ def test_fp16_conversion(target, dev): | |||
| test_basic_build() | |||
| test_fp16_build() | |||
| test_fp16_conversion() | |||
There was a problem hiding this comment.
Do we need to add test_bf16_conversion as fp16?
There was a problem hiding this comment.
|
LGTM! |
|
Thanks for reviewing, this PR has been merged to the official repo. |
yangulei
pushed a commit
that referenced
this pull request
Sep 8, 2022
* Revert "[skip ci] Revert "[ci] Default to n=2 for test parallelism (apache#12376)" (apache#12413)" This reverts commit 478b672. * [ci] Default to n=2 for test parallelism This is attempt #2 of apache#12376 which was reverted in apache#12413. The changes in `plugin.py` should keep all the tests on the same node so sporadic failures don't happen due to scheduling. Co-authored-by: driazati <driazati@users.noreply.github.com>
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.
Motivation:
We are enabling bfloat16 in BYOC-oneDNN following the path: [float32 graph] --> <AMP> --> [bfloat16 graph] --> <BYOC> --> [TVM + oneDNN module]. While some of the Passes like
FoldConstantcan not work for bfloat16 before the improvements below.Changes:
With those improvements, a float32 graph could be converted to bfloat16 through AMP, and then be lowered to inference in bfloat16 mode now.
Tested Models (gluoncv):
As @AndrewZhaoLuo said at apache#8069
Pending:
The support for bfloat16 in BYOC-oneDNN is based on multi-blocking layout transform and the extensions on BYOC-oneDNN and pending.