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

frequently merge new asm file to miopen #117

Open
2 tasks
shaojiewang opened this issue Sep 18, 2021 · 3 comments
Open
2 tasks

frequently merge new asm file to miopen #117

shaojiewang opened this issue Sep 18, 2021 · 3 comments
Assignees

Comments

@shaojiewang
Copy link
Contributor

shaojiewang commented Sep 18, 2021

As we continue optimizing the performance and stability for igemmGen and this tool can generate more efficient kernels for igemm or direct conv, we may think about how to merge new asm files to miopen frequently. I think we can have a discussion here.
I could make some proposals here:

@junliume
Copy link

junliume commented Sep 18, 2021

There is no obvious conflict between the two approaches, IMHO.

We could mimic the MIOpen staging approach:

  1. Establish a branch within MIOpen repo, e.g. iGEMM-develop;
  2. Automatically merge most recent ASM kernels generated by this tool to the above branch;
  3. TUNA should keep track of the branch's performance data in its database, and when the "staging" performance is good, merge it to MIOpen develop branch.

Well, we really need the performance tuning and monitoring automation tool (TUNA) to be deployed. i.e. our performance CI 😀
CC: @JehandadKhan @atamazov

@carlushuang
Copy link
Collaborator

For updating the kernel structure list, a possible approach could be:

static const std::vector<PerformanceConfigAsmImplicitGemmGTCBwdXdlopsNHWC> kernel_param_list {
#include <conv_asm_implicit_gemm_gtc_bwd_nhwc_param.h>
};

Then only need to update the structure inside the header file conv_asm_implicit_gemm_gtc_bwd_nhwc_param.h

This way can avoid using regex, in case the regex may have some side effect if we change this cpp file in the future

@atamazov
Copy link

atamazov commented Sep 22, 2021

@carlushuang I am not against "non-standard" usage of the preprocessor. However this requires careful and clean design. Please explicitly ask me to review relevant PRs.

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

No branches or pull requests

6 participants