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

BathNorm tune API #3646

Draft
wants to merge 3 commits into
base: develop
Choose a base branch
from
Draft

BathNorm tune API #3646

wants to merge 3 commits into from

Conversation

bghimireamd
Copy link
Contributor

@bghimireamd bghimireamd commented Mar 21, 2025

Now the priorities for find enforce are:

  1. FindOptions
  2. TuningPolicy (this is the new thing)
  3. MIOPEN_FIND_ENFORCE value
  4. defaulting to FindEnforceAction::None

This PR introduces TuningPolicy (this is the new thing)

This allows us to Tune from API.

miopenSetTuningPolicy(handle, miopenTuningPolicySearch); // set tuning
miopenBatchnorm...(...);
miopenSetTuningPolicy(handle, miopenTuningPolicyNone); // unset tuning

This effect is not limited to bnorm.

@bghimireamd bghimireamd changed the title Ddu/bmorm tune api BathNorm tune API Mar 21, 2025
@@ -84,6 +84,8 @@ using hipblasLt_handle_ptr = MIOPEN_MANAGE_PTR(hipblasLtHandle_t, hipblasLtDestr

struct MIOPEN_EXPORT Handle : miopenHandle
{
miopenTuningPolicy_t tuning_policy;
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be private since you have getters and setters for this?

miopenTuningPolicyNone = 1, /* do not enforce anything */
miopenTuningPolicyDbUpdate = 2, /* do not load existing entry, do not tune */
miopenTuningPolicySearch = 3, /* do not load existing entry, tune */
miopenTuningPolicyDbClean = 5, /* remove existing entry, do not tune */
Copy link
Contributor

@JonathanLichtnerAMD JonathanLichtnerAMD Mar 21, 2025

Choose a reason for hiding this comment

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

I know you mentioned skipping SEARCH_DB_UPDATE, which corresponds to a 4, and was basically equivalent to miopenTuningPolicySearch. But doesn't this new api also apply to convolutions, so wouldn't we want to support SEARCH_DB_UPDATE in this api?

miopenStatus_t miopenSetTuningPolicy(miopenHandle_t handle, miopenTuningPolicy_t newValue)
{
return miopen::try_([&] {
if(newValue < miopenTuningPolicyNone || newValue > miopenTuningPolicyDbClean)
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this is an enum, the user could actually enter a 4 here. I assume this would just default to SEARCH_DB_UPDATE behaviour, but I wanted to double check...

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.

3 participants