Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[microNPU] Adding a option to enable striping #11263

Merged
merged 1 commit into from
May 12, 2022

Conversation

manupak
Copy link
Contributor

@manupak manupak commented May 10, 2022

This commit adds a cascader option to enable
striping explicitly.

When doing so fixed a bug that is associated
with block config selection, that will be
triggered when striping is disabled.

Co-authored-by: Elen Kalda [email protected]

cc @Mousius @lhutton1

@manupak manupak force-pushed the disable_striping branch from 8a8ee18 to 258715b Compare May 10, 2022 17:30
@github-actions github-actions bot requested review from Mousius and lhutton1 May 10, 2022 17:31
Copy link
Contributor

@lhutton1 lhutton1 left a comment

Choose a reason for hiding this comment

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

Thanks @manupa-arm LGTM! Just one small nit, but feel free to ignore

also cc @ekalda

@@ -105,7 +105,8 @@ std::vector<bool> GetCascadableAxes(const Part& part) {
return cascadable_axes;
}

std::vector<StripeConfig> GenerateOutputStripeConfigs(const Part& part, int stripe_factors) {
std::vector<StripeConfig> GenerateOutputStripeConfigs(const Part& part, int stripe_factors,
bool disable_striping = false) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: probably no need for a default here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed and removed the default.

@manupak manupak force-pushed the disable_striping branch from 258715b to 0b8afb8 Compare May 11, 2022 17:33
@manupak
Copy link
Contributor Author

manupak commented May 11, 2022

After having an offline discussion with @lhutton1 , we also decided to switch the polarity to enable the striping instead because we'd like to use block config selection done by the cascader without striping in a default run.

@manupak manupak changed the title [microNPU] Adding a option to disable striping [microNPU] Adding a option to enable striping May 11, 2022
@manupak manupak force-pushed the disable_striping branch from 0b8afb8 to 9c8fc36 Compare May 11, 2022 17:42
This commit adds a cascader option to enable
striping explicitly.

When doing so fixed a bug that is associated
with block config selection, that will be
triggered when striping is disabled.

Co-authored-by: Elen Kalda <[email protected]>
@manupak manupak force-pushed the disable_striping branch from 9c8fc36 to 73e1f73 Compare May 11, 2022 17:44
Copy link
Contributor

@lhutton1 lhutton1 left a comment

Choose a reason for hiding this comment

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

LGTM!

@lhutton1 lhutton1 merged commit 366a566 into apache:main May 12, 2022
@lhutton1
Copy link
Contributor

Thanks @manupa-arm!

mehrdadh pushed a commit to mehrdadh/tvm that referenced this pull request May 16, 2022
This commit adds a cascader option to enable
striping explicitly.

When doing so fixed a bug that is associated
with block config selection, that will be
triggered when striping is disabled.

Co-authored-by: Elen Kalda <[email protected]>
shtinsa pushed a commit to Deelvin/tvm that referenced this pull request May 17, 2022
This commit adds a cascader option to enable
striping explicitly.

When doing so fixed a bug that is associated
with block config selection, that will be
triggered when striping is disabled.

Co-authored-by: Elen Kalda <[email protected]>
shingjan pushed a commit to shingjan/tvm that referenced this pull request May 17, 2022
This commit adds a cascader option to enable
striping explicitly.

When doing so fixed a bug that is associated
with block config selection, that will be
triggered when striping is disabled.

Co-authored-by: Elen Kalda <[email protected]>
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.

2 participants