Skip to content

Remove BFloat16 Specialized Code for ReduceSum#10476

Merged
Lafi7e merged 5 commits into
masterfrom
weicwang/reduction
Feb 8, 2022
Merged

Remove BFloat16 Specialized Code for ReduceSum#10476
Lafi7e merged 5 commits into
masterfrom
weicwang/reduction

Conversation

@Lafi7e

@Lafi7e Lafi7e commented Feb 7, 2022

Copy link
Copy Markdown
Contributor

The BFloat16 speciailized code for ReduceSum is required when we used nv_bfloat16 as data type for calculation. Now we switch to BFloat16 type, this piece of code is no longer needed, we can use the default implementation for BFloat16 ReduceSum.

@Lafi7e Lafi7e added the training issues related to ONNX Runtime training; typically submitted using template label Feb 7, 2022
@Lafi7e Lafi7e requested review from weixingzhang and ytaous February 7, 2022 01:49
ytaous
ytaous previously approved these changes Feb 7, 2022
@ytaous ytaous mentioned this pull request Feb 7, 2022

CudnnReduceDescriptor reduce_desc;
if (std::is_same<T, MLFloat16>::value) {
if (std::is_same<T, MLFloat16>::value || std::is_same<T, BFloat16>::value) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

if (

ORT_IF_CONSTEXPR

@yuslepukhin

Copy link
Copy Markdown
Member
typeid(ElemType).name());

Can we make it static_assert() for compile time error and make all specializations constexpr?


Refers to: onnxruntime/core/providers/cuda/cudnn_common.cc:119 in 56a1a6f. [](commit_id = 56a1a6f, deletion_comment = False)

@Lafi7e

Lafi7e commented Feb 8, 2022

Copy link
Copy Markdown
Contributor Author
typeid(ElemType).name());

Can we make it static_assert() for compile time error and make all specializations constexpr?

Refers to: onnxruntime/core/providers/cuda/cudnn_common.cc:119 in 56a1a6f. [](commit_id = 56a1a6f, deletion_comment = False)

To do this, we may need to move those code from cc file to h file? And it's better we do this for all structs/classes in cudnn_common.h and miopen_common.h, which is not related to this PR. Maybe in a new one?

@Lafi7e Lafi7e merged commit 655f490 into master Feb 8, 2022
@Lafi7e Lafi7e deleted the weicwang/reduction branch February 8, 2022 23:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

training issues related to ONNX Runtime training; typically submitted using template

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants