-
Notifications
You must be signed in to change notification settings - Fork 241
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
[mlir] Use find_package() to locate MLIR static library #1673
Conversation
@zhanglx13 Could you update https://github.com/ROCmSoftwarePlatform/MIOpen/blob/develop/requirements.txt so that on MIOpen end it can pull in latest commit and run through MIOpen regressions? |
@zhanglx13 FYI this is a ROCm 5.4 feature so can only merge after MIOpen branches. Could you take a look at test failures and try to work through? Just so that when MIOpen finished branching we are good to go. |
I thought MIOpen had branched already? In any case, this is still blocking deploying my changes because of the order the changes are going into MLIR. |
@junliume Do you know how to rebuild the docker image? For now, Jenkins let me set the parameter The last successful CI build shows that the compatibility function is working well
And this also shows that the image used contains the old version of MLIR static library. To test the new exported library, a image needs to be built with the updated version of MLIR in the requirements.txt. |
Hello @zhanglx13. You can rebuild the docker yourself with the docker build arguments that our CI echos, push it to either the internal or the external registry and then override the CI runs using the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@junliume I haven't seen a successful build of the docker image with the new dependencies. Still trying to figure out the cause. |
There is a blocker on the CK side. |
Another issue caused by CK integration. See #1777 for details. |
Decision needs to be made by @junliume @JehandadKhan about CK integration. See #1777 for details. |
a81fa43
to
da83fa7
Compare
Blocked by PR #1761. |
I'd just like to reiterate that the changes in rocMLIR that require this PR to land are already in our 5.4 release branch, and so therefore this PR must land on MIOpen's 5.4 release branch - or at least that's what I can tell of the situation |
The latest version of MLIR exports the static library that can be found by
find_package()
. The changes made on the MLIR side can be found here.New names (static library related):
The corresponding changes on the MIOpen side are:
find_package(rocMLIR 1.0.0)
to find therocMLIR
package of version 1.0.0. This is the first version of MLIR that exports the static library.find_library()
to locate the library in the old way iffind_package()
does not find the package.rocMLIR::rockCompiler
to the link library of targetMIOpen
.requirements.txt
.