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

Copy *tune_gemm* from triton-mlir branch to main_perf branch #614

Conversation

brunomazzottiamd
Copy link
Collaborator

@brunomazzottiamd brunomazzottiamd commented Jul 18, 2024

The source commit in triton-mlir branch is the following one:

commit cf44637139ba441342b909977f51f7cfec2c9963
Author: Lixun Zhang <[email protected]>
Date:   Tue Jul 23 14:22:01 2024 -0500

    [tuning] gemm tuning script v3.3 (#606)

tune_gemm was copied from the source branch directory scripts/amd/gemm
to the destination branch directory python/perf-kernels/tune_gemm.

The SHA-256 hashes of tune_gemm files are the following ones:

423aef1deb6c60f6578a1ecfc94d2473f8746b00d0368c553d31641fcfa5e354  README.md
46ab93978fee33f75df23332f12546dae7910478c391f08b7b1ebd415d8266b7  icache_flush.py
f18711544641b810a652e6a6629bfa2b613f6ade87399e88fdf05b81d4af58a4  matmul.py
84a1c80ede36d3154e51188276eda2d2d0f52ed4f496ff69349c390d83b8ec10  matmul_kernel.py
2812b40183637bc8d7e47d283c7d66b1792134a43de76f3eacf7b9b3e1c2431a  one_config.py
0ac09c33b0173cea06ddabbf9f4e3afa1816781dea4fdcce5894a7e7d6a80e19  rocprof_gemm.py
00eff41cf1c0bfc41d623e42b51706af67639fec76146741e2067d2a93e0148a  utils/file_generator.py
cb7afb773ccee835b00396cccf87e0d44fe513131161f031fae42453725b3c82  utils/utils.py
59f23811b660e49e566927853926a21f02a7014bb19c8ea67e6b382db6c59900  tune_gemm.py
e787f35d750b869f113b3c01692f64243a9cb8a71a18ade2f0465f614f7284e4  tune_gemm.sh

The files were kept as-is despite pre-commit intentions to change them.

@vgokhale

This comment was marked as resolved.

@brunomazzottiamd

This comment was marked as resolved.

@brunomazzottiamd

This comment was marked as resolved.

@vgokhale

This comment was marked as resolved.

@brunomazzottiamd brunomazzottiamd force-pushed the copy-tune-gemm-from-==triton-mlir==-to-==main_perf== branch from 671182a to c492a1c Compare July 19, 2024 17:10
@brunomazzottiamd

This comment was marked as resolved.

The source commit in `triton-mlir` branch is the following one:
```
commit cf44637
Author: Lixun Zhang <[email protected]>
Date:   Tue Jul 23 14:22:01 2024 -0500

    [tuning] gemm tuning script v3.3 (triton-lang#606)
```

*tune_gemm* was copied from the source branch directory `scripts/amd/gemm`
to the destination branch directory `python/perf-kernels/tune_gemm`.

The SHA-256 hashes of *tune_gemm* files are the following ones:
```
423aef1deb6c60f6578a1ecfc94d2473f8746b00d0368c553d31641fcfa5e354  README.md
46ab93978fee33f75df23332f12546dae7910478c391f08b7b1ebd415d8266b7  icache_flush.py
f18711544641b810a652e6a6629bfa2b613f6ade87399e88fdf05b81d4af58a4  matmul.py
84a1c80ede36d3154e51188276eda2d2d0f52ed4f496ff69349c390d83b8ec10  matmul_kernel.py
2812b40183637bc8d7e47d283c7d66b1792134a43de76f3eacf7b9b3e1c2431a  one_config.py
0ac09c33b0173cea06ddabbf9f4e3afa1816781dea4fdcce5894a7e7d6a80e19  rocprof_gemm.py
00eff41cf1c0bfc41d623e42b51706af67639fec76146741e2067d2a93e0148a  utils/file_generator.py
cb7afb773ccee835b00396cccf87e0d44fe513131161f031fae42453725b3c82  utils/utils.py
59f23811b660e49e566927853926a21f02a7014bb19c8ea67e6b382db6c59900  tune_gemm.py
e787f35d750b869f113b3c01692f64243a9cb8a71a18ade2f0465f614f7284e4  tune_gemm.sh
```

The files were kept as-is despite `pre-commit` intentions to change them.
@brunomazzottiamd brunomazzottiamd force-pushed the copy-tune-gemm-from-==triton-mlir==-to-==main_perf== branch from da03061 to 531ffb9 Compare July 25, 2024 14:08
@brunomazzottiamd

This comment was marked as outdated.

@zhanglx13
Copy link

@vgokhale @brunomazzottiamd
Should we land this PR asap so that I can rebase/migrate my latest changes to the tuning script onto the main_perf branch?
Or should we wait for my changes to land first? It's gonna take 1 or 2 days.

@brunomazzottiamd
Copy link
Collaborator Author

brunomazzottiamd commented Jul 25, 2024

@vgokhale @brunomazzottiamd Should we land this PR asap so that I can rebase/migrate my latest changes to the tuning script onto the main_perf branch? Or should we wait for my changes to land first? It's gonna take 1 or 2 days.

As far as I can see, Lixun's changes are easy to incorporate later. I can redo my PR if it's better for the team. For me it's just a matter of prioritizing the PRs. @vgokhale, your opinion is valuable here.

@brunomazzottiamd
Copy link
Collaborator Author

@vgokhale @brunomazzottiamd Should we land this PR asap so that I can rebase/migrate my latest changes to the tuning script onto the main_perf branch? Or should we wait for my changes to land first? It's gonna take 1 or 2 days.

@zhanglx13, go ahead and merge your latest changes! I'll redo this PR later. Thanks for you patience.

@brunomazzottiamd
Copy link
Collaborator Author

@zhanglx13 said his changes (#616) aren't that important and we can merge this PR first.

@brunomazzottiamd brunomazzottiamd merged commit 11e4447 into ROCm:main_perf Aug 13, 2024
3 of 4 checks passed
@brunomazzottiamd brunomazzottiamd deleted the copy-tune-gemm-from-==triton-mlir==-to-==main_perf== branch August 13, 2024 18:28
@micmelesse
Copy link
Collaborator

micmelesse commented Aug 14, 2024

@brunomazzottiamd please don't merge pr if one of the checks is failing. The code formating check is failing in this case. Can you do a follow up pr to fix it

@brunomazzottiamd
Copy link
Collaborator Author

@brunomazzottiamd please don't merge pr if one of the checks is failing. The code formating check is failing in this case. Can you do a follow up pr to fix it

Hi @micmelesse. I didn't notice the check failed, sorry for that...

However, I consciously by-passed the code formatting pre-commit hook with git commit --no-verify. My idea was to copy tune_gemm script AS IS from triton-mlir branch, without changing anything that was not necessary for the correct operation. So we could minimize code review overhead to people already used to tune_gemm code.

I can apply Triton's code formatting pre-commit hook in another PR if it's the right thing to do, but code in triton-mlir branch wasn't following this policy.

@brunomazzottiamd
Copy link
Collaborator Author

By the way, can we disable the merge button if checks are failing?

@micmelesse
Copy link
Collaborator

yep, the merge button is disabled now if the checks don't pass.

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.

5 participants