Skip to content

Conversation

@edgchen1
Copy link
Contributor

@edgchen1 edgchen1 commented Feb 5, 2021

Description
Enable type reduction for ConstantOfShape CPU kernel.

Motivation and Context
Enable potential binary size decrease when limiting enabled types.

@edgchen1 edgchen1 requested a review from skottmckay February 5, 2021 22:57
@edgchen1 edgchen1 requested a review from a team as a code owner February 5, 2021 22:57
// type list type containing the given types
// Note: this is useful for passing TypeLists to macros which don't accept the
// comma-separated template arguments
#define ORT_TYPE_LIST(...) ::onnxruntime::TypeList<__VA_ARGS__>
Copy link
Member

@yuslepukhin yuslepukhin Feb 8, 2021

Choose a reason for hiding this comment

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

(... [](start = 21, length = 4)

Is empty typelist OK? #Resolved

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, that's also valid


In reply to: 572282034 [](ancestors = 572282034)

KernelDefBuilder()
.TypeConstraint("T1", DataTypeImpl::GetTensorType<int64_t>())
.TypeConstraint("T2", BuildKernelDefConstraintsFunctorFromTypeList<EnabledOutputTypes>{}()),
ConstantOfShape);
Copy link
Contributor

@skottmckay skottmckay Feb 10, 2021

Choose a reason for hiding this comment

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

Needs update to pass full and enabled types #Resolved

default:
ORT_THROW("Unsupported value attribute datatype: ", tensor_type);
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it better to use a dispatcher here given otherwise you're defining code for all types even if they're disabled?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

tried using a dispatcher but wasn't able to match the binary size of the switch version when all types are enabled


In reply to: 573371802 [](ancestors = 573371802)

Copy link
Contributor

@skottmckay skottmckay left a comment

Choose a reason for hiding this comment

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

:shipit:

@skottmckay skottmckay merged commit 78e408d into master Feb 12, 2021
@skottmckay skottmckay deleted the edgchen1/constant_of_shape_type_reduction branch February 12, 2021 08:27
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.

4 participants