Skip to content

[CI] Enable building StableHLO in CI #4170

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

penguin-wwy
Copy link
Collaborator

@penguin-wwy penguin-wwy commented Apr 30, 2025

Context: Due to the LLVM version upgrade, stablehlo CI tests have been temporarily disabled.
Ref link of discussion: #4152

Content of the current PR: The stablehlo version has been updated, but there are still compilation issues. The main reason is that stablehlo’s LLVM cannot be updated to the latest version for now, so it cannot stay in sync with torch-mlir. Therefore, I have added a patch file to fix some API differences. This does not affect functionality. The patch is temporary and is intended to help restore CI testing for the stablehlo backend as soon as possible.

@penguin-wwy penguin-wwy marked this pull request as draft April 30, 2025 15:47
@marbre
Copy link
Member

marbre commented Apr 30, 2025

What about patching StableHLO instead of applying a patch as part of the build?

@penguin-wwy penguin-wwy marked this pull request as ready for review May 5, 2025 16:33
@penguin-wwy
Copy link
Collaborator Author

What about patching StableHLO instead of applying a patch as part of the build?

Upstream is currently unable to update the dependent LLVM to the latest version due to some blocking issues openxla/stablehlo#2788. This results in version differences between the LLVM used by upstream and the one we are using, which leads to compilation problems.

Copy link
Collaborator

@vivekkhandelwal1 vivekkhandelwal1 left a comment

Choose a reason for hiding this comment

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

Hi, I'm not sure if this is the right approach to fix this issue. I have never seen any such patch before where we're directly modifying an external repo inside the project. If the issue is not fixed upstream, then why not wait for some more time?

@marbre
Copy link
Member

marbre commented May 6, 2025

Hi, I'm not sure if this is the right approach to fix this issue. I have never seen any such patch before where we're directly modifying an external repo inside the project. If the issue is not fixed upstream, then why not wait for some more time?

I strongly agree as this adds an additional maintenance burden to torch-mlir. Is there any strong reason why this cannot be added to StableHLO upstream?

@penguin-wwy
Copy link
Collaborator Author

This patch file can be removed after the two upstream PRs are merged
openxla/stablehlo#2798
openxla/stablehlo#2797

@vivekkhandelwal1
Copy link
Collaborator

This patch file can be removed after the two upstream PRs are merged openxla/stablehlo#2798 openxla/stablehlo#2797

Let's wait for those upstream PRs to be merged and then directly do a bump instead of modifying an external repo.

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.

4 participants