[hipBLASLt] Check rocisa version before running the generator#617
[hipBLASLt] Check rocisa version before running the generator#617KKyang wants to merge 1 commit into
Conversation
KKyang
requested review from
AlexBrownAMD,
Jinp800125,
Serge45,
TonyYHsieh,
aazz44ss,
babakpst,
bnemanich,
davidd-amd,
hcman2,
imcarsonliao,
jichangjichang,
msujon-AMD,
solaslin and
vin-huang
as code owners
| raise RuntimeError(f"rocisa version mismatch: C++ version {cppVersion} != Python version {pyVersion}. Please rebuild rocisa.") | ||
| print(f"rocisa version {cppVersion} (git commit hash) is same as Python version {pyVersion}") | ||
|
|
||
| verifyRocisaVersion() |
There was a problem hiding this comment.
@ellosel Is there a place that is good for running this code and make sure all entrance (Tensile, TensileCreateLibrary, etc.) will trigger the check?
There was a problem hiding this comment.
I wonder if it belongs in an init. I think that would prevent it from running multiple times.
There was a problem hiding this comment.
I can't find a proper function to run this. Any recommendations?
|
I don't love the unconditional subprocess spawn, but I understand it's guarding against an error state. I'm curious, how would one end up in this error state? If you build through the typical documented measures would this problem arise? |
Can you add this comment to a diff? |
| execute_process( | ||
| COMMAND git rev-parse --short HEAD | ||
| WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR} | ||
| OUTPUT_VARIABLE ROCISA_GIT_COMMIT_HASH | ||
| OUTPUT_STRIP_TRAILING_WHITESPACE | ||
| ) | ||
| add_compile_definitions(ROCISA_GIT_COMMIT_HASH="${ROCISA_GIT_COMMIT_HASH}") | ||
|
|
There was a problem hiding this comment.
Why do we need to do this?
There was a problem hiding this comment.
To get the commit hash when compiling, so we can compare with the generator to make sure they are using the same commit.
|
@KKyang please add a description so we understand why these changes are necessary. What problem are you solving? |
| def verifyRocisaVersion(): | ||
| import subprocess | ||
| def getGitVersionPython(): | ||
| return subprocess.check_output( | ||
| ['git', 'rev-parse', '--short', 'HEAD'] | ||
| ).decode('ascii').strip() | ||
| cppVersion = getGitVersion() | ||
| pyVersion = getGitVersionPython() | ||
| if cppVersion != pyVersion: | ||
| raise RuntimeError(f"rocisa version mismatch: C++ version {cppVersion} != Python version {pyVersion}. Please rebuild rocisa.") | ||
| print(f"rocisa version {cppVersion} (git commit hash) is same as Python version {pyVersion}") | ||
|
|
||
| verifyRocisaVersion() |
There was a problem hiding this comment.
Should this function be in rocisa?
There was a problem hiding this comment.
No, we need both. The hash from rocisa and the hash from the generator. Then we compare.
Bumps [rocm-docs-core](https://github.com/ROCm/rocm-docs-core) from 1.18.4 to 1.19.0. - [Release notes](https://github.com/ROCm/rocm-docs-core/releases) - [Changelog](https://github.com/ROCm/rocm-docs-core/blob/develop/CHANGELOG.md) - [Commits](ROCm/rocm-docs-core@v1.18.4...v1.19.0) --- updated-dependencies: - dependency-name: rocm-docs-core dependency-version: 1.19.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> [ROCm/hipSPARSE commit: 6df87c3]
[ROCm/rocFFT commit: 95bf7fc]
|
This pull request has been inactive for 25 days and will be marked as stale. If you would like to keep this PR open, please:
This PR will be automatically closed in 5 days if no further activity occurs. |
|
This pull request has been automatically closed due to inactivity (30 days with no updates). If you'd like to continue working on this, feel free to reopen the PR or create a new one. |
3 participants
If someone choose to use pip install rocisa instead of using the auto-generated shell script, to prevent the user using an outdated rocisa, thus we add a commit hash check.